int optopt; int main () { optopt = 4; return 0; } fails to link with GCC 5 on powerpc64{,le}-linux: gcc -O2 -o a a.c /usr/bin/ld: /tmp/cckV8RvX.o: In function `main': a.c:(.text.startup+0x6): unresolvable R_PPC64_TOC16_HA against `optopt@@GLIBC_2.3' /usr/bin/ld: a.c:(.text.startup+0xe): unresolvable R_PPC64_TOC16_LO against `optopt@@GLIBC_2.3' /usr/bin/ld: final link failed: Nonrepresentable section on output collect2: error: ld returned 1 exit status The problem seems to be that targetm.binds_local_p now returns true for !shlib DECL_COMMON with NULL (or error_mark_node) DECL_INITIAL. Looking at the difference between 4.9 and 5, there is: if (defined_locally && weak_dominate && !shlib) resolved_locally = true; and finally (similar to 4.9) /* Uninitialized COMMON variable may be unified with symbols resolved from other modules. */ if (DECL_COMMON (exp) && !resolved_locally && (DECL_INITIAL (exp) == NULL || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node))) return false; Looking at what x86_64-linux linker does in this case, assuming that an unitialized common binds locally in the executable if it is defined there works fine, but clearly not on ppc*. Either it is a linker bug there, or we for powerpc* (some other targets too) should treat the uninitialized DECL_COMMON defined_locally && weak_dominate && !shlib differently - not set resolved_locally in that case? Can people try other targets? The testcase relies on optopt being an integer variable defined in libc shared library, if that is not the case, create a shared library with that symbol and link it to the binary.
The testcase fails on mipsel-linux-gnu with: $ gcc-5 test.c /usr/bin/ld: non-dynamic relocations refer to dynamic symbol optopt@@GLIBC_2.0 /usr/bin/ld: failed to set dynamic section sizes: Bad value collect2: error: ld returned 1 exit status It works with 4.9.
Please provide the output of "readelf -sW a.o" to verify if optopt is COMMON: [hjl@gnu-6 gcc]$ ./xgcc -B./ -O2 -c /tmp/a.c [hjl@gnu-6 gcc]$ readelf -sW a.o Symbol table '.symtab' contains 12 entries: Num: Value Size Type Bind Vis Ndx Name 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND 1: 0000000000000000 0 FILE LOCAL DEFAULT ABS a.c 2: 0000000000000000 0 SECTION LOCAL DEFAULT 1 3: 0000000000000000 0 SECTION LOCAL DEFAULT 2 4: 0000000000000000 0 SECTION LOCAL DEFAULT 3 5: 0000000000000000 0 SECTION LOCAL DEFAULT 4 6: 0000000000000000 0 SECTION LOCAL DEFAULT 5 7: 0000000000000000 0 SECTION LOCAL DEFAULT 8 8: 0000000000000000 0 SECTION LOCAL DEFAULT 9 9: 0000000000000000 0 SECTION LOCAL DEFAULT 7 10: 0000000000000000 13 FUNC GLOBAL DEFAULT 5 main 11: 0000000000000004 4 OBJECT GLOBAL DEFAULT COM optopt [hjl@gnu-6 gcc]$
Also does -fno-common make a difference?
optopt is common, and from a linker perspective I believe the correct resolution of a common symbol in the executable and a strong definition in a shared library, is the shared library symbol. Therefore the change to binds_local_p is wrong. What's going on here is that HJ has made changes for x86 to always copy variables defined in a shared library into the executable's .dynbss if referenced by the executable. This is necessary to support non-PIC without text relocations, but isn't necessary for PIC. It is certainly possible to do this for PIC but I think only x86 does so.
I can reproduce it with binutils 2.24 on x86-64: [hjl@gnu-tools-1 gcc]$ ./xgcc -B./ -fPIE -pie /tmp/a.c /export/build/gnu/binutils/release/usr/local/bin/ld: /tmp/ccazj1RF.o: relocation R_X86_64_PC32 against undefined symbol `optopt@@GLIBC_2.2.5' can not be used when making a shared object; recompile with -fPIC /export/build/gnu/binutils/release/usr/local/bin/ld: final link failed: Bad value collect2: error: ld returned 1 exit status [hjl@gnu-tools-1 gcc]$
Created attachment 35325 [details] A patch Please try this.
Created attachment 35326 [details] An updated patch Copy reloc in PIE only works for x86-64. It isn't allowed for -m32.
Changing the summary back to as it was. -fPIE is not necessary to show this bug on powerpc64 (or mips by the look of comment #1).
Created attachment 35327 [details] A different patch On x86, this issue only shows up with PIE. Here is a different patch to treat common symbol defined locally only if the backend passes true common_maybe_local. For x86-64, it is true only if HAVE_LD_PIE_COPYRELOC is 1. For i386, it is always false. If we aren't building PIE, common_maybe_local is true or false doesn't make a difference for x86 since the common symbol is always referenced normally with copy reloc. For PIE on x86-64, common symbol is local only if HAVE_LD_PIE_COPYRELOC is 1.
(In reply to H.J. Lu from comment #9) > Created attachment 35327 [details] > A different patch > > On x86, this issue only shows up with PIE. Here is a different > patch to treat common symbol defined locally only if the backend > passes true common_maybe_local. For x86-64, it is true only if > HAVE_LD_PIE_COPYRELOC is 1. For i386, it is always false. If > we aren't building PIE, common_maybe_local is true or false > doesn't make a difference for x86 since the common symbol is > always referenced normally with copy reloc. For PIE on x86-64, > common symbol is local only if HAVE_LD_PIE_COPYRELOC is 1. + + /* For common symbol, it is defined locally only if common_maybe_local + is true. */ + bool defined_locally = (!DECL_EXTERNAL (exp) + && (!DECL_COMMON (exp) || common_maybe_local)); I think better would be: bool uninited_common = (DECL_COMMON (exp) && (DECL_INITIAL (exp) == NULL || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node))); /* For common symbol, it is defined locally only if common_maybe_local is true. */ bool defined_locally = (!DECL_EXTERNAL (exp) && (!uninited_common || common_maybe_local)); ... and then use /* Uninitialized COMMON variable may be unified with symbols resolved from other modules. */ if (uninited_common && !resolved_locally) return false;
And, shouldn't common_maybe_local for i?86/x86_64 be !flag_pic || (TARGET_64BIT && HAVE_LD_PIE_COPYRELOC != 0) ? What about other targets that are known to generate COPY relocations in this case for non-PIE executables? Should they pass !flag_pic? Perhaps there should be a generic default_binds_local_p* entry point that passes !flag_pic as common_maybe_local, that those targets could (after maintainers test it properly with various vintage linkers?) use as their TARGET_BINDS_LOCAL_P ? Clearly rs6000 should pass always false though, perhaps many others too.
I've tried the #c0 testcase with gcc 5.1 rc and binutils 2.25 on various linux architectures. On armv7hl, x86_64, s390 and s390x no errors are reported for both normal executable and PIE, for i686 PIE link fails, normal works, for powerpc64{,le} expectedly both normal and PIE link fail, aarch64 I'm still waiting for results.
(In reply to Jakub Jelinek from comment #11) > And, shouldn't common_maybe_local for i?86/x86_64 be > !flag_pic || (TARGET_64BIT && HAVE_LD_PIE_COPYRELOC != 0) > ? What about other targets that are known to generate COPY relocations in > this case for non-PIE executables? Should they pass !flag_pic? > Perhaps there should be a generic default_binds_local_p* entry point that > passes > !flag_pic as common_maybe_local, that those targets could (after maintainers > test it properly with various vintage linkers?) use as their > TARGET_BINDS_LOCAL_P ? Check flag_pic isn't necessary. For non-PIC, the same code sequence and relocation are used to access defined and undefined symbols, common or not.
(In reply to Jakub Jelinek from comment #10) > (In reply to H.J. Lu from comment #9) > > Created attachment 35327 [details] > > A different patch > > > > On x86, this issue only shows up with PIE. Here is a different > > patch to treat common symbol defined locally only if the backend > > passes true common_maybe_local. For x86-64, it is true only if > > HAVE_LD_PIE_COPYRELOC is 1. For i386, it is always false. If > > we aren't building PIE, common_maybe_local is true or false > > doesn't make a difference for x86 since the common symbol is > > always referenced normally with copy reloc. For PIE on x86-64, > > common symbol is local only if HAVE_LD_PIE_COPYRELOC is 1. > > + > + /* For common symbol, it is defined locally only if common_maybe_local > + is true. */ > + bool defined_locally = (!DECL_EXTERNAL (exp) > + && (!DECL_COMMON (exp) || common_maybe_local)); > > I think better would be: > bool uninited_common = (DECL_COMMON (exp) > && (DECL_INITIAL (exp) == NULL > || (!in_lto_p && DECL_INITIAL (exp) == > error_mark_node))); > /* For common symbol, it is defined locally only if common_maybe_local > is true. */ > bool defined_locally = (!DECL_EXTERNAL (exp) && (!uninited_common || > common_maybe_local)); > ... > and then use > /* Uninitialized COMMON variable may be unified with symbols > resolved from other modules. */ > if (uninited_common && !resolved_locally) > return false; What does initialized COMMON look like to linker? If it is marked as COMMON symbol to linker, it will be treated the same as uninitialized COMMON symbol.
(In reply to Jakub Jelinek from comment #10) > (In reply to H.J. Lu from comment #9) > > Created attachment 35327 [details] > > A different patch > > > > On x86, this issue only shows up with PIE. Here is a different > > patch to treat common symbol defined locally only if the backend > > passes true common_maybe_local. For x86-64, it is true only if > > HAVE_LD_PIE_COPYRELOC is 1. For i386, it is always false. If > > we aren't building PIE, common_maybe_local is true or false > > doesn't make a difference for x86 since the common symbol is > > always referenced normally with copy reloc. For PIE on x86-64, > > common symbol is local only if HAVE_LD_PIE_COPYRELOC is 1. > > + > + /* For common symbol, it is defined locally only if common_maybe_local > + is true. */ > + bool defined_locally = (!DECL_EXTERNAL (exp) > + && (!DECL_COMMON (exp) || common_maybe_local)); > > I think better would be: > bool uninited_common = (DECL_COMMON (exp) > && (DECL_INITIAL (exp) == NULL > || (!in_lto_p && DECL_INITIAL (exp) == > error_mark_node))); > /* For common symbol, it is defined locally only if common_maybe_local > is true. */ > bool defined_locally = (!DECL_EXTERNAL (exp) && (!uninited_common || > common_maybe_local)); > ... > and then use > /* Uninitialized COMMON variable may be unified with symbols > resolved from other modules. */ > if (uninited_common && !resolved_locally) > return false; Can we find a tectase with initialized COMMON variable and compile it as PIE?
(In reply to H.J. Lu from comment #13) > Check flag_pic isn't necessary. For non-PIC, the same code sequence > and relocation are used to access defined and undefined symbols, common > or not. What do you mean by is not necessary? Without that, you'll return false from targetm.binds_local_p for DECL_COMMON in the testcase say on i686-linux, or if you have old linker.
(In reply to H.J. Lu from comment #15) > Can we find a tectase with initialized COMMON variable and compile > it as PIE? I don't know where initialized DECL_COMMON could come from, but this spot certainly isn't the only one that is counting with that, e.g. when deciding what section to use comm_section is used only if it is bbs_initializer_p, etc. At least for GCC 5, I'd strongly prefer small provably correct changes at this point.
(In reply to Jakub Jelinek from comment #10) > (In reply to H.J. Lu from comment #9) > > Created attachment 35327 [details] > > A different patch > > > > On x86, this issue only shows up with PIE. Here is a different > > patch to treat common symbol defined locally only if the backend > > passes true common_maybe_local. For x86-64, it is true only if > > HAVE_LD_PIE_COPYRELOC is 1. For i386, it is always false. If > > we aren't building PIE, common_maybe_local is true or false > > doesn't make a difference for x86 since the common symbol is > > always referenced normally with copy reloc. For PIE on x86-64, > > common symbol is local only if HAVE_LD_PIE_COPYRELOC is 1. > > + > + /* For common symbol, it is defined locally only if common_maybe_local > + is true. */ > + bool defined_locally = (!DECL_EXTERNAL (exp) > + && (!DECL_COMMON (exp) || common_maybe_local)); > > I think better would be: > bool uninited_common = (DECL_COMMON (exp) > && (DECL_INITIAL (exp) == NULL > || (!in_lto_p && DECL_INITIAL (exp) == > error_mark_node))); > /* For common symbol, it is defined locally only if common_maybe_local > is true. */ > bool defined_locally = (!DECL_EXTERNAL (exp) && (!uninited_common || > common_maybe_local)); > ... > and then use > /* Uninitialized COMMON variable may be unified with symbols > resolved from other modules. */ > if (uninited_common && !resolved_locally) > return false; Here is a testcase: --- int optopt = 5; int optopt; int main () { optopt = 4; return 0; } ---- ~
Created attachment 35330 [details] A new patch
(In reply to Jakub Jelinek from comment #16) > (In reply to H.J. Lu from comment #13) > > Check flag_pic isn't necessary. For non-PIC, the same code sequence > > and relocation are used to access defined and undefined symbols, common > > or not. > > What do you mean by is not necessary? Without that, you'll return false > from targetm.binds_local_p for DECL_COMMON in the testcase say on > i686-linux, or if you have old linker. I guess it won't hurt.
I've repeated my test on the various architectures, this time with additional readelf -Ws test | grep optopt if the link succeeds. And indeed, x86_64 with recent linker is the only one where optopt is defined, rather than SHN_UNDEF in the PIE. armv7hl, s390, s390x, i686 and x86_64 with old linker all have optopt defined in the binary for normal executable (!flag_pic) and SHN_UNDEF for PIE. Thus, based on this I'd say that i386 backend should pass !flag_pic || (TARGET_64BIT && HAVE_LD_PIE_COPYRELOC != 0) to the new param (in ix86_binds_local_p). Then, perhaps you should make default_binds_local_p_3 also non-static and declared in output.h, ix86_binds_local_p should perhaps use it directly, and default_binds_local_p_2 should have just a single argument, so that arm and s390 backends (dunno, maybe aarch64 and a few others too) could use it directly as their TARGET_BINDS_LOCAL_P definition. default_binds_local_p_2 would then call default_binds_local_p_3 with exp, flag_shlib != 0, true, false, !flag_pic arguments. And obviously all the two (default_binds_local_p{,2}) should have better documentation on how they differ.
Created attachment 35331 [details] A patch I am testing this.
(In reply to H.J. Lu from comment #22) > Created attachment 35331 [details] > A patch > > I am testing this. +static bool +ix86_binds_local_p (const_tree exp) +{ + return default_binds_local_p_3 (exp, flag_shlib != 0, true, false, + (!flag_pic + || (TARGET_64BIT + && HAVE_LD_PIE_COPYRELOC != 0))); +} shouldn't the 4th argument be true? At least before this patch, i386 backend was the only one that passed true as extern_protected_data, but after this patch you never pass true to that parameter, making it dead again. Also, a typo: lcoal -> local
Created attachment 35332 [details] A patch
Comment on attachment 35332 [details] A patch An non-external shouldn't this be A non-external ? Other than that LGTM, but I'd prefer another pair of eyes on this.
Created attachment 35333 [details] A patch with updated comments
(In reply to H.J. Lu from comment #26) > Created attachment 35333 [details] > A patch with updated comments Found a couple of issues, here is incremental diff, mostly formatting improvements, and in the case of default_binds_local_p_2 (right now unused, hopefully incrementally used by arm, s390 and perhaps other backends later), passing true to common_maybe_local unconditionally, when only in non-PIE binaries (thus !flag_pic) it works fine. --- gcc/varasm.c +++ gcc/varasm.c @@ -6811,8 +6811,7 @@ bool default_binds_local_p_3 (const_tree exp, bool shlib, bool weak_dominate, - bool extern_protected_data, - bool common_maybe_local) + bool extern_protected_data, bool common_maybe_local) { /* A non-decl is an entry in the constant pool. */ if (!DECL_P (exp)) @@ -6902,8 +6901,7 @@ bool default_binds_local_p (const_tree exp) { - return default_binds_local_p_3 (exp, flag_shlib != 0, true, false, - false); + return default_binds_local_p_3 (exp, flag_shlib != 0, true, false, false); } /* Similar to default_binds_local_p, but common symbol may be local. */ @@ -6912,14 +6910,13 @@ default_binds_local_p_2 (const_tree exp) { return default_binds_local_p_3 (exp, flag_shlib != 0, true, false, - true); + !flag_pic); } bool default_binds_local_p_1 (const_tree exp, int shlib) { - return default_binds_local_p_3 (exp, shlib != 0, false, false, - false); + return default_binds_local_p_3 (exp, shlib != 0, false, false, false); } /* Return true when references to DECL must bind to current definition in
Created attachment 35335 [details] The final patch
Oh how I wish all this stuff was better documented, both in the GCC sources and at a higher level in some linkers/loaders documentation. ix86_binds_local_p needs a function comment. I think with a function comment, no comments would be needed in the function body. I think it's also advisable to get a function comment for the default_binds_local* that don't currently have one. What I'm trying to wrap my head around is what "defined_locally" really means. Is it a "must" or "maybe" property when it gets set in default_binds_local_p_3? If it's a must property, then "common_maybe_local" seems mis-named and/or mis-used (the former seems most likely to me). If it's a maybe property, then why is a common uninit local filtered out? At this point we don't know if a common uninit local will be defined locally or not, so it's a maybe. I can probably go forward with an approval or specific change recommendations that would lead to approval once someone can tell me if defined_locally is a maybe or must property.
Created attachment 35336 [details] The final patch
(In reply to Jeffrey A. Law from comment #29) > Oh how I wish all this stuff was better documented, both in the GCC sources > and at a higher level in some linkers/loaders documentation. > > ix86_binds_local_p needs a function comment. I think with a function > comment, no comments would be needed in the function body. Updated. > I think it's also advisable to get a function comment for the > default_binds_local* that don't currently have one. That can be done in a flow-up patch. > What I'm trying to wrap my head around is what "defined_locally" really > means. Is it a "must" or "maybe" property when it gets set in > default_binds_local_p_3? > > If it's a must property, then "common_maybe_local" seems mis-named and/or > mis-used (the former seems most likely to me). > > If it's a maybe property, then why is a common uninit local filtered out? > At this point we don't know if a common uninit local will be defined locally > or not, so it's a maybe. > > I can probably go forward with an approval or specific change > recommendations that would lead to approval once someone can tell me if > defined_locally is a maybe or must property. A common symbol in PIC will never be local, for any targets, unless it has hidden/internal visibility.
HJ, you're missing my point. I need to understand the meaning of the variable defined_locally to move forward with the patch. Is it a must property (ie, if true, the symbol is always defined locally), or is it a may property (ie, if set, the symbol may be defined locally, but might also be defined externally). Much of the correctness of the patch hinges on that question.
(In reply to Jeffrey A. Law from comment #32) > HJ, you're missing my point. I need to understand the meaning of the > variable defined_locally to move forward with the patch. Is it a must > property (ie, if true, the symbol is always defined locally), or is it a may > property (ie, if set, the symbol may be defined locally, but might also be > defined externally). > > Much of the correctness of the patch hinges on that question. There are 2 variables, defined_locally and resolved_locally. A symbol can be defined locally, but not resolved locally, depending on PIC and/or visibility.
(In reply to Jeffrey A. Law from comment #29) > What I'm trying to wrap my head around is what "defined_locally" really > means. Is it a "must" or "maybe" property when it gets set in > default_binds_local_p_3? I'd say it is a "must" property, the decl must be defined locally. A common var is of course a corner case, because uninitialized common var "maybe" defined locally if it doesn't have a strong definition elsewhere. And, for executables (normal and PIE), if the linker arranges for the definition to be present (through COPY relocation) in the executable, it "must" be defined locally too, even if it has a strong definition in a dependent shared library. > If it's a must property, then "common_maybe_local" seems mis-named and/or > mis-used (the former seems most likely to me). I'd say mis-named; so what about common_local_p, with a comment describing it - if the linker can guarantee that an uninited common symbol in the executable will still be defined (through COPY relocation) in the executable, then this flag should be set.
Created attachment 35341 [details] The final patch with variable name change and updated comments
Patch in c#35 is approved.
Author: hjl Date: Fri Apr 17 16:23:24 2015 New Revision: 222184 URL: https://gcc.gnu.org/viewcvs?rev=222184&root=gcc&view=rev Log: Properly handle uninitialized common symbol Uninitialized common symbol behavior in executables is target and linker dependent. default_binds_local_p_3 is made public and updated to take an argument to indicate if the linker can guarantee that an uninitialized common symbol in the executable will still be defined (through COPY relocation) in the executable. If common symbol is local to executable, default_binds_local_p_3 will treat non-external variable as defined locally. default_binds_local_p_2 is changed to treat common symbol as local for non-PIE binaries. For i386, common symbol is local only for non-PIE binaries. For x86-64, common symbol is local only for non-PIE binaries or linker supports copy reloc in PIE binaries. If a target treats common symbol as local only for non-PIE binaries, it can define TARGET_BINDS_LOCAL_P as default_binds_local_p_2. gcc/ PR target/65780 * output.h (default_binds_local_p_3): New. * varasm.c (default_binds_local_p_3): Make it public. Take an argument to indicate if common symbol may be local. If common symbol may be local, treat non-external variable as defined locally. (default_binds_local_p_2): Pass !flag_pic to default_binds_local_p_3. (default_binds_local_p_1): Pass false to default_binds_local_p_3. * config/i386/i386.c (ix86_binds_local_p): New. (TARGET_BINDS_LOCAL_P): Replace default_binds_local_p_2 with ix86_binds_local_p. gcc/testsuite/ PR target/65780 * gcc.dg/pr65780-1.c: New test. * gcc.dg/pr65780-2.c: Likewise. * gcc.target/i386/pr32219-9.c: Likewise. * gcc.target/i386/pr32219-1.c (xxx): Make it initialized common symbol. * gcc.target/i386/pr64317.c (c): Initialize. Added: trunk/gcc/testsuite/gcc.dg/pr65780-1.c trunk/gcc/testsuite/gcc.dg/pr65780-2.c trunk/gcc/testsuite/gcc.target/i386/pr32219-9.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.c trunk/gcc/output.h trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/i386/pr32219-1.c trunk/gcc/testsuite/gcc.target/i386/pr64317.c trunk/gcc/varasm.c
Fixed on trunk so far.
Please commit it to the branch too, I'll do another RC tomorrow.
Author: hjl Date: Fri Apr 17 16:36:22 2015 New Revision: 222185 URL: https://gcc.gnu.org/viewcvs?rev=222185&root=gcc&view=rev Log: Properly handle uninitialized common symbol Uninitialized common symbol behavior in executables is target and linker dependent. default_binds_local_p_3 is made public and updated to take an argument to indicate if the linker can guarantee that an uninitialized common symbol in the executable will still be defined (through COPY relocation) in the executable. If common symbol is local to executable, default_binds_local_p_3 will treat non-external variable as defined locally. default_binds_local_p_2 is changed to treat common symbol as local for non-PIE binaries. For i386, common symbol is local only for non-PIE binaries. For x86-64, common symbol is local only for non-PIE binaries or linker supports copy reloc in PIE binaries. If a target treats common symbol as local only for non-PIE binaries, it can define TARGET_BINDS_LOCAL_P as default_binds_local_p_2. gcc/ PR target/65780 * output.h (default_binds_local_p_3): New. * varasm.c (default_binds_local_p_3): Make it public. Take an argument to indicate if common symbol may be local. If common symbol may be local, treat non-external variable as defined locally. (default_binds_local_p_2): Pass !flag_pic to default_binds_local_p_3. (default_binds_local_p_1): Pass false to default_binds_local_p_3. * config/i386/i386.c (ix86_binds_local_p): New. (TARGET_BINDS_LOCAL_P): Replace default_binds_local_p_2 with ix86_binds_local_p. gcc/testsuite/ PR target/65780 * gcc.dg/pr65780-1.c: New test. * gcc.dg/pr65780-2.c: Likewise. * gcc.target/i386/pr32219-9.c: Likewise. * gcc.target/i386/pr32219-1.c (xxx): Make it initialized common symbol. * gcc.target/i386/pr64317.c (c): Initialize. Added: branches/gcc-5-branch/gcc/testsuite/gcc.dg/pr65780-1.c branches/gcc-5-branch/gcc/testsuite/gcc.dg/pr65780-2.c branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr32219-9.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/i386/i386.c branches/gcc-5-branch/gcc/output.h branches/gcc-5-branch/gcc/testsuite/ChangeLog branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr32219-1.c branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr64317.c branches/gcc-5-branch/gcc/varasm.c
Fixed for 5.1.
Revision r222184 breaks bootstrap on x86_64-apple-darwin14: /opt/gcc/build_w/./prev-gcc/xg++ -B/opt/gcc/build_w/./prev-gcc/ -B/opt/gcc/gcc6w/x86_64-apple-darwin14.3.0/bin/ -nostdinc++ -B/opt/gcc/build_w/prev-x86_64-apple-darwin14.3.0/libstdc++-v3/src/.libs -B/opt/gcc/build_w/prev-x86_64-apple-darwin14.3.0/libstdc++-v3/libsupc++/.libs -I/opt/gcc/build_w/prev-x86_64-apple-darwin14.3.0/libstdc++-v3/include/x86_64-apple-darwin14.3.0 -I/opt/gcc/build_w/prev-x86_64-apple-darwin14.3.0/libstdc++-v3/include -I/opt/gcc/work/libstdc++-v3/libsupc++ -L/opt/gcc/build_w/prev-x86_64-apple-darwin14.3.0/libstdc++-v3/src/.libs -L/opt/gcc/build_w/prev-x86_64-apple-darwin14.3.0/libstdc++-v3/libsupc++/.libs -c -DIN_GCC_FRONTEND -DIN_GCC_FRONTEND -g -O2 -gtoggle -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Icp -I../../work/gcc -I../../work/gcc/cp -I../../work/gcc/../include -I./../intl -I../../work/gcc/../libcpp/include -I/opt/mp-new/include -I../../work/gcc/../libdecnumber -I../../work/gcc/../libdecnumber/dpd -I../libdecnumber -I../../work/gcc/../libbacktrace -I/opt/mp-new/include -o cp/optimize.o -MT cp/optimize.o -MMD -MP -MF cp/.deps/optimize.TPo ../../work/gcc/cp/optimize.c ../../work/gcc/config/i386/i386.c:51747:1: error: 'bool ix86_binds_local_p(const_tree)' defined but not used [-Werror=unused-function] ix86_binds_local_p (const_tree exp) ^ cc1plus: all warnings being treated as errors Makefile:2071: recipe for target 'i386.o' failed make[3]: *** [i386.o] Error 1 Bootstrap is restored if I comment the block static bool ix86_binds_local_p (const_tree exp) { return default_binds_local_p_3 (exp, flag_shlib != 0, true, true, (!flag_pic || (TARGET_64BIT && HAVE_LD_PIE_COPYRELOC != 0))); }
Author: hjl Date: Fri Apr 17 21:54:22 2015 New Revision: 222201 URL: https://gcc.gnu.org/viewcvs?rev=222201&root=gcc&view=rev Log: Don't define ix86_binds_local_p for MacOS nor Windows PR target/65780 * config/i386/i386.c (ix86_binds_local_p): Define only if TARGET_MACHO and TARGET_DLLIMPORT_DECL_ATTRIBUTES are false. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.c
Author: hjl Date: Fri Apr 17 21:55:05 2015 New Revision: 222202 URL: https://gcc.gnu.org/viewcvs?rev=222202&root=gcc&view=rev Log: Don't define ix86_binds_local_p for MacOS nor Windows PR target/65780 * config/i386/i386.c (ix86_binds_local_p): Define only if TARGET_MACHO and TARGET_DLLIMPORT_DECL_ATTRIBUTES are false. Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/i386/i386.c
Is this considered fixed now for 5.1?
I think so.
The patch caused significant regressions (see below) on spec2000 INT benchmarks compiled with options “-fPIE -pie -O2 -ffast-math -mfpmath=sse -m32 -march=corei7-avx” or “-fPIE -pie -O2 -ffast-math -mfpmath=sse -m32 -march=slm”. Sandybridge: 164.gzip 1635.0000 1602.0000 -2.01% 197.parser 2159.0000 2117.0000 -1.94% 254.gap 2909.0000 2886.0000 -0.79% 256.bzip2 2232.0000 2154.0000 -3.49% Silvermont: 164.gzip 964.0000 945.0000 -1.97% 197.parser 951.0000 940.0000 -1.15% 254.gap 1328.0000 1296.0000 -2.40% 256.bzip2 952.0000 898.0000 -5.67%
(In reply to Stupachenko Evgeny from comment #47) > The patch caused significant regressions (see below) on spec2000 INT > benchmarks compiled with options “-fPIE -pie -O2 -ffast-math -mfpmath=sse > -m32 -march=corei7-avx” or “-fPIE -pie -O2 -ffast-math -mfpmath=sse -m32 > -march=slm”. > > Sandybridge: > 164.gzip 1635.0000 1602.0000 -2.01% > 197.parser 2159.0000 2117.0000 -1.94% > 254.gap 2909.0000 2886.0000 -0.79% > 256.bzip2 2232.0000 2154.0000 -3.49% > > Silvermont: > 164.gzip 964.0000 945.0000 -1.97% > 197.parser 951.0000 940.0000 -1.15% > 254.gap 1328.0000 1296.0000 -2.40% > 256.bzip2 952.0000 898.0000 -5.67% That is the price to pay for correctness. If you want to avoid that penalty, I'd say change ld.bfd as well as ld.gold on i?86 to use COPY relocations for PIEs like x86_64 has been changed some time ago, then when configured against improved linker and with small patch to gcc you can recover not just that cost, but some more on top of that.
(In reply to Stupachenko Evgeny from comment #47) > The patch caused significant regressions (see below) on spec2000 INT > benchmarks compiled with options “-fPIE -pie -O2 -ffast-math -mfpmath=sse > -m32 -march=corei7-avx” or “-fPIE -pie -O2 -ffast-math -mfpmath=sse -m32 > -march=slm”. > > Sandybridge: > 164.gzip 1635.0000 1602.0000 -2.01% > 197.parser 2159.0000 2117.0000 -1.94% > 254.gap 2909.0000 2886.0000 -0.79% > 256.bzip2 2232.0000 2154.0000 -3.49% > > Silvermont: > 164.gzip 964.0000 945.0000 -1.97% > 197.parser 951.0000 940.0000 -1.15% > 254.gap 1328.0000 1296.0000 -2.40% > 256.bzip2 952.0000 898.0000 -5.67% As Jakub pointed out, this commit is for correctness. Please open a new bug report to optimize undefined/common symbol access in PIE if linker supports copy reloc in PIE: https://sourceware.org/bugzilla/show_bug.cgi?id=18289
Author: jakub Date: Mon May 11 07:09:04 2015 New Revision: 222992 URL: https://gcc.gnu.org/viewcvs?rev=222992&root=gcc&view=rev Log: PR target/65780 * config/s390/linux.h (TARGET_BINDS_LOCAL_P): Define to default_binds_local_p_2. * config/arm/linux-elf.h (TARGET_BINDS_LOCAL_P): Likewise. * config/aarch64/aarch64-linux.h (TARGET_BINDS_LOCAL_P): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/config/aarch64/aarch64-linux.h trunk/gcc/config/arm/linux-elf.h trunk/gcc/config/s390/linux.h
Author: jakub Date: Mon May 11 07:14:10 2015 New Revision: 222993 URL: https://gcc.gnu.org/viewcvs?rev=222993&root=gcc&view=rev Log: PR target/65780 * config/s390/linux.h (TARGET_BINDS_LOCAL_P): Define to default_binds_local_p_2. * config/arm/linux-elf.h (TARGET_BINDS_LOCAL_P): Likewise. * config/aarch64/aarch64-linux.h (TARGET_BINDS_LOCAL_P): Likewise. Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/aarch64/aarch64-linux.h branches/gcc-5-branch/gcc/config/arm/linux-elf.h branches/gcc-5-branch/gcc/config/s390/linux.h