* [PATCH 0/1] gold: Avoid duplicate PLT stub symbols on PowerPC
@ 2017-06-18 21:58 James Clarke
2017-06-18 21:58 ` [PATCH 1/1] " James Clarke
0 siblings, 1 reply; 7+ messages in thread
From: James Clarke @ 2017-06-18 21:58 UTC (permalink / raw)
To: binutils; +Cc: James Clarke
If two objects are compiled with -fPIC or -fPIE and call the same
function, two different PLT stubs are created, one for each object, but
the same stub symbol name is used for both. For example:
> (sid_powerpc-dchroot)jrtc27@redpanda:~$ cat test.c
> #include <stdio.h>
>
> extern void f(void);
>
> int main(int argc, char **argv) {
> printf("Main\n");
> f();
> return 0;
> }
> (sid_powerpc-dchroot)jrtc27@redpanda:~$ cat f.c
> #include <stdio.h>
>
> void f(void) {
> printf("f\n");
> }
> (sid_powerpc-dchroot)jrtc27@redpanda:~$ gcc -pie -fPIE -fuse-ld=gold -o test test.c f.c
> /usr/bin/ld.gold: error: invalid STB_LOCAL symbol in external symbols
> /usr/bin/ld.gold: error: linker defined: multiple definition of '00000001.plt_call.puts+8000'
> /usr/bin/ld.gold: command line: previous definition here
> collect2: error: ld returned 1 exit status
It seems BFD just ignores the issue, and will only add the symbol the
first time. I first wrote a patch to do that, but it struck me that this
might be nicer, as from what I can tell there is no guarantee that the
PLT stubs for the same symbol are next to each other so you cannot
easily tell the symbol for any of the subsequent PLT stubs. If you would
rather stick to mirroring BFD in this respect (even though the names
themselves are already different), however, I can submit that instead.
James Clarke (1):
gold: Avoid duplicate PLT stub symbols on PowerPC
gold/powerpc.cc | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
--
2.13.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] gold: Avoid duplicate PLT stub symbols on PowerPC 2017-06-18 21:58 [PATCH 0/1] gold: Avoid duplicate PLT stub symbols on PowerPC James Clarke @ 2017-06-18 21:58 ` James Clarke 2017-06-19 3:04 ` Alan Modra 0 siblings, 1 reply; 7+ messages in thread From: James Clarke @ 2017-06-18 21:58 UTC (permalink / raw) To: binutils; +Cc: James Clarke If two objects are compiled with -fPIC or -fPIE and call the same function, two different PLT entries are created, one for each object, but the same stub symbol name is used for both. Therefore let's make the names unique by incorporating the object's uniq_ value where necessary. gold/ * (Stub_table::define_stub_syms): Include object's uniq_ value when set for non-local symbols as well. --- gold/powerpc.cc | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/gold/powerpc.cc b/gold/powerpc.cc index 1f2bc9ede7..a448efbc94 100644 --- a/gold/powerpc.cc +++ b/gold/powerpc.cc @@ -4656,19 +4656,25 @@ Stub_table<size, big_endian>::define_stub_syms(Symbol_table* symtab) add[0] = 0; if (cs->first.addend_ != 0) sprintf(add, "+%x", static_cast<uint32_t>(cs->first.addend_)); - char localname[18]; - const char *symname; - if (cs->first.sym_ == NULL) + char obj[10]; + obj[0] = 0; + if (cs->first.object_) { const Powerpc_relobj<size, big_endian>* ppcobj = static_cast <const Powerpc_relobj<size, big_endian>*>(cs->first.object_); - sprintf(localname, "%x:%x", ppcobj->uniq(), cs->first.locsym_); + sprintf(obj, "%x:", ppcobj->uniq()); + } + char localname[9]; + const char *symname; + if (cs->first.sym_ == NULL) + { + sprintf(localname, "%x", cs->first.locsym_); symname = localname; } else symname = cs->first.sym_->name(); - char* name = new char[8 + 10 + strlen(symname) + strlen(add) + 1]; - sprintf(name, "%08x.plt_call.%s%s", this->uniq_, symname, add); + char* name = new char[8 + 10 + strlen(obj) + strlen(symname) + strlen(add) + 1]; + sprintf(name, "%08x.plt_call.%s%s%s", this->uniq_, obj, symname, add); Address value = this->stub_address() - this->address() + cs->second; unsigned int stub_size = this->plt_call_size(cs); this->targ_->define_local(symtab, name, this, value, stub_size); -- 2.13.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] gold: Avoid duplicate PLT stub symbols on PowerPC 2017-06-18 21:58 ` [PATCH 1/1] " James Clarke @ 2017-06-19 3:04 ` Alan Modra 2017-06-19 3:14 ` Alan Modra 0 siblings, 1 reply; 7+ messages in thread From: Alan Modra @ 2017-06-19 3:04 UTC (permalink / raw) To: James Clarke; +Cc: binutils On Sun, Jun 18, 2017 at 10:58:03PM +0100, James Clarke wrote: > If two objects are compiled with -fPIC or -fPIE and call the same > function, two different PLT entries are created, one for each object, I'd be surprised if that was so when using your simple testcase, and testing here verifies it. There is a situation where this can occur: In a very large binary it may be necessary to create multiple stubs to call a given function, because calls are so far apart that a single stub cannot be reached by a "bl" instruction. However, that case ought to be covered by this->uniq_ being different for each stub_table. So I'm not sure how you are getting the failure. > but the same stub symbol name is used for both. Therefore let's make > the names unique by incorporating the object's uniq_ value where > necessary. > > gold/ > * (Stub_table::define_stub_syms): Include object's uniq_ value > when set for non-local symbols as well. This won't work. object_ isn't set for ppc64 globals. See the Plt_stub_ent constructors. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] gold: Avoid duplicate PLT stub symbols on PowerPC 2017-06-19 3:04 ` Alan Modra @ 2017-06-19 3:14 ` Alan Modra 2017-06-19 11:08 ` James Clarke 0 siblings, 1 reply; 7+ messages in thread From: Alan Modra @ 2017-06-19 3:14 UTC (permalink / raw) To: James Clarke; +Cc: binutils On Mon, Jun 19, 2017 at 12:34:42PM +0930, Alan Modra wrote: > On Sun, Jun 18, 2017 at 10:58:03PM +0100, James Clarke wrote: > > If two objects are compiled with -fPIC or -fPIE and call the same > > function, two different PLT entries are created, one for each object, > > I'd be surprised if that was so when using your simple testcase, and > testing here verifies it. There is a situation where this can occur: > In a very large binary it may be necessary to create multiple stubs to > call a given function, because calls are so far apart that a single > stub cannot be reached by a "bl" instruction. However, that case > ought to be covered by this->uniq_ being different for each > stub_table. So I'm not sure how you are getting the failure. Incidentally, you can provoke multiple stubs with the small testcase by using a ridiculously small --stub-group-size=64 (and ignore all the warnings about section "exceeds group size". If I do that, I see nm printf | grep puts 0000000000000840 t 00000004.plt_call.puts 00000000000008a0 t 00000005.plt_call.puts U puts objdump also shows the two plt call stubs for puts. > > but the same stub symbol name is used for both. Therefore let's make > > the names unique by incorporating the object's uniq_ value where > > necessary. > > > > gold/ > > * (Stub_table::define_stub_syms): Include object's uniq_ value > > when set for non-local symbols as well. > > This won't work. object_ isn't set for ppc64 globals. See the > Plt_stub_ent constructors. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] gold: Avoid duplicate PLT stub symbols on PowerPC 2017-06-19 3:14 ` Alan Modra @ 2017-06-19 11:08 ` James Clarke 2017-06-20 0:29 ` Alan Modra 0 siblings, 1 reply; 7+ messages in thread From: James Clarke @ 2017-06-19 11:08 UTC (permalink / raw) To: Alan Modra; +Cc: binutils On 19 Jun 2017, at 04:13, Alan Modra <amodra@gmail.com> wrote: > On Mon, Jun 19, 2017 at 12:34:42PM +0930, Alan Modra wrote: >> On Sun, Jun 18, 2017 at 10:58:03PM +0100, James Clarke wrote: >>> If two objects are compiled with -fPIC or -fPIE and call the same >>> function, two different PLT entries are created, one for each object, >> >> I'd be surprised if that was so when using your simple testcase, and >> testing here verifies it. There is a situation where this can occur: >> In a very large binary it may be necessary to create multiple stubs to >> call a given function, because calls are so far apart that a single >> stub cannot be reached by a "bl" instruction. However, that case >> ought to be covered by this->uniq_ being different for each >> stub_table. So I'm not sure how you are getting the failure. > > Incidentally, you can provoke multiple stubs with the small testcase > by using a ridiculously small --stub-group-size=64 (and ignore all the > warnings about section "exceeds group size". If I do that, I see > > nm printf | grep puts > 0000000000000840 t 00000004.plt_call.puts > 00000000000008a0 t 00000005.plt_call.puts > U puts > > objdump also shows the two plt call stubs for puts. > >>> but the same stub symbol name is used for both. Therefore let's make >>> the names unique by incorporating the object's uniq_ value where >>> necessary. >>> >>> gold/ >>> * (Stub_table::define_stub_syms): Include object's uniq_ value >>> when set for non-local symbols as well. >> >> This won't work. object_ isn't set for ppc64 globals. See the >> Plt_stub_ent constructors. It's because, with -fPIC/-fPIE, one GOT is used per object file (.got2), so each object file needs its own PLT stubs referring to the right GOT, which is indicated by the addend being the magic 0x8000. You can see in one of the Plt_stub_ent constructors that the object_ field is non-NULL for globals only in this case. This means the same stub table ends up with multiple stub entries for a given global function. This is only an issue on 32-bit PowerPC as 64-bit PowerPC doesn't have this weirdness, and your multiple stub tables case *is* covered correctly as you saw yourself. Regards, James ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] gold: Avoid duplicate PLT stub symbols on PowerPC 2017-06-19 11:08 ` James Clarke @ 2017-06-20 0:29 ` Alan Modra 2017-06-20 5:12 ` James Clarke 0 siblings, 1 reply; 7+ messages in thread From: Alan Modra @ 2017-06-20 0:29 UTC (permalink / raw) To: James Clarke; +Cc: binutils On Mon, Jun 19, 2017 at 12:08:24PM +0100, James Clarke wrote: > This is only an issue on 32-bit PowerPC as 64-bit > PowerPC doesn't have this weirdness, and your multiple stub tables case *is* > covered correctly as you saw yourself. Oh.. I was thinking you'd said powerpc64, but the problem is on ppc32. The patch is good. Please apply. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] gold: Avoid duplicate PLT stub symbols on PowerPC 2017-06-20 0:29 ` Alan Modra @ 2017-06-20 5:12 ` James Clarke 0 siblings, 0 replies; 7+ messages in thread From: James Clarke @ 2017-06-20 5:12 UTC (permalink / raw) To: Alan Modra; +Cc: Binutils On 20 Jun 2017, at 01:28, Alan Modra <amodra@gmail.com> wrote: > On Mon, Jun 19, 2017 at 12:08:24PM +0100, James Clarke wrote: >> This is only an issue on 32-bit PowerPC as 64-bit >> PowerPC doesn't have this weirdness, and your multiple stub tables case *is* >> covered correctly as you saw yourself. > > Oh.. I was thinking you'd said powerpc64, but the problem is on > ppc32. The patch is good. Please apply. Ah, I never explicitly mentioned 32-bit in my original message, and given that the symbol stubs were added in the context of ppc64 I can see how this confusion arose. I don't have commit access; please apply on my behalf (including to the 2.28 branch). Thanks, James ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-20 5:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-18 21:58 [PATCH 0/1] gold: Avoid duplicate PLT stub symbols on PowerPC James Clarke 2017-06-18 21:58 ` [PATCH 1/1] " James Clarke 2017-06-19 3:04 ` Alan Modra 2017-06-19 3:14 ` Alan Modra 2017-06-19 11:08 ` James Clarke 2017-06-20 0:29 ` Alan Modra 2017-06-20 5:12 ` James Clarke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).