Bind defined symbol locally in PIE

[Jens Remus]

Hello H.J.,

On 7/31/2025 5:35 PM, H.J. Lu wrote:
> On Mon, Jul 28, 2025 at 9:29 AM Jens Remus <jremus@linux.ibm.com> wrote:
>> On 24.07.2025 16:08, H.J. Lu wrote:
>>> On Thu, Jul 24, 2025 at 4:04 AM Jens Remus <jremus@linux.ibm.com> wrote:
>>
>>>> I am porting the x86-64 change from your x86 commit 4e0c91e45402 ("Bind
>>>> defined symbol locally in PIE") to s390 for PR 33141.
>>>>
>>>> The change in elf_x86_64_relocate_section is clear to me.  The change
>>>
>>> This one checks if input relocation should be copied to output.
>>>
>>>> to elf_x86_64_check_relocs (now scan_relocs) is not.
>>>
>>> This one checks if dynamic relocation, which may be generated by linker, not
>>> copied from input, is needed.
>>
>> Ok.  As to my attempt to figure out what this change does, as the other
>> architectures did not port that change, by reverting that change on
>> master/4e0c91e45402 and not being able to experience any x86 test cases
> 
> Codes have been changed significantly on master.    The relevant codes
> on master are GENERATE_DYNAMIC_RELOCATION_P and COPY_INPUT_RELOC_P.
> Were you saying that changing them didn't cause any regressions on
> i386 and x86-64?

TL;DR:  For master I manually reverted the change in
NEED_DYNAMIC_RELOCATION_P.  See below for details.  Note that I will be
away from keyboard for a few weeks and thus won't be able to respond for
a while.


I naively tried the following on s390 64-bit (s390x):


On 4e0c91e45402 ("Bind defined symbol locally in PIE"):

$ git checkout 4e0c91e45402
$ mkdir build-x86
$ cd build-x86
$ ../configure --prefix=/tmp/binutils-x86 --target=x86_64-pc-linux-gnu --disable-gdb --disable-gdbserver --disable-gdbsupport --disable-readline --disable-werror
$ make -j $(nproc)
$ make check-ld
...
# of expected passes            843
# of expected failures          1
# of untested testcases         22
# of unsupported tests          33

Revert the change to check_relocs as follows:

$ cd ..
$ git apply --ignore-whitespace
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 1ebb61524d54..9ac08cc0a1d3 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1874,8 +1874,7 @@ do_size:
               && (sec->flags & SEC_ALLOC) != 0
               && (r_type != R_386_PC32
                   || (h != NULL
-                      && (! (bfd_link_pie (info)
-                             || SYMBOLIC_BIND (info, h))
+                      && (! SYMBOLIC_BIND (info, h)
                           || h->root.type == bfd_link_hash_defweak
                           || !h->def_regular))))
              || (ELIMINATE_COPY_RELOCS
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index dc7738a35645..e555157cebbf 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -2073,8 +2073,7 @@ do_size:
               && (sec->flags & SEC_ALLOC) != 0
               && (! IS_X86_64_PCREL_TYPE (r_type)
                   || (h != NULL
-                      && (! (bfd_link_pie (info)
-                             || SYMBOLIC_BIND (info, h))
+                      && (! SYMBOLIC_BIND (info, h)
                           || h->root.type == bfd_link_hash_defweak
                           || !h->def_regular))))
              || (ELIMINATE_COPY_RELOCS
<Ctrl+D>

$ cd build-x86
$ make -j $(nproc)
$ make check-ld
...
# of expected passes            843
# of expected failures          1
# of untested testcases         22
# of unsupported tests          33


On master:

$ git checkout master
$ mkdir build-x86
$ cd build-x86
$ ../configure --prefix=/tmp/binutils-x86 --target=x86_64-pc-linux-gnu --disable-gdb --disable-gdbserver --disable-gdbsupport
$ make -j $(nproc)
$ make check-ld
...
# of expected passes            1801
# of expected failures          4
# of untested testcases         26
# of unsupported tests          135

Revert the change to check_relocs (now scan_relocs) as follows:

$ cd ..
$ git apply --ignore-whitespace
diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h
index 88bfa05bce20..a859e43fb557 100644
--- a/bfd/elfxx-x86.h
+++ b/bfd/elfxx-x86.h
@@ -163,8 +163,7 @@
   ((bfd_link_pic (INFO) \
     && (! X86_PCREL_TYPE_P (IS_X86_64, R_TYPE) \
        || ((H) != NULL \
-           && (! (bfd_link_pie (INFO) \
-                  || SYMBOLIC_BIND ((INFO), (H))) \
+           && (! SYMBOLIC_BIND ((INFO), (H)) \
                || (H)->root.type == bfd_link_hash_defweak \
                || (!(bfd_link_pie (INFO) \
                      && (PCREL_PLT) \
<Ctrl+D>

$ cd build-x86
$ make -j $(nproc)
$ make check-ld
...
# of expected passes            1801
# of expected failures          4
# of untested testcases         26
# of unsupported tests          135

>> to fail:  Could it be that the change in the check never makes any
>> difference?  Or do you otherwise happen to have a suggestion for a test
>> case that would show how the change takes effect?
>>
>>> These 2 are related, but different.
>>>
>>>> Other architectures that supposedly ported your fix (see list of commits
>>>> in forwarded PR update below) did only change relocate_section.
Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/