[dsymutil] Fix handling of common symbols.

llvm-dsymutil was misinterpreting the value of common symbols as their
address when it actually contains their size. This didn't impact
llvm-dsymutil's ability to link the debug information for common symbols
because these are always found by name and not by address. Things could
however go wrong when the size of a common object matched the object
file address of another symbol. Depending on the link order of the symbols
the common object might incorrectly evict this other object from the
address to symbol mapping, and then link the evicted symbol with a wrong
binary address.

Use the new ability to have symbols without an object file address to fix
this.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@259318 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/test/tools/dsymutil/Inputs/common.macho.x86_64 b/test/tools/dsymutil/Inputs/common.macho.x86_64
new file mode 100755 (executable)
index 0000000..c5c090e
Binary files /dev/null and b/test/tools/dsymutil/Inputs/common.macho.x86_64 differ

diff --git a/test/tools/dsymutil/Inputs/common.macho.x86_64.o b/test/tools/dsymutil/Inputs/common.macho.x86_64.o
new file mode 100644 (file)
index 0000000..491009b
Binary files /dev/null and b/test/tools/dsymutil/Inputs/common.macho.x86_64.o differ

diff --git a/test/tools/dsymutil/X86/common-sym.test b/test/tools/dsymutil/X86/common-sym.test
new file mode 100644 (file)
index 0000000..a3ba419
--- /dev/null
+++ b/test/tools/dsymutil/X86/common-sym.test
@@ -0,0 +1,24 @@
+RUN: llvm-dsymutil -oso-prepend-path %p/.. %p/../Inputs/common.macho.x86_64 -f -o - | llvm-dwarfdump -debug-dump=info - | Filecheck %s
+
+The test was compiled from a single source:
+$ cat common.c 
+char common[16];
+int main() { return 0;}
+void bar() {}
+$ clang -g -c common.c -o common.macho.x86_64.o
+$ clang common.macho.x86_64.o -o common.macho.x86_64
+
+CHECK:   DW_TAG_variable
+CHECK:     DW_AT_name {{.*}} "common"
+CHECK-NOT: {{NULL|DW_TAG}}
+CHECK:     DW_AT_location {{.*}} (<0x09> 03 00 10 00 00 01 00 00 00 )
+
+CHECK:   DW_TAG_subprogram
+CHECK-NEXT:     DW_AT_low_pc{{.*}}(0x0000000100000f80)
+CHECK-NOT: {{NULL|DW_TAG}}
+CHECK:     DW_AT_name {{.*}} "main"
+
+CHECK:   DW_TAG_subprogram
+CHECK-NEXT:     DW_AT_low_pc{{.*}}(0x0000000100000f90)
+CHECK-NOT: {{NULL|DW_TAG}}
+CHECK:     DW_AT_name {{.*}} "bar"

diff --git a/test/tools/dsymutil/absolute_symbol.test b/test/tools/dsymutil/absolute_symbol.test
index cdd6ae8..65eb0de 100644 (file)
--- a/test/tools/dsymutil/absolute_symbol.test
+++ b/test/tools/dsymutil/absolute_symbol.test
@@ -13,4 +13,5 @@ compiled for i386. This create an absolute symbol .objc_class_name_Foo
 We must not consider this symbol for debug info linking as its address
 might conflict with other real symbols in the same file.
 
-CHECK-NOT: objc_class_name_Foo
+CHECK: objc_class_name_Foo
+CHECK-SAME-NOT: objAddr

diff --git a/test/tools/dsymutil/basic-linking.test b/test/tools/dsymutil/basic-linking.test
index bff5b5d..5be9010 100644 (file)
--- a/test/tools/dsymutil/basic-linking.test
+++ b/test/tools/dsymutil/basic-linking.test
@@ -44,7 +44,7 @@ CHECK-NEXT: TAG_compile_unit
 CHECK-NOT: TAG
 CHECK: AT_name {{.*}}basic3.c
 CHECK-NOT: Found valid debug map entry
-CHECK: Found valid debug map entry: _val       0000000000000004 => 0000000100001004
+CHECK: Found valid debug map entry: _val       ffffffffffffffff => 0000000100001004
 CHECK-NEXT: DW_TAG_variable
 CHECK-NEXT:   DW_AT_name {{.*}}"val"
 CHECK-NOT: Found valid debug map entry
@@ -137,7 +137,7 @@ CHECK-ARCHIVE-NEXT: TAG_compile_unit
 CHECK-ARCHIVE-NOT: TAG
 CHECK-ARCHIVE: AT_name {{.*}}basic3.c
 CHECK-ARCHIVE-NOT: Found valid debug map entry
-CHECK-ARCHIVE: Found valid debug map entry: _val       0000000000000004 => 0000000100001008
+CHECK-ARCHIVE: Found valid debug map entry: _val       ffffffffffffffff => 0000000100001008
 CHECK-ARCHIVE-NEXT: DW_TAG_variable
 CHECK-ARCHIVE-NEXT:   DW_AT_name {{.*}}"val"
 CHECK-ARCHIVE-NOT: Found valid debug map entry

diff --git a/test/tools/dsymutil/debug-map-parsing.test b/test/tools/dsymutil/debug-map-parsing.test
index 2b9d091..05beb8e 100644 (file)
--- a/test/tools/dsymutil/debug-map-parsing.test
+++ b/test/tools/dsymutil/debug-map-parsing.test
@@ -21,7 +21,7 @@ CHECK-DAG:    sym: _private_int, objAddr: 0x0000000000000560, binAddr: 0x000000010
 CHECK: filename{{.*}}/Inputs/basic3.macho.x86_64.o
 CHECK-DAG:     sym: _bar, objAddr: 0x0000000000000020, binAddr: 0x0000000100000F40, size: 0x00000050
 CHECK-DAG:     sym: _inc, objAddr: 0x0000000000000070, binAddr: 0x0000000100000F90, size: 0x00000019
-CHECK-DAG:     sym: _val, objAddr: 0x0000000000000004, binAddr: 0x0000000100001004, size: 0x00000000
+CHECK-DAG:     sym: _val, binAddr: 0x0000000100001004, size: 0x00000000
 CHECK: ...
 
 
@@ -65,7 +65,7 @@ CHECK-ARCHIVE-DAG:    sym: _private_int, objAddr: 0x0000000000000560, binAddr: 0x0
 CHECK-ARCHIVE: /Inputs/./libbasic.a(basic3.macho.x86_64.o)
 CHECK-ARCHIVE-DAG:     sym: _bar, objAddr: 0x0000000000000020, binAddr: 0x0000000100000F40, size: 0x00000050
 CHECK-ARCHIVE-DAG:     sym: _inc, objAddr: 0x0000000000000070, binAddr: 0x0000000100000F90, size: 0x00000019
-CHECK-ARCHIVE-DAG:     sym: _val, objAddr: 0x0000000000000004, binAddr: 0x0000000100001008, size: 0x00000000
+CHECK-ARCHIVE-DAG:     sym: _val, binAddr: 0x0000000100001008, size: 0x00000000
 CHECK-ARCHIVE: ...
 
 Check that we warn about missing object files (this presumes that the files aren't

diff --git a/test/tools/dsymutil/yaml-object-address-rewrite.test b/test/tools/dsymutil/yaml-object-address-rewrite.test
index 749719f..a108d63 100644 (file)
--- a/test/tools/dsymutil/yaml-object-address-rewrite.test
+++ b/test/tools/dsymutil/yaml-object-address-rewrite.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT: filename:{{.*}}/Inputs/./libbasic.a(basic3.macho.x86_64.o)'
 # CHECK-NEXT: timestamp: 0
 # CHECK-NEXT: symbols:
-# CHECK-DAG:   sym: _val, objAddr: 0x0000000000000004, binAddr: 0x0000000100001008, size: 0x00000000
+# CHECK-DAG:   sym: _val, binAddr: 0x0000000100001008, size: 0x00000000
 # CHECK-DAG:   sym: _bar, objAddr: 0x0000000000000020, binAddr: 0x0000000100000F40, size: 0x00000050
 # CHECK-DAG:   sym: _inc, objAddr: 0x0000000000000070, binAddr: 0x0000000100000F90, size: 0x00000019
 # CHECK-NOT: { sym:
@@ -42,7 +42,7 @@ objects:
       - { sym: _baz, objAddr: 0x0, binAddr: 0x0000000100001000, size: 0x00000000 }
   - filename: /Inputs/./libbasic.a(basic3.macho.x86_64.o)
     symbols:
-      - { sym: _val, objAddr: 0x0, binAddr: 0x0000000100001008, size: 0x00000000 }
+      - { sym: _val, binAddr: 0x0000000100001008, size: 0x00000000 }
       - { sym: _bar, objAddr: 0x0, binAddr: 0x0000000100000F40, size: 0x00000050 }
       - { sym: _inc, objAddr: 0x0, binAddr: 0x0000000100000F90, size: 0x00000019 }
 ...

diff --git a/tools/dsymutil/DebugMap.cpp b/tools/dsymutil/DebugMap.cpp
index d2d5b61..114e22c 100644 (file)
--- a/tools/dsymutil/DebugMap.cpp
+++ b/tools/dsymutil/DebugMap.cpp
@@ -229,7 +229,8 @@ MappingTraits<dsymutil::DebugMapObject>::YamlDMO::denormalize(IO &IO) {
     for (const auto &Sym : ErrOrObjectFile->symbols()) {
       uint64_t Address = Sym.getValue();
       ErrorOr<StringRef> Name = Sym.getName();
-      if (!Name)
+      if (!Name ||
+          (Sym.getFlags() & (SymbolRef::SF_Absolute | SymbolRef::SF_Common)))
         continue;
       SymbolAddresses[*Name] = Address;
     }

diff --git a/tools/dsymutil/MachODebugMapParser.cpp b/tools/dsymutil/MachODebugMapParser.cpp
index 33845f4..02c3ab0 100644 (file)
--- a/tools/dsymutil/MachODebugMapParser.cpp
+++ b/tools/dsymutil/MachODebugMapParser.cpp
@@ -389,8 +389,6 @@ void MachODebugMapParser::handleStabSymbolTableEntry(uint32_t StringIndex,
   if (ObjectSymIt == CurrentObjectAddresses.end())
     return Warning("could not find object file symbol for symbol " +
                    Twine(Name));
-  if (!ObjectSymIt->getValue())
-    return;
   if (!CurrentDebugMapObject->addSymbol(Name, ObjectSymIt->getValue(), Value,
                                         Size))
     return Warning(Twine("failed to insert symbol '") + Name +
@@ -407,12 +405,15 @@ void MachODebugMapParser::loadCurrentObjectFileSymbols(
     ErrorOr<StringRef> Name = Sym.getName();
     if (!Name)
       continue;
-    // Objective-C on i386 uses artificial absolute symbols to
-    // perform some link time checks. Those symbols have a fixed 0
-    // address that might conflict with real symbols in the object
-    // file. As I cannot see a way for absolute symbols to find
-    // their way into the debug information, let's just ignore those.
-    if (Sym.getFlags() & SymbolRef::SF_Absolute)
+    // The value of some categories of symbols isn't meaningful. For
+    // example common symbols store their size in the value field, not
+    // their address. Absolute symbols have a fixed address that can
+    // conflict with standard symbols. These symbols (especially the
+    // common ones), might still be referenced by relocations. These
+    // relocations will use the symbol itself, and won't need an
+    // object file address. The object file address field is optional
+    // in the DebugMap, leave it unassigned for these symbols.
+    if (Sym.getFlags() & (SymbolRef::SF_Absolute | SymbolRef::SF_Common))
       CurrentObjectAddresses[*Name] = None;
     else
       CurrentObjectAddresses[*Name] = Addr;