Re: set_labels_identifiers_target -fsanitize=undefined error

[Sam James]

Gavin Smith <gavinsmith0123@gmail.com> writes:

> On Sat, Nov 04, 2023 at 01:10:47PM +0000, Sam James wrote:
>> 
>> John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
>> 
>> > Hi Gavin!
>> >
>> > On Sat, 2023-11-04 at 11:00 +0000, Gavin Smith wrote:
>> >> The line in question is:
>> >> 
>> >>   memcpy (targets, list_of_labels, labels_number * sizeof(LABEL));
>> >> 
>> >> - again, the second argument of memcpy.
>> >> 
>> >> However, main/targets.c was only introduced after Texinfo 7.1 so
>> >> this is not the original problem.
>> >
>> > I'll provide a backtrace as well as the commit that introduced the 
>> > regression
>> > on SPARC within the next days. Need to set up two new SPARC servers next 
>> > week
>> > first.
>> >
>> 
>> OK, I tried this out on sparc with Gavin's fix on master, and got...
>> 
>> export UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1
>> ./autogen.sh;  ./configure PERL_EXT_CFLAGS="-O2 -ggdb3
>> -fsanitize=undefined" CFLAGS="-O2 -ggdb3 -fsanitize=undefined"   ; make
>> -j$(nproc) ; make check -j$(nproc)
>> 
>> parsetexi/tree.c:77:11: runtime error: member access within misaligned 
>> address 0x0100010e9744 for type 'struct ELEMENT', which requires 8 byte 
>> alignment
>> 0x0100010e9744: note: pointer points here
>>   00 00 01 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  
>> 00 00 00 00 00 00 00 00
>>               ^
>>     #0 0xfff8000102fc12ec in new_element parsetexi/tree.c:77
>
> It is probably a problem with the obstack allocation method that was being
> used for ELEMENT (an attempted optimisation, although it's questionable how
> successful it was).
>
> According to the glibc manual,
>
>   Each obstack has an “alignment boundary”; each object allocated in the
>   obstack automatically starts on an address that is a multiple of the
>   specified boundary.  By default, this boundary is aligned so that the
>   object can hold any type of data.
>
> (Info node (libc)Obstacks Data Alignment.)
>
> However, we also use the gnulib obstacks module.  Between glibc and gnulib,
> it was evidently not meeting this promise.
>

Oh, interesting. I knew of obstacks but I never looked at the implementations.

> You can try to set the alignment explicitly:
>
> diff tree.c.old tree.c -u
> --- tree.c.old  2023-11-04 16:15:13.632755680 +0000
> +++ tree.c      2023-11-04 16:16:36.211072521 +0000
> @@ -43,7 +43,10 @@
>    if (obs_element_first)
>      obstack_free (&obs_element, obs_element_first);
>    else
> -    obstack_init (&obs_element);
> +    {
> +      obstack_alignment_mask (&obs_element) = 7; /* 8-byte alignment */
> +      obstack_init (&obs_element);
> +    }
>  
>    obs_element_first = obstack_alloc (&obs_element, sizeof (int));
>
>
> Can you check if that works?

Thanks! This both survives the build and the test suite passes.