sparc: define _GLOBAL_OFFSET_TABLE_ when referenced
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
GCC testsuite gcc.dg/20050321-2.c hit link errors on undefined
_GLOBAL_OFFSET_TABLE_. The compiler output referenced only
_GLOBAL_OFFSET_TABLE_-offsets to set it up, and to compute the
GOT-relative address of local symbols, none of which triggered the
machinery that enabled the creation of the dynamic section, so
_GLOBAL_OFFSET_TABLE_ ended up undefined.
Enable the dynamic section if we find a relocation involving _G_O_T_
that we'd otherwise ignore.
Tested with --target=sparc-leon3-elf. Ok to install?
for bfd/ChangeLog
* elfxx-sparc.c (_bfd_sparc_elf_check_relocs): Create the
got section upon finding a _GLOBAL_OFFSET_TABLE_-referencing
relocation that we'd otherwise ignore.
for ld/ChangeLog
* testsuite/ld-sparc/got-def.s: New test.
* testsuite/ld-sparc/sparc.exp: Add it.
---
bfd/elfxx-sparc.c | 7 ++++++-
ld/testsuite/ld-sparc/got-def.s | 15 +++++++++++++++
ld/testsuite/ld-sparc/sparc.exp | 8 ++++++++
3 files changed, 29 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-sparc/got-def.s
Comments
On Fri, Jan 31, 2025 at 12:04:49AM -0300, Alexandre Oliva wrote:
>
> GCC testsuite gcc.dg/20050321-2.c hit link errors on undefined
> _GLOBAL_OFFSET_TABLE_. The compiler output referenced only
> _GLOBAL_OFFSET_TABLE_-offsets to set it up, and to compute the
> GOT-relative address of local symbols, none of which triggered the
> machinery that enabled the creation of the dynamic section, so
> _GLOBAL_OFFSET_TABLE_ ended up undefined.
>
> Enable the dynamic section if we find a relocation involving _G_O_T_
> that we'd otherwise ignore.
>
> Tested with --target=sparc-leon3-elf. Ok to install?
>
>
> for bfd/ChangeLog
>
> * elfxx-sparc.c (_bfd_sparc_elf_check_relocs): Create the
> got section upon finding a _GLOBAL_OFFSET_TABLE_-referencing
> relocation that we'd otherwise ignore.
>
> for ld/ChangeLog
>
> * testsuite/ld-sparc/got-def.s: New test.
> * testsuite/ld-sparc/sparc.exp: Add it.
> ---
> bfd/elfxx-sparc.c | 7 ++++++-
> ld/testsuite/ld-sparc/got-def.s | 15 +++++++++++++++
> ld/testsuite/ld-sparc/sparc.exp | 8 ++++++++
> 3 files changed, 29 insertions(+), 1 deletion(-)
> create mode 100644 ld/testsuite/ld-sparc/got-def.s
>
> diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
> index 71061621e8bfb..cec270e427acc 100644
> --- a/bfd/elfxx-sparc.c
> +++ b/bfd/elfxx-sparc.c
> @@ -1647,7 +1647,12 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>
> if (h != NULL
> && strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
> - break;
> + {
> + if (!htab->elf.sgot
> + && !_bfd_elf_create_got_section (htab->elf.dynobj, info))
> + return false;
> + break;
> + }
Your changelog says "create the got section upon finding a _G_O_T_
referencing relocation, but that isn't what this does. It only
creates the got section for 5 particular relocations. Does the ABI
disallow _G_O_T_ on other relocations? Also, what if htab->elf.dynobj
is NULL?
See elf32-ppc.c code handling this, and note that you will only need
one strcmp against _G_O_T_ in the entire file, the one creating the
got section on seeing a reference to _G_O_T_. The rest can compare
h == htab->elf.hgot.
> /* Fall through. */
>
> case R_SPARC_DISP8:
> diff --git a/ld/testsuite/ld-sparc/got-def.s b/ld/testsuite/ld-sparc/got-def.s
> new file mode 100644
> index 0000000000000..efca9109dfcb1
> --- /dev/null
> +++ b/ld/testsuite/ld-sparc/got-def.s
> @@ -0,0 +1,15 @@
> + .text
> +.LLGETPC0:
> + retl
> + add %o7, %l7, %l7
> + .global got
> + .type got, #function
> + .proc 04
> +got:
> + save %sp, -160, %sp
> + sethi %hi(_GLOBAL_OFFSET_TABLE_-4), %l7
> + call .LLGETPC0
> + add %l7, %lo(_GLOBAL_OFFSET_TABLE_+4), %l7
> + mov %l7, %o0
> + ret
> + restore
> diff --git a/ld/testsuite/ld-sparc/sparc.exp b/ld/testsuite/ld-sparc/sparc.exp
> index 9d684899ee2c2..3e21ed738c443 100644
> --- a/ld/testsuite/ld-sparc/sparc.exp
> +++ b/ld/testsuite/ld-sparc/sparc.exp
> @@ -94,6 +94,10 @@ set sparctests {
> {"32-bit: TLS -fpie" "-melf32_sparc -pie tmpdir/libtlslib32.so" ""
> "--32 -K PIC" {tlspie32.s}
> {{objdump -drj.text tlspie32.dd}} "tlspie32"}
> + {"32-bit: GOT definition"
> + "-melf32_sparc" ""
> + "--32 -K PIC" {got-def.s}
> + {} "got-def"}
> {"32-bit: GOTDATA relocations"
> "-shared -melf32_sparc --hash-style=sysv" ""
> "--32 -K PIC" {gotop32.s}
> @@ -134,6 +138,10 @@ set sparc64tests {
> "-melf64_sparc -pie -Ttext-segment=0x100000 tmpdir/libtlslib64.so" ""
> "--64 -Av9 -K PIC" {tlspie64.s}
> {{objdump -drj.text tlspie64.dd}} "tlspie64"}
> + {"64-bit: GOT definition"
> + "-melf64_sparc" ""
> + "--64 -Av9 -K PIC" {got-def.s}
> + {} "got-def"}
> {"64-bit: GOTDATA relocations"
> "-shared -melf64_sparc --hash-style=sysv" ""
> "--64 -Av9 -K PIC" {gotop64.s}
>
> --
> Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
> Free Software Activist GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Hello, Alan,
Thanks for your prompt response; sorry it took me so long to get back to
you.
On Jan 31, 2025, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Jan 31, 2025 at 12:04:49AM -0300, Alexandre Oliva wrote:
>> * elfxx-sparc.c (_bfd_sparc_elf_check_relocs): Create the
>> got section upon finding a _GLOBAL_OFFSET_TABLE_-referencing
>> relocation that we'd otherwise ignore.
> Your changelog says "create the got section upon finding a _G_O_T_
> referencing relocation, but that isn't what this does.
Erhm, it is, it's just not *any* _G_O_T_-referecing relocation; it's
only those "that we'd otherwise ignore".
> Does the ABI disallow _G_O_T_ on other relocations?
'fraid I don't know. I'd be happy to check, but I don't have a copy of
any of the SPARC ABIs handy, and I don't know how to get them. I tried
to search around, but all I could find was an incremental 64-bit ABI
extension (v1.33, dated Aug 1994, oddly marked confidential), that
didn't specify any changes relevant to the question at hand. Pointers,
anyone?
What I *do* know is that these are relocations used to set up the
_G_O_T_ register, and IMHO they *should* trigger the creation of the
symbol, because there's nothing wrong about setting it up and then
having the code that would use it optimized out, so that none of the
relocations that currently trigger the creation of the symbol are
present, as in the test that brought me here.
> Also, what if htab->elf.dynobj is NULL?
It would then be set to abfd at the top of the function.
> See elf32-ppc.c code handling this, and note that you will only need
> one strcmp against _G_O_T_ in the entire file, the one creating the
> got section on seeing a reference to _G_O_T_. The rest can compare
> h == htab->elf.hgot.
*nod*, thanks. I didn't feel like making changes that could affect
other target variants and that I wouldn't be able to test thoroughly
enough (as in, on targets and variants I'm not even aware of, and others
I know I can't test, such as sparc-wrs-vxworks), so I went for the more
conservative change I proposed.
Now, given your encouragement, here's a follow up change that implements
the improvement you suggest on top of the previous patch, if you'd take
it tested on sparc-leon3-elf only. I'd be happy to squash them into one
if that would be preferred.
sparc: optimize checking for _GLOBAL_OFFSET_TABLE_ refs
Compare hashed symbols rather than strings, after ensuring that
_GLOBAL_OFFSET_TABLE_ is defined if it's referenced.
for bfd/ChangeLog
* elfxx-sparc.c (_bfd_sparc_elf_check_relocs): Check for
_GLOBAL_OFFSET_TABLE_ references early, then compare hashed
symbols instead of strings.
(_bfd_sparc_elf_relocate_section): Compare hashed symbols.
---
bfd/elfxx-sparc.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index cec270e427acc..6e8ffdbd18b36 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -1426,6 +1426,16 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
h->plt.refcount += 1;
}
+ /* If a relocation refers to _GLOBAL_OFFSET_TABLE_, create the .got. */
+ if (h != NULL
+ && htab->elf.sgot == NULL
+ && strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
+ {
+ if (!_bfd_elf_create_got_section (htab->elf.dynobj, info))
+ return false;
+ BFD_ASSERT (h == htab->elf.hgot);
+ }
+
/* Compatibility with old R_SPARC_REV32 reloc conflicting
with R_SPARC_TLS_GD_HI22. */
if (! ABI_64_P (abfd) && ! checked_tlsgd)
@@ -1645,14 +1655,8 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
if (h != NULL)
h->non_got_ref = 1;
- if (h != NULL
- && strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
- {
- if (!htab->elf.sgot
- && !_bfd_elf_create_got_section (htab->elf.dynobj, info))
- return false;
- break;
- }
+ if (h != NULL && htab->elf.hgot)
+ break;
/* Fall through. */
case R_SPARC_DISP8:
@@ -3257,8 +3261,7 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
case R_SPARC_PC_HH22:
case R_SPARC_PC_HM10:
case R_SPARC_PC_LM22:
- if (h != NULL
- && strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
+ if (h != NULL && h == htab->elf.hgot)
break;
/* Fall through. */
case R_SPARC_DISP8:
On Thu, Feb 06, 2025 at 05:03:23AM -0300, Alexandre Oliva wrote:
> * elfxx-sparc.c (_bfd_sparc_elf_check_relocs): Check for
> _GLOBAL_OFFSET_TABLE_ references early, then compare hashed
> symbols instead of strings.
> (_bfd_sparc_elf_relocate_section): Compare hashed symbols.
Looks good, except for
> @@ -1645,14 +1655,8 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> if (h != NULL)
> h->non_got_ref = 1;
>
> - if (h != NULL
> - && strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
> - {
> - if (!htab->elf.sgot
> - && !_bfd_elf_create_got_section (htab->elf.dynobj, info))
> - return false;
> - break;
> - }
> + if (h != NULL && htab->elf.hgot)
if (h != NULL && h == htab->elf.hgot)
> + break;
> /* Fall through. */
>
> case R_SPARC_DISP8:
@@ -1647,7 +1647,12 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
if (h != NULL
&& strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
- break;
+ {
+ if (!htab->elf.sgot
+ && !_bfd_elf_create_got_section (htab->elf.dynobj, info))
+ return false;
+ break;
+ }
/* Fall through. */
case R_SPARC_DISP8:
new file mode 100644
@@ -0,0 +1,15 @@
+ .text
+.LLGETPC0:
+ retl
+ add %o7, %l7, %l7
+ .global got
+ .type got, #function
+ .proc 04
+got:
+ save %sp, -160, %sp
+ sethi %hi(_GLOBAL_OFFSET_TABLE_-4), %l7
+ call .LLGETPC0
+ add %l7, %lo(_GLOBAL_OFFSET_TABLE_+4), %l7
+ mov %l7, %o0
+ ret
+ restore
@@ -94,6 +94,10 @@ set sparctests {
{"32-bit: TLS -fpie" "-melf32_sparc -pie tmpdir/libtlslib32.so" ""
"--32 -K PIC" {tlspie32.s}
{{objdump -drj.text tlspie32.dd}} "tlspie32"}
+ {"32-bit: GOT definition"
+ "-melf32_sparc" ""
+ "--32 -K PIC" {got-def.s}
+ {} "got-def"}
{"32-bit: GOTDATA relocations"
"-shared -melf32_sparc --hash-style=sysv" ""
"--32 -K PIC" {gotop32.s}
@@ -134,6 +138,10 @@ set sparc64tests {
"-melf64_sparc -pie -Ttext-segment=0x100000 tmpdir/libtlslib64.so" ""
"--64 -Av9 -K PIC" {tlspie64.s}
{{objdump -drj.text tlspie64.dd}} "tlspie64"}
+ {"64-bit: GOT definition"
+ "-melf64_sparc" ""
+ "--64 -Av9 -K PIC" {got-def.s}
+ {} "got-def"}
{"64-bit: GOTDATA relocations"
"-shared -melf64_sparc --hash-style=sysv" ""
"--64 -Av9 -K PIC" {gotop64.s}