[PATCH] D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, ruiu.
Herald added a project: LLVM.

Both MS link.exe and GNU ld.bfd handle it this way; one can have multiple object files defining the same absolute symbols, as long as it defines it to the same value. But if there are multiple absolute symbols with differing values, it is treated as an error.

As a general related note - LLD differs from link.exe (and GNU ld.bfd) when it comes to conflicts between a defined absolute regular and defined regular; LLD silently accepts these (in both orders), while both link.exe and ld.bfd error out on it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71981

Files:
  lld/COFF/SymbolTable.cpp
  lld/COFF/Symbols.h
  lld/test/COFF/duplicate-absolute-same.s
  lld/test/COFF/duplicate-absolute.s


Index: lld/test/COFF/duplicate-absolute.s
===================================================================
--- lld/test/COFF/duplicate-absolute.s
+++ lld/test/COFF/duplicate-absolute.s
@@ -1,6 +1,6 @@
 // REQUIRES: x86
 // RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t.obj %s
-// RUN: echo -e ".globl myabsolute\nmyabsolute = 0" > %t.dupl.s
+// RUN: echo -e ".globl myabsolute\nmyabsolute = 1" > %t.dupl.s
 // RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t.dupl.obj %t.dupl.s
 // RUN: not lld-link /out:%t.exe %t.obj %t.dupl.obj 2>&1 | FileCheck %s
 
Index: lld/test/COFF/duplicate-absolute-same.s
===================================================================
--- /dev/null
+++ lld/test/COFF/duplicate-absolute-same.s
@@ -0,0 +1,14 @@
+// REQUIRES: x86
+// RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t.obj %s
+// RUN: echo -e ".globl myabsolute\nmyabsolute = 0" > %t.dupl.s
+// RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t.dupl.obj %t.dupl.s
+// RUN: lld-link /out:%t.exe %t.obj %t.dupl.obj -subsystem:console -entry:entry 2>&1 | FileCheck --allow-empty %s
+
+// CHECK-NOT: error: duplicate symbol: myabsolute
+
+.globl myabsolute
+myabsolute = 0
+
+.globl entry
+entry:
+    ret
Index: lld/COFF/Symbols.h
===================================================================
--- lld/COFF/Symbols.h
+++ lld/COFF/Symbols.h
@@ -229,6 +229,14 @@
   uint64_t getRVA() { return va - config->imageBase; }
   void setVA(uint64_t v) { va = v; }
 
+  bool isEqual(COFFSymbolRef s) const {
+    return va == s.getValue();
+  }
+
+  bool isEqual(uint64_t otherVa) const {
+    return va == otherVa;
+  }
+
   // Section index relocations against absolute symbols resolve to
   // this 16 bit number, and it is the largest valid section index
   // plus one. This variable keeps it.
Index: lld/COFF/SymbolTable.cpp
===================================================================
--- lld/COFF/SymbolTable.cpp
+++ lld/COFF/SymbolTable.cpp
@@ -591,7 +591,10 @@
   s->isUsedInRegularObj = true;
   if (wasInserted || isa<Undefined>(s) || s->isLazy())
     replaceSymbol<DefinedAbsolute>(s, n, sym);
-  else if (!isa<DefinedCOFF>(s))
+  else if (auto *da = dyn_cast<DefinedAbsolute>(s)) {
+    if (!da->isEqual(sym))
+      reportDuplicate(s, nullptr);
+  } else if (!isa<DefinedCOFF>(s))
     reportDuplicate(s, nullptr);
   return s;
 }
@@ -603,7 +606,10 @@
   s->isUsedInRegularObj = true;
   if (wasInserted || isa<Undefined>(s) || s->isLazy())
     replaceSymbol<DefinedAbsolute>(s, n, va);
-  else if (!isa<DefinedCOFF>(s))
+  else if (auto *da = dyn_cast<DefinedAbsolute>(s)) {
+    if (!da->isEqual(va))
+      reportDuplicate(s, nullptr);
+  } else if (!isa<DefinedCOFF>(s))
     reportDuplicate(s, nullptr);
   return s;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71981.235524.patch
Type: text/x-patch
Size: 2789 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191229/a7376568/attachment.bin>