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]

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


Hi all,

I am investigating some arm regressions on gcc trunk.

There are other issues around those LTO test cases.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78529

Apart from the gcc lto issues, I noticed a new issues which I believe is bfd linker's problem. so I opened this separate bugzilla ticket.

I create a simple test case to reproduce this. The same situation should apply to symbols defined in static library.
foo_thumb.c --> foo_thumb.o, thumb mode foo function definition.
foo_arm.c --> foo_arm.o, arm mode foo function definition
main_arm.c --> main_arm.o, references foo function.

arm-none-eabi-gcc main_arm.o foo_thumb.o foo_arm.o -specs=aprofile-validation.specs -o main.exe -Wl,--allow-multiple-definition

With this command line, thumb foo () implementation is in the main.exe while BL instruction is used to jump to the thumb function. With additional debug, I found the foo symbol definition in foo_arm.c is overriding the foo symbol definition in foo_thumb.c while the h.root.u.def.section is left unchanged, which means the function body is still the thumb version.
That's why, the symbol type is ARM, and the code references the thumb version.

I checked the documentation of --allow-multiple-definition option,
'''Normally when a symbol is defined multiple times, the linker will report a fatal error.
These options allow multiple definitions and the first definition will be used.'''

The behavior of _bfd_elf_merge_symbol and _bfd_generic_link_add_one_symbol is inconsistent.
In this case, _bfd_elf_merge_symbol decided to override the old symbol definition with the new defintion, (size, type, target data) In _bfd_generic_link_add_one_symbol, it simply return without doing anything because of allow-multiple-definition is provided.
This leaves the symbol in a wrong state.

Here, following the documentation, I made this patch to force the old definition override the new definition if the old symbol is not dynamic or weak. Because, in those two cases, it's expected to do some merge. I have checked that, those two cases are properly handled.

ld test checked Okay without any issues. Okay to commit?
I don't know if I missed any other cases, I would appreciate any suggestions.

bfd/changelog:

2017-07-03  renlin li  <renlin.li@arm.com>

	PR ld/21703
	* elflink.c (_bfd_elf_merge_symbol): Set override to TRUE when
	allow_multiple_definition is true and the old symbol is a normal symbol.
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 471e8ad..32d4d6a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1569,6 +1569,15 @@ _bfd_elf_merge_symbol (bfd *abfd,
       *size_change_ok = TRUE;
     }
 
+  /* The old symbol definition is overriding the new one if the symbol is a
+     normal symbol.  */
+  if (olddef && !olddyn && !oldweak
+      && newdef && info->allow_multiple_definition)
+    {
+      *override = TRUE;
+      return TRUE;
+    }
+
   /* If we are looking at a dynamic object, and we have found a
      definition, we need to see if the symbol was already defined by
      some other object.  If so, we want to use the existing

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