Message ID | alpine.DEB.2.10.1502201807380.30214@digraph.polyomino.org.uk |
---|---|
State | New |
Headers | show |
On Fri, Feb 20, 2015 at 06:07:53PM +0000, Joseph Myers wrote: > soft-fp calls abort in various cases that the code doesn't handle, all > cases that should never actually occur for any supported choice of > types. > > Calling an abort function is not appropriate for kernel use, so the > Linux kernel redefines abort as a macro in various ways in the ports > using this code, typically to "return 0" or similar. > > One use of abort in soft-fp is inside a comma expression and doesn't > work with such a macro. This patch changes it to use a statement > expression. > > Tested for powerpc-nofpu that installed shared libraries are unchanged > by this patch. Committed. > > (There are two classes of aborts: those to make control flow visible > to the compiler, in default cases of switches over _FP_CLS_COMBINE, > which could reasonably change to __builtin_unreachable for glibc but > would still need to handle pre-4.5 compilers for kernel use, and those > intended to detect any use of soft-fp for combinations of types the > code doesn't know how to handle, which could reasonably become link > failures if the calls should always be optimized away. But those are > separate possible future enhancements.) > > 2015-02-20 Joseph Myers <joseph@codesourcery.com> > > * soft-fp/op-common.h (_FP_FROM_INT): Wrap call to abort in > expression inside statement expression. > > diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h > index 342532a..14fd6cd 100644 > --- a/soft-fp/op-common.h > +++ b/soft-fp/op-common.h > @@ -1815,7 +1815,7 @@ > X##_e = (_FP_EXPBIAS_##fs + 2 * _FP_W_TYPE_SIZE - 1 \ > - _FP_FROM_INT_lz); \ > }) \ > - : (abort (), 0))); \ > + : ({ abort (); 0; }))); \ > \ > if ((rsize) - 1 + _FP_EXPBIAS_##fs >= _FP_EXPMAX_##fs \ > && X##_e >= _FP_EXPMAX_##fs) \ Could these be replaced by __builtin_trap() or similar? I'm not clear on whether this code is purely glibc+kernel or also used by gcc, but if it's used by gcc too it would be very much preferable not to have libgcc pulling in a reference to abort (this affects -ffreestanding, etc.). Rich
From: Rich Felker <dalias@libc.org> Date: Fri, 20 Feb 2015 13:46:14 -0500 > Could these be replaced by __builtin_trap() or similar? I'm not clear > on whether this code is purely glibc+kernel or also used by gcc, but > if it's used by gcc too it would be very much preferable not to have > libgcc pulling in a reference to abort (this affects -ffreestanding, > etc.). Another thing that occurs to me is that perhaps this can be coded in such a way as to cause compile time errors.
On Fri, 20 Feb 2015, Rich Felker wrote: > Could these be replaced by __builtin_trap() or similar? I'm not clear > on whether this code is purely glibc+kernel or also used by gcc, but > if it's used by gcc too it would be very much preferable not to have > libgcc pulling in a reference to abort (this affects -ffreestanding, > etc.). This code is used by libgcc as well. The particular case in this patch is one that could become a call to a nonexistent __softfp_link_failure or similar. The ones that may sometimes end up as abort calls could be __builtin_unreachable. There is never a need for an actual abort. (Actually the aborts from _FP_CLS_COMBINE switches seem to get optimized out by current GCC anyway - I don't see them in the TFmode functions in libgcc from a recent x86_64 build, for example; looking at some old powerpc libraries I have to hand, I see them from GCC 4.3 but not 4.4. The ones in those switches are the only ones needing more than simple constant folding to optimize away.)
On Fri, 20 Feb 2015, David Miller wrote: > From: Rich Felker <dalias@libc.org> > Date: Fri, 20 Feb 2015 13:46:14 -0500 > > > Could these be replaced by __builtin_trap() or similar? I'm not clear > > on whether this code is purely glibc+kernel or also used by gcc, but > > if it's used by gcc too it would be very much preferable not to have > > libgcc pulling in a reference to abort (this affects -ffreestanding, > > etc.). > > Another thing that occurs to me is that perhaps this can be coded > in such a way as to cause compile time errors. Static assertions (again, allowing for older GCC being used to compile the kernel) may well work for at least some of the cases where it's a matter of "soft-fp doesn't handle this possibility". (Subject to making sure the relevant quantities are always constants; sometimes the kernel may use non-constant arguments even when glibc and libgcc always use constants.)
diff --git a/soft-fp/op-common.h b/soft-fp/op-common.h index 342532a..14fd6cd 100644 --- a/soft-fp/op-common.h +++ b/soft-fp/op-common.h @@ -1815,7 +1815,7 @@ X##_e = (_FP_EXPBIAS_##fs + 2 * _FP_W_TYPE_SIZE - 1 \ - _FP_FROM_INT_lz); \ }) \ - : (abort (), 0))); \ + : ({ abort (); 0; }))); \ \ if ((rsize) - 1 + _FP_EXPBIAS_##fs >= _FP_EXPMAX_##fs \ && X##_e >= _FP_EXPMAX_##fs) \