This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [BFD][PR21703]Override the new defined symbol with the old normal symbol when --allow-multiple-definition is provided


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?

Regards,
Renlin

On 11/10/17 22:24, H.J. Lu wrote:
On 10/11/17, Renlin Li <renlin.li@foss.arm.com> wrote:
Ping ~

Regards,
Renlin

On 12/07/17 15:21, Renlin Li wrote:
Hi,

On 12/07/17 15:17, Renlin Li wrote:
Hi H.J,

On 11/07/17 19:04, H.J. Lu wrote:
On Tue, Jul 11, 2017 at 10:31 AM, Renlin Li <renlin.li@foss.arm.com>
wrote:
ld/ChangeLog:

2017-07-11  Renlin Li  <renlin.li@arm.com>

          PR ld/21703
          * testsuite/ld-elf/elf.exp : Run new test case.
          * testsuite/ld-elf/pr21703-1.s: New.
          * testsuite/ld-elf/pr21703-2.s: New.
          * testsuite/ld-elf/pr21703.sd: New.

bfd/ChangeLog:

2017-07-11  Renlin Li  <renlin.li@arm.com>


          PR ld/21703
          * elflink.c (_bfd_elf_merge_symbol): Handle multiple symbol
definition
          case.



Please add some a test to cover versioned_sym in:

This is because _bfd_elf_merge_symbol will be called for versioned symbol
with the short
name.

For example, ld/testsuite/ld-elf/pr18735.s
It defines foo and an alias foo@FOO.
in addition to that, _bfd_elf_add_default_symbol will be called for
foo@FOO to create a
new symbol which short name (foo in this case). For the original
condition, this will
generate a multiple definition error.

"matched" flag seems introduced by you. Here I use it to differentiate
the versioned
symbol case from normal symbol definition.



    /* There are multiple definitions of a normal symbol.  */
+  if (olddef && !olddyn && !oldweak && newdef && !newdyn && !newweak
+      && !versioned_sym && h->def_regular)
+    {

What should happen if symbol is defined in linker script or command
line?
This is a good point. I missed this case.
I have read the code a little bit.

Multiple definition handling is broken for this case.

the following command throws error about multiple definitions.
ld --defsym foo=0x10 pr21703-1.o -o tmpdir/dump

The command links fine, generating foo symbol with *MERGED* information
from two
definition.
ld pr21703-1.o --defsym foo=0x10 -o tmpdir/dump
00000010     4 OBJECT  GLOBAL DEFAULT  ABS foo

The functions to process symbol definitions from object file and command
line is different.
Multiple-definition is not handled for command line symbol definition.
And the symbol
value is overwritten by the symbol absolute value. The definition section
is overwritten
as well to ABS section.

It's intended. Alan had a patch to allow multiple symbol definition
without provide
--allow-multiple-definition option.
https://sourceware.org/ml/binutils/2014-12/msg00247.html

The partial symbol information overwritten is the same problem as the
original bug I
reported.

So there is another question. What's the expected override behavior for
command line,
linker script and object/(static/dynamic)library defined symbols?

Is there a defined rule for those cases that I have missed?

Thanks!
Renlin.



With or without the patch, the behavior remains the same.
The symbol defined in command line option don't set h->def_regular flag.
h->ldscript_def flag is set instead.

Regards,
Renlin



I still like to see testcases with symbol versioning and
--allow-multiple-definition.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]