Message ID | 1405464182.4285.24.camel@otta |
---|---|
State | New |
Headers | show |
On Tue, Jul 15, 2014 at 6:43 PM, Peter Bergner <bergner@vnet.ibm.com> wrote: > With a recent libsanitizer merge from upstream, we're now seeing a lot > of ASAN test suite failures with the following error: > > ==26426==ASan runtime does not come first in initial library list; you should > either link runtime to your application or manually preload it with LD_PRELOAD. > > This is caused by powerpc*-linux not defining LIBASAN_EARLY_SPEC which is > defined in gnu-user.h. It looks like all *-linux architectures include > gnu-user.h except for powerpc*-linux. The following patch makes powerpc*-linux > match the other linux architectures... and fixes a compiler error when we > try to redefine CC1_SPEC. > > This passed bootstrap and regtesting on powerpc64-linux with no regressions. > Ok for mainline? > > Peter > > * config.gcc (powerpc*-*-linux*): Include gnu-user.h in tm_file. > * config/rs6000/sysv4.h (CC!_SPEC): Undefine it before defining it. Typo in ChangeLog (CC!)? This seems weird. Why wasn't this file included before or whenever it was added for other *-linux targets? This seems to define SPECs that should have been necessary before now. Thanks, David
On Wed, Jul 16, 2014 at 05:18:06AM -0400, David Edelsohn wrote: > > This passed bootstrap and regtesting on powerpc64-linux with no regressions. > > Ok for mainline? > > > > Peter > > > > * config.gcc (powerpc*-*-linux*): Include gnu-user.h in tm_file. > > * config/rs6000/sysv4.h (CC!_SPEC): Undefine it before defining it. > > Typo in ChangeLog (CC!)? > > This seems weird. Why wasn't this file included before or whenever it > was added for other *-linux targets? This seems to define SPECs that > should have been necessary before now. All other Linux targets where asan is supported got the right definitions from gnu-user.h, it was needed even on the older release branches. As including gnu-user.h there might be too risky for the release branches, perhaps it would be better to copy the LIB[AT]SAN* macros from gnu-user.h to say rs6000/linux.h or rs6000/linux64.h on the release branches (and in 4.8 also STATIC_LIB[AT]SAN_LIBS). Jakub
On Wed, 2014-07-16 at 11:23 +0200, Jakub Jelinek wrote: > On Wed, Jul 16, 2014 at 05:18:06AM -0400, David Edelsohn wrote: > > > This passed bootstrap and regtesting on powerpc64-linux with no regressions. > > > Ok for mainline? > > > > > > Peter > > > > > > * config.gcc (powerpc*-*-linux*): Include gnu-user.h in tm_file. > > > * config/rs6000/sysv4.h (CC!_SPEC): Undefine it before defining it. > > > > Typo in ChangeLog (CC!)? Woops, good catch. I'll fix that. > > This seems weird. Why wasn't this file included before or whenever it > > was added for other *-linux targets? This seems to define SPECs that > > should have been necessary before now. This was comitted by Joseph with revision 168711 and submitted here: https://gcc.gnu.org/ml/gcc-patches/2010-12/msg02055.html The patch seems to move some defines from linux.h to gnu-user.h and it looks like powerpc*-linux doesn't include linux.h either. Is that another header file we're supposed to include? ...or do our rs6000/linux{,64}.h header files completely obviate the need for that? > All other Linux targets where asan is supported got the right definitions > from gnu-user.h, it was needed even on the older release branches. > > As including gnu-user.h there might be too risky for the release branches, > perhaps it would be better to copy the LIB[AT]SAN* macros from gnu-user.h > to say rs6000/linux.h or rs6000/linux64.h on the release branches (and in 4.8 also > STATIC_LIB[AT]SAN_LIBS). That's fine with me. I'll make that change and bootstrap/regtest it. Peter
On Wed, 2014-07-16 at 11:23 +0200, Jakub Jelinek wrote: > On Wed, Jul 16, 2014 at 05:18:06AM -0400, David Edelsohn wrote: > > This seems weird. Why wasn't this file included before or whenever it > > was added for other *-linux targets? This seems to define SPECs that > > should have been necessary before now. > > All other Linux targets where asan is supported got the right definitions > from gnu-user.h, it was needed even on the older release branches. Ok, I compiled a source file that #includes tm.h and looked at the -dD -E output to see what macros get defined with and without this change. There are a few macros the gnu-user.h defines that are identical to what some of the rs6000/sysv4.h or rs6000/linux{,64}.h header files define, so we could remove those. There are some that are different too, so we'd need to keep those. The following are the macros that are identical: ... in rs6000/sysv4.h are: LINK_EH_SPEC NO_IMPLICIT_EXTERN_C However, there are some non-linux (eg, freebsd, lynx, etc.) that also include rs6000/sysv4.h and not gnu-user.h, so I think we need to keep this defines in rs6000/sysv4.h...or I could move them to freebsd{,64}.h, lynx.h, etc. Your preference is??? ...in rs6000/linux{,64}.h are: ASM_APP_ON ASM_APP_OFF CPLUSPLUS_CPP_SPEC LINK_GCC_C_SEQUENCE_SPEC USE_LD_AS_NEEDED TARGET_POSIX_IO These we should be able to freely remove from rs6000/linux{,64}.h since gnu-user.h provides the same identical definitions. There are also a few macros that gnu-user.h defines that are different than the what rs6000/*.h files define (eg, STARTFILE_SPEC, ENDFILE_SPEC, CC1_SPEC LIB_SPEC and TARGET_LIBC_HAS_FUNCTION), so we'll want to leave those macro definitions as well. Peter
On Wed, 16 Jul 2014, Peter Bergner wrote: > > > This seems weird. Why wasn't this file included before or whenever it > > > was added for other *-linux targets? This seems to define SPECs that > > > should have been necessary before now. > > This was comitted by Joseph with revision 168711 and submitted here: > > https://gcc.gnu.org/ml/gcc-patches/2010-12/msg02055.html > > The patch seems to move some defines from linux.h to gnu-user.h and > it looks like powerpc*-linux doesn't include linux.h either. > Is that another header file we're supposed to include? ...or do > our rs6000/linux{,64}.h header files completely obviate the need > for that? I believe the historical reason for not including the architecture-independent headers was the -mcall-* handling in rs6000/sysv4.h to allow a compiler for one target to build / link as if for another target for that architecture. That arrangement means all the main specs are defined via per-OS named specs, with rs6000/sysv4.h containing the definitions of the underlying specs for each OS. When specs come from the architecture-independent linux.h or gnu-user.h, other OS compilers with -mcall-linux will not get those specs and so will not properly link in the way a compiler configured for GNU/Linux target will. Enough configuration is in fact done in the OS-specific headers that I don't think the -mcall-* handling of specs is useful any more (or has been for a long time). (The compile-time handling of -mcall-* to select a particular ABI in SUBTARGET_OVERRIDE_OPTIONS may be more useful and seems unproblematic.) So as I said in the message you refer to (and elsewhere), I think it would be a useful cleanup (and would not remove any feature that is actually usable) to remove the -mcall-* specs handling and make powerpc* targets more like other architectures in this regard - each OS's headers define the specs for that OS without the added indirection. (I'm doubtful of the current use of -mads -myellowknife -mmvme as well - other architectures avoid having such special compiler options for each BSP.)
Index: gcc/config.gcc =================================================================== --- gcc/config.gcc (revision 212572) +++ gcc/config.gcc (working copy) @@ -2243,7 +2243,7 @@ powerpc-*-rtems*) tmake_file="${tmake_file} rs6000/t-fprules rs6000/t-rtems rs6000/t-ppccomm" ;; powerpc*-*-linux*) - tm_file="${tm_file} dbxelf.h elfos.h freebsd-spec.h rs6000/sysv4.h" + tm_file="${tm_file} dbxelf.h elfos.h gnu-user.h freebsd-spec.h rs6000/sysv4.h" extra_options="${extra_options} rs6000/sysv4.opt" tmake_file="rs6000/t-fprules rs6000/t-ppcos ${tmake_file} rs6000/t-ppccomm" extra_objs="$extra_objs rs6000-linux.o" Index: gcc/config/rs6000/sysv4.h =================================================================== --- gcc/config/rs6000/sysv4.h (revision 212572) +++ gcc/config/rs6000/sysv4.h (working copy) @@ -539,6 +539,7 @@ ENDIAN_SELECT(" -mbig", " -mlittle", DEF #endif /* Pass -G xxx to the compiler. */ +#undef CC1_SPEC #define CC1_SPEC "%{G*} %(cc1_cpu)" \ "%{meabi: %{!mcall-*: -mcall-sysv }} \ %{!meabi: %{!mno-eabi: \