This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: Define __start/__stop symbols when there is only a dynamic def
On Tue, Jan 30, 2018 at 6:14 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jan 30, 2018 at 6:03 AM, Michael Matz <matz@suse.de> wrote:
>> Hi,
>>
>> On Tue, 30 Jan 2018, Alan Modra wrote:
>>
>>> On Mon, Jan 29, 2018 at 03:36:05PM -0800, H.J. Lu wrote:
>>> > There is a reason why I have been keeping asking for a testcase.
>>> > I will update my patch with the above testcase.
>>>
>>> The last few emails in this thread illustrate just why changes based
>>> on testcases can be suspect.
>>
>> Only when it's the wrong testcase ;-)
>>
>>> I don't believe that ld 2.28 or earlier defined dynamic __start and
>>> __stop symbols without a dynamic reference.
>>
>> If you include dynamic defs (from linked shared libs) as dynamic refs,
>> then you're right. Old binutils did define dynamic __start in app when
>> there was a dynamic def in some linked shared lib.
>> I.e. the situation of the test in
>> https://sourceware.org/ml/binutils/2018-01/msg00432.html
>> That worked in ld 2.26 and is the situation pacemaker wants.
>>
>>> In fact, you need to use
>>> -E or some other way of making symbols dynamic in an executable to
>>> support dlsym on *normal* symbols. I'm going to commit the extra test
>>> I posted earlier without -E, plus this patch.
>>>
>>> * elflink.c (bfd_elf_define_start_stop): Make __start and __stop
>>> symbols dynamic.
>>>
>>> diff --git a/bfd/elflink.c b/bfd/elflink.c
>>> index e81f6c6..f1ec880 100644
>>> --- a/bfd/elflink.c
>>> +++ b/bfd/elflink.c
>>> @@ -14354,8 +14354,13 @@ bfd_elf_define_start_stop (struct bfd_link_info *info,
>>> bed = get_elf_backend_data (info->output_bfd);
>>> (*bed->elf_backend_hide_symbol) (info, h, TRUE);
>>> }
>>> - else if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
>>> - h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
>>> + else
>>> + {
>>> + if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
>>> + h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
>>> + if (h->ref_dynamic || h->def_dynamic)
>>> + bfd_elf_link_record_dynamic_symbol (info, h);
>>> + }
>>
>> This doesn't work as you seemingly intended (testcase above fails).
>> See the larger context:
The code looks odd, but works correctly since bfd_elf_define_start_stop
is called after all input objects have been seen. All def_dynamic have
been turned into ref_dynamic.
>> if (h != NULL
>> && (h->root.type == bfd_link_hash_undefined
>> ...
>> h->def_dynamic = 0;
>> ...
>> if (h->ref_dynamic || h->def_dynamic)
>> bfd_elf_link_record_dynamic_symbol (info, h);
>>
>> It does work after moving the overriding of def_dynamic downwards or
>> remembering the old status:
>>
>> diff --git a/bfd/elflink.c b/bfd/elflink.c
>> index f1ec880..3fe4555 100644
>> --- a/bfd/elflink.c
>> +++ b/bfd/elflink.c
>> @@ -14340,6 +14340,7 @@ bfd_elf_define_start_stop (struct bfd_link_info
>> *info,
>> || h->root.type == bfd_link_hash_undefweak
>> || ((h->ref_regular || h->def_dynamic) && !h->def_regular)))
We should check if __start and __stop symbols are referenced
by shared objects, not defined by shared objects.
>> {
>> + bfd_boolean was_dynamic = h->ref_dynamic || h->def_dynamic;
>> h->root.type = bfd_link_hash_defined;
>> h->root.u.def.section = sec;
>> h->root.u.def.value = 0;
>> @@ -14358,7 +14359,7 @@ bfd_elf_define_start_stop (struct bfd_link_info
>> *info,
>> {
>> if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
>> h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
>> - if (h->ref_dynamic || h->def_dynamic)
>> + if (was_dynamic)
>> bfd_elf_link_record_dynamic_symbol (info, h);
>> }
>> return &h->root;
>>
>
> Can you include your testcase with this patch?
>
Here is a patch to check ref_dynamic instead. I added a testcase to verify
that __start/__stop symbols aren't made dynamic when there is no reference
from shared objects.
OK for master?
--
H.J.
From e48a77ccfc59dd6c16d611a5c57bf4799bd9a97d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 29 Jan 2018 13:07:34 -0800
Subject: [PATCH] Check if __start/__stop symbols are referenced by shared
objects
Since bfd_elf_define_start_stop is called after all input objects have
been seen, we should check if __start and __stop symbols are referenced
by shared objects, not defined by shared objects.
bfd/
PR ld/21964
* elflink.c (bfd_elf_define_start_stop): Check if __start and
__stop symbols are referenced by shared objects.
ld/
PR ld/21964
* testsuite/ld-elf/pr21964-4.c: New file.
* testsuite/ld-elf/shared.exp: Run pr21964-4 test on Linux.
---
bfd/elflink.c | 4 ++--
ld/testsuite/ld-elf/pr21964-4.c | 23 +++++++++++++++++++++++
ld/testsuite/ld-elf/shared.exp | 12 ++++++++++++
3 files changed, 37 insertions(+), 2 deletions(-)
create mode 100644 ld/testsuite/ld-elf/pr21964-4.c
diff --git a/bfd/elflink.c b/bfd/elflink.c
index f1ec880f98..3787c85e49 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -14338,7 +14338,7 @@ bfd_elf_define_start_stop (struct bfd_link_info *info,
if (h != NULL
&& (h->root.type == bfd_link_hash_undefined
|| h->root.type == bfd_link_hash_undefweak
- || ((h->ref_regular || h->def_dynamic) && !h->def_regular)))
+ || ((h->ref_regular || h->ref_dynamic) && !h->def_regular)))
{
h->root.type = bfd_link_hash_defined;
h->root.u.def.section = sec;
@@ -14358,7 +14358,7 @@ bfd_elf_define_start_stop (struct bfd_link_info *info,
{
if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
- if (h->ref_dynamic || h->def_dynamic)
+ if (h->ref_dynamic)
bfd_elf_link_record_dynamic_symbol (info, h);
}
return &h->root;
diff --git a/ld/testsuite/ld-elf/pr21964-4.c b/ld/testsuite/ld-elf/pr21964-4.c
new file mode 100644
index 0000000000..4b19a9e7bc
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21964-4.c
@@ -0,0 +1,23 @@
+#define _GNU_SOURCE
+#include <dlfcn.h>
+#include <stdio.h>
+
+extern int __start___verbose[];
+extern int __stop___verbose[];
+int bar (void)
+{
+ static int my_var __attribute__ ((section("__verbose"), used)) = 6;
+ int *ptr;
+ ptr = (int*) dlsym (RTLD_DEFAULT, "__start___verbose");
+ if (ptr != NULL)
+ return -1;
+ return 0;
+}
+
+int
+main ()
+{
+ if (bar () == 0)
+ printf ("PASS\n");
+ return 0;
+}
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 8223d3df01..833a428055 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -1099,6 +1099,18 @@ if { [istarget *-*-linux*]
"pr22393-2-static" \
"pass.out" \
] \
+ [list \
+ "Run pr21964-4" \
+ "" \
+ "" \
+ {pr21964-4.c} \
+ "pr21964-4" \
+ "pass.out" \
+ "" \
+ "" \
+ "" \
+ "-ldl" \
+ ] \
]
}
--
2.14.3