Re: feature/asan-gc-poisoning 5c653d3ec9: Add support for additional memory checks using AddressSanitizer.

Re: feature/asan-gc-poisoning 5c653d3ec9: Add support for additional memory checks using AddressSanitizer.

[Po Lu]

Vibhav Pant <vibhavp@gmail.com> writes:

>     Add support for additional memory checks using AddressSanitizer.
>     
>     When Emacs is compiled with AddressSanitizer support, enable
>     poisoning/unpoisoning freed/unused Lisp objects and other internal
>     memory management structures. If enabled, this will mark freed bytes
>     that have been put on free lists for future use, and initially
>     allocated memory blocks/chunks as "poisoned", triggering an ASan error
>     if they are accessed improperly. Structures are unpoisoned when they
>     have been taken off their respective free lists.
>     
>     * configure.ac: Check for the existence of the ASan API header.
>     
>     * src/alloc.c (ASAN_POISON_ABLOCK, ASAN_UNPOISON_ABLOCK)
>     (ASAN_POISON_INTERVAL_BLOCK, ASAN_UNPOISON_INTERVAL_BLOCK)
>     (ASAN_POISON_INTERVAL, ASAN_UNPOISON_INTERVAL)
>     (ASAN_PREPARE_DEAD_SDATA, ASAN_PREPARE_LIVE_SDATA)
>     (ASAN_POISON_SBLOCK_DATA, ASAN_POISON_STRING_BLOCK)
>     (ASAN_UNPOISON_STRING_BLOCK, ASAN_POISON_STRING)
>     (ASAN_UNPOISON_STRING, ASAN_POISON_FLOAT_BLOCK)
>     (ASAN_UNPOISON_FLOAT_BLOCK, ASAN_POISON_FLOAT)
>     (ASAN_UNPOISON_FLOAT, ASAN_POISON_CONS_BLOCK)
>     (ASAN_POISON_CONS, ASAN_UNPOISON_CONS)
>     (ASAN_POISON_VECTOR_CONTENTS, ASAN_UNPOISON_VECTOR_CONTENTS)
>     (ASAN_UNPOISON_VECTOR_BLOCK, ASAN_POISON_SYMBOL_BLOCK)
>     (ASAN_UNPOISON_SYMBOL_BLOCK, ASAN_POISON_SYMBOL)
>     (ASAN_UNPOISON_SYMBOL) [ADDRESS_SANITIZER]: New functions. When
>     address sanitization is enabled, define them to poison/unpoison
>     objects.
>     
>     (lisp_align_malloc): Poison newly allocated blocks on `free_ablock',
>     unpoison ablocks taken from it respectively.
>     (lisp_align_free): Poison individual ablocks when they are put on the
>     free list, unpoison them when an entire `ablocks' chunk is being
>     freed.
>     
>     (make_interval): Poison interval blocks on initial allocation,
>     unpoison individual intervals on allocation and removal from
>     `interval_free_list'.
>     (sweep_intervals): Unpoison interval blocks before sweeping, poison
>     dead/unmarked intervals.
>     
>     (allocate_string): Poison string blocks on initial allocation,
>     unpoison Lisp_Strings on removal from the free list.
>     (allocate_string_data): Poison `sblock' data on initial allocation,
>     unpoison individual `sdata' contents on allocation or removal from the
>     free list. Call `ASAN_PREPARE_LIVE_SDATA' on the new `sdata' struct.
>     (sweep_strings): Unpoison string blocks before sweeping them,
>     poisoning dead strings and their sdata afterwards.
>     (compact_small_strings): Call `ASAN_PREPARE_LIVE_DATA' on the `sdata'
>     to where compacted strings to moved to.
>     (pin_string): Call `ASAN_PREPARE_DEAD_SDATA' on `old_sdata'.
>     
>     (make_float): Poison float blocks on allocation, unpoisoning
>     individual Lisp_Floats on allocation or removal from
>     `float_free_list'.
>     (sweep_floats): Unpoison float blocks before sweeping, poison
>     dead/unmarked floats.
>     
>     (free_cons): Poison `ptr'.
>     (Fcons): Poison cons blocks on allocation, unpoisoning individual
>     Lisp_Cons on allocation or removal from `cons_free_list'.
>     (sweep_conses): Poison dead/unmarked conses.
>     
>     (setup_free_list): Poison vectors put on `vector_free_lists'.
>     (allocate_vector_from_block): Unpoison vectors taken from the free
>     list, poison excess vector bytes when vectors allocated from the free
>     list are larger than requested.
>     (sweep_vectors): Unpoison vector blocks before sweeping them.
>     
>     (Fmake_symbol): Poison symbol blocks on initial allocation,
>     unpoisoning individual Lisp_Symbols on allocation or removal from
>     `symbol_free_list'.
>     (sweep_symbols): Unpoison symbol blocks before sweeping, poisoning
>     dead/unmarked symbols.

Thank you for writing the change log entry correctly this time!
> +   This feature can be disabled wtih the run-time flag
> +   `allow_user_poisoning' set to zero.
> +*/

Please put "*/" at the end of "zero.".

> +  (void) (b);

> +  (void) (b);

I thought Emacs doesn't make the compiler warn about unused arguments.

> +#if GC_ASAN_POISON_OBJECTS
> +	/* Ensure that accessing excess bytes does not trigger ASan.
> +	 */
> +	__asan_unpoison_memory_region (ADVANCE (vector, nbytes),
> +				       restbytes);
> +#endif

Please fix the comment here as well.

The rest LGTM.  Thanks for paying attention to our coding style this
time.

Re: feature/asan-gc-poisoning 5c653d3ec9: Add support for additional memory checks using AddressSanitizer.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Vibhav Pant <vibhavp@gmail.com>
> Date: Wed, 30 Nov 2022 12:58:09 +0800
> 
> Vibhav Pant <vibhavp@gmail.com> writes:
> 
> >     Add support for additional memory checks using AddressSanitizer.
> >     
> >     When Emacs is compiled with AddressSanitizer support, enable
> >     poisoning/unpoisoning freed/unused Lisp objects and other internal
> >     memory management structures. If enabled, this will mark freed bytes
> >     that have been put on free lists for future use, and initially
> >     allocated memory blocks/chunks as "poisoned", triggering an ASan error
> >     if they are accessed improperly. Structures are unpoisoned when they
> >     have been taken off their respective free lists.

To be more useful, this should be mentioned in etc/DEBUG, where we describe
how to build and run Emacs with memory-debugging facilities.

Thanks.