Re: set_labels_identifiers_target -fsanitize=undefined error

[John Paul Adrian Glaubitz]

Hi!

On Sat, 2023-11-04 at 16:38 +0000, Gavin Smith wrote:
> 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.
> 
> 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?

Yes, I can confirm that this patch fixes the crash for me.

Would be great if this fix could be included for the next release!

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913