This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [BFD][PR21703]Override the new defined symbol with the old normal symbol when --allow-multiple-definition is provided
- From: Renlin Li <renlin dot li at foss dot arm dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Alan Modra <amodra at gmail dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 17 Oct 2017 14:54:12 +0100
- Subject: Re: [BFD][PR21703]Override the new defined symbol with the old normal symbol when --allow-multiple-definition is provided
- Authentication-results: sourceware.org; auth=none
- References: <595BF19A.6040000@foss.arm.com> <20170706054331.GD14520@bubble.grove.modra.org> <595FB9A0.9080508@foss.arm.com> <20170708021129.GL14520@bubble.grove.modra.org> <59650B69.3030806@foss.arm.com> <CAMe9rOoYeqs91xnwEj9tcDUZ_PUQGrmKtLb9kcO4gvp0R5t2_g@mail.gmail.com> <59662F7C.7030705@foss.arm.com> <59663072.8070900@foss.arm.com> <59DE41D9.1030505@foss.arm.com> <CAMe9rOogAHhA0oRTXaY9jq-CfXpLmEjPoCQ5W9vLHgFyT7dzsg@mail.gmail.com> <59DF6B01.7090004@foss.arm.com> <CAMe9rOprmJgwxd8LLn1RGpqw9_ER8fWcu0E-ybRPbSX5-4YmwA@mail.gmail.com>
Hi H.J,
Here is the updated patch. Two new test cases are added to test "-r" and "-shared" with
version symbol.
X86, arm, aarch64 are tested Okay with the change and test cases.
By the way, by this change, some warnings about changing symbol size and type are
eliminated. This is because the later defined symbol will be skipped instead of merging
with old definition.
Regards,
Renlin
bfd/ChangeLog:
2017-10-17 Renlin Li <renlin.li@arm.com>
* elflink.c (_bfd_elf_merge_symbol): Handle multiple definition case.
ld/ChangeLog:
2017-10-17 Renlin Li <renlin.li@arm.com>
* testsuite/ld-elf/elf.exp: Run new tests.
* testsuite/ld-elf/pr21703-1.s: New.
* testsuite/ld-elf/pr21703-2.s: New.
* testsuite/ld-elf/pr21703-3.s: New.
* testsuite/ld-elf/pr21703-4.s: New.
* testsuite/ld-elf/pr21703-r.sd: New.
* testsuite/ld-elf/pr21703-shared.sd: New.
* testsuite/ld-elf/pr21703.sd: New.
* testsuite/ld-elf/pr21703.ver: New.
On 12/10/17 21:52, H.J. Lu wrote:
On 10/12/17, Renlin Li <renlin.li@foss.arm.com> wrote:
Hi H.J.
According to the logic, if there are versioned symbols with the same symbol
name and
version, e.g. sym_name@VERSION. It will treated as multiple definition of
symbol.
Until now, the behavior is the same as before. The linker will throw
multiple definition
error.
With the change here, and with "--allow-multiple-definition" option
provided,
the first defined one will fully override a later definition.
For default version symbol, it's slightly different.
You can only declare one version of a symbol as the default in this manner;
otherwise you
would effectively have multiple definitions of the same symbol.
For sym_name@@VERSION, two symbols will be added. One is sym_name@@VERSION.
As above, you
cannot define sym_name@@VERSION twice.
what's more, _bfd_elf_add_default_symbol will add one "sym_name" indirect
symbol. The
check for this symbol definition is exclude explicitly in this patch as the
condition
indicates.
pr21703-3.s
.text
.global foo
.symver foo, foo@FOO
.type foo, %function
foo:
.space 4
.size foo, 4
pr21703-4.s
.text
.global bar
.symver bar, foo@FOO
.type bar, %function
bar:
.space 16
.size bar, 16
With the following command line:
as-new pr21703-3.o pr21703-3.s
as-new pr21703-4.o pr21703-4.s
ld-new -o pr21703 -z norelro pr21703-3.o pr21703-4.o
--allow-multiple-definition
without the patch,
The symbol table of final object is:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000400078 0 SECTION LOCAL DEFAULT 1
2: 0000000000600090 0 OBJECT LOCAL DEFAULT 1
_GLOBAL_OFFSET_TABLE_
3: 0000000000400078 16 FUNC GLOBAL DEFAULT 1 foo@FOO
4: 000000000060008c 0 NOTYPE GLOBAL DEFAULT 1 __bss_start
5: 000000000060008c 0 NOTYPE GLOBAL DEFAULT 1 _edata
6: 0000000000600090 0 NOTYPE GLOBAL DEFAULT 1 _end
7: 000000000040007c 16 FUNC GLOBAL DEFAULT 1 bar
foo@FOO is mapped to foo (the value) in pr21703-3.o with size from bar in
pr21703-4.o.
with the patch:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000400078 0 SECTION LOCAL DEFAULT 1
2: 0000000000600090 0 OBJECT LOCAL DEFAULT 1
_GLOBAL_OFFSET_TABLE_
3: 0000000000400078 4 FUNC GLOBAL DEFAULT 1 foo@FOO
4: 000000000060008c 0 NOTYPE GLOBAL DEFAULT 1 __bss_start
5: 000000000060008c 0 NOTYPE GLOBAL DEFAULT 1 _edata
6: 0000000000600090 0 NOTYPE GLOBAL DEFAULT 1 _end
7: 000000000040007c 16 FUNC GLOBAL DEFAULT 1 bar
foo@FOO is fully mapped to foo.
The above behavior is observed both on x64 and arm.
Is this the kind of test you are suggesting?
Yes, test both foo@FOO1 and foo@@OFOO2 with -r and -shared.
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 1a99058..9a60695 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1036,6 +1036,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
bfd_boolean newweak, oldweak, newfunc, oldfunc;
const struct elf_backend_data *bed;
char *new_version;
+ bfd_boolean default_sym = *matched;
*skip = FALSE;
*override = FALSE;
@@ -1557,6 +1558,18 @@ _bfd_elf_merge_symbol (bfd *abfd,
sec = *psec;
}
+ /* There are multiple definitions of a normal symbol.
+ Skip the default symbol as well. */
+ if (olddef && !olddyn && !oldweak && newdef && !newdyn && !newweak
+ && !default_sym && h->def_regular)
+ {
+ /* Handle a multiple definition. */
+ (*info->callbacks->multiple_definition) (info, &h->root,
+ abfd, sec, *pvalue);
+ *skip = TRUE;
+ return TRUE;
+ }
+
/* If both the old and the new symbols look like common symbols in a
dynamic object, set the size of the symbol to the larger of the
two. */
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 655f0da..eac29e0 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -70,6 +70,18 @@ run_ld_link_tests [list \
{symbol3w.s} {} "symbol3w.a" ] \
]
+run_ld_link_tests [list \
+ [list "PR ld/21703" \
+ "--allow-multiple-definition tmpdir/pr21703-1.o tmpdir/pr21703-2.o" "" "" \
+ {pr21703-1.s pr21703-2.s} {{readelf {-s} pr21703.sd}} "pr21703" ] \
+ [list "PR ld/21703 -r" \
+ "-r --allow-multiple-definition tmpdir/pr21703-3.o tmpdir/pr21703-4.o" "" "" \
+ {pr21703-3.s pr21703-4.s} {{readelf {-s} pr21703-r.sd}} "pr21703.o" ] \
+ [list "PR ld/21703 shared" \
+ "-shared --allow-multiple-definition --version-script pr21703.ver tmpdir/pr21703-3.o tmpdir/pr21703-4.o" "" "" \
+ {pr21703-3.s pr21703-4.s} {{readelf {--dyn-syms} pr21703-shared.sd}} "pr21703.so" ] \
+]
+
if { [check_shared_lib_support] } then {
run_ld_link_tests {
{"Build pr14170a.o" "" "" "" {pr14170a.s} {} "pr14170.a" }
diff --git a/ld/testsuite/ld-elf/pr21703-1.s b/ld/testsuite/ld-elf/pr21703-1.s
new file mode 100644
index 0000000..92a4718
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703-1.s
@@ -0,0 +1,6 @@
+ .text
+ .globl foo
+ .type foo, %function
+foo:
+ .space 4
+ .size foo, 4
diff --git a/ld/testsuite/ld-elf/pr21703-2.s b/ld/testsuite/ld-elf/pr21703-2.s
new file mode 100644
index 0000000..1d65304
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703-2.s
@@ -0,0 +1,6 @@
+ .text
+ .globl foo
+ .type foo, %function
+foo:
+ .space 16
+ .size foo, 16
diff --git a/ld/testsuite/ld-elf/pr21703-3.s b/ld/testsuite/ld-elf/pr21703-3.s
new file mode 100644
index 0000000..6da6de8
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703-3.s
@@ -0,0 +1,15 @@
+ .text
+ .global foo
+ .type foo, %function
+foo:
+ .space 4
+ .size foo, 4
+
+ .global foo1
+ .type foo1, %function
+foo1:
+ .space 32
+ .size foo1, 32
+
+ .symver foo, foo@FOO
+ .symver foo1, foo@@FOO1
diff --git a/ld/testsuite/ld-elf/pr21703-4.s b/ld/testsuite/ld-elf/pr21703-4.s
new file mode 100644
index 0000000..9390e94
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703-4.s
@@ -0,0 +1,15 @@
+ .text
+ .global bar
+ .type bar, %function
+bar:
+ .space 16
+ .size bar, 16
+
+ .global bar1
+ .type bar1, %function
+bar1:
+ .space 8
+ .size bar1, 8
+
+ .symver bar, foo@FOO
+ .symver bar1, foo@@FOO1
diff --git a/ld/testsuite/ld-elf/pr21703-r.sd b/ld/testsuite/ld-elf/pr21703-r.sd
new file mode 100644
index 0000000..6758088
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703-r.sd
@@ -0,0 +1,9 @@
+Symbol table '.symtab' contains .* entries:
+#...
+.*: [0-9a-fA-F]* +4 +FUNC +GLOBAL +DEFAULT +. foo@FOO
+.*: [0-9a-fA-F]* +32 +FUNC +GLOBAL +DEFAULT +. foo1
+.*: [0-9a-fA-F]* +32 +FUNC +GLOBAL +DEFAULT +. foo@@FOO1
+.*: [0-9a-fA-F]* +8 +FUNC +GLOBAL +DEFAULT +. bar1
+.*: [0-9a-fA-F]* +4 +FUNC +GLOBAL +DEFAULT +. foo
+.*: [0-9a-fA-F]* +16 +FUNC +GLOBAL +DEFAULT +. bar
+#pass
diff --git a/ld/testsuite/ld-elf/pr21703-shared.sd b/ld/testsuite/ld-elf/pr21703-shared.sd
new file mode 100644
index 0000000..9b6b1b9
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703-shared.sd
@@ -0,0 +1,8 @@
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+#...
+ +[0-9]+: +[0-9a-f]+ +4 +FUNC +GLOBAL +DEFAULT +[0-9] +foo@FOO
+ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +ABS +FOO1
+ +[0-9]+: +[0-9a-f]+ +32 +FUNC +GLOBAL +DEFAULT +[0-9] +foo@@FOO1
+ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +ABS +FOO
+#...
diff --git a/ld/testsuite/ld-elf/pr21703.sd b/ld/testsuite/ld-elf/pr21703.sd
new file mode 100644
index 0000000..955cf17
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703.sd
@@ -0,0 +1,4 @@
+Symbol table '.symtab' contains .* entries:
+#...
+.*: [0-9a-fA-F]* +4 +FUNC +GLOBAL +DEFAULT +. foo
+#pass
diff --git a/ld/testsuite/ld-elf/pr21703.ver b/ld/testsuite/ld-elf/pr21703.ver
new file mode 100644
index 0000000..c36f292
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr21703.ver
@@ -0,0 +1,4 @@
+FOO
+{ global: foo; local: *; };
+FOO1
+{ global: foo; local: *; };