Re: divdi3: incomplete fix in db3d848e154b?

[Jeff Law <law at redhat dot com>]

On 05/15/2017 08:58 AM, Adhemerval Zanella wrote:
> On 14/05/2017 03:22, Alexey Neyman wrote:
> > Hi,
> >
> > I was compiling glibc (2.25 + d40dbe722f004) for sh4-unknown-linux-gnu and the build failed due to rtld pulling abort.os again. This has been "fixed" in master in db3d848e154b - which made divdi3 built only for certain architectures. However, I think this just masks the real issue that GCC7 inserts a trap where it knows something gets divided by zero.
> >
> > Here is a fragment of generated code for i686-unknown-linux:
> >
> > objdump -dlr csu/divdi3.os:
> > /home/avn/work/ctng/crosstool-ng/.build/src/glibc-2.25/csu/../sysdeps/wordsize-32/divdi3.c:91
> >    d0:   85 db                   test   %ebx,%ebx
> >    d2:   75 0c                   jne    e0 <__udivmoddi4+0xe0>
> >    d4:   0f 0b                   ud2
> >
> > divdi3.c:
> > 90          if (d0 == 0)
> > 91            d0 = 1 / d0;        /* Divide intentionally by zero.  */
> >
> > Isn't the purpose of this intentional division to generate a SIGFPE, rather than SIGILL?
> >
> > Shouldn't divdi3.c do something else here, e.g. raise(SIGFPE) or be compiled with -fno-sanitize=integer-divide-by-zero?
> >
> > Regards,
> > Alexey.
>
> This is a known issue with GCC for SH [1], which still does not have support for
> __builtin_trap.  From last comments it is still in discussion if the trap will
> use either the '#trap' instruction or a undefined instruction.
>
> Unless we get an unconditional trap fix this fix will be a better effort in the
> sense each new GCC version will bring potentially new analysis that might
> generate the trap instructions for current code.  So it is basically two efforts
> here: fix/avoid any glibc code that might generate this kind of trap for SH
> (d40dbe722f004 for instance) and check if the resulting gcc build actually
> get it right (db3d848e154b fix for instance).
>
> Another option could still requiring using GCC specific flags to build glibc
> on SH (-fno-isolate-erroneous-paths-dereference and
> -fno-isolate-erroneous-paths-attribute for instance).  I would prefer to avoid
> it and have a SH support from default headers (assuming gcc was not configured
> to use them).
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70216
Carlos and I discussed some stuff in this space a year or so ago in 
person.  His assertion was that creating a trap instruction when 
building glibc, even if GCC found undefined behaviour was fundamentally 
wrong.

The basic idea was that if glibc happened to be working, even though it 
was tickling undefined behaviour that converting the UB into a trap 
could result in an un-bootable system and code that just halts 
unexpectedly when it worked before.  The better behaviour was to just 
let the UB occur and allow the system to continue to run (I'm going from 
memory, so if I've missed something Carlos, don't hesitate to correct me).

I certainly understand Carlos's position, even if I don't 100% agree 
with it (particularly from a security standpoint).  However, given that 
position, building all glibc targets with -fno-isolate-erroneous-paths 
would seem to be the right thing to do.

Right now GCC will only convert NULL pointer dereferences and division 
by zero to a trap.  However, I expect that by the time GCC 8 is ready 
we'll also be converting out-of-bounds array accesses (only those which 
are proved out of bounds, of course).  There may be other cases of UB 
that we ultimately want to convert -- ie, we shouldn't assume we're 
necessarily done with conversion of erroneous code to traps.

Jeff