[PATCH] [PECOFF] Support yet another new type of weak symbol.

Rui Ueyama ruiu at google.com
Fri Mar 7 17:39:19 PST 2014


Hi Bigcheese, shankarke, kledzik,

COMDAT_SELECT_LARGEST is a COMDAT type that make linker to choose the largest
definition from among all of the definition of a symbol. If the size is the
same, the choice is arbitrary.

http://llvm-reviews.chandlerc.com/D3011

Files:
  include/lld/Core/DefinedAtom.h
  lib/Core/SymbolTable.cpp
  lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
  lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  test/pecoff/Inputs/merge-largest1.obj.yaml
  test/pecoff/Inputs/merge-largest2.obj.yaml
  test/pecoff/merge-largest.test

Index: include/lld/Core/DefinedAtom.h
===================================================================
--- include/lld/Core/DefinedAtom.h
+++ include/lld/Core/DefinedAtom.h
@@ -99,6 +99,8 @@
     mergeAsWeakAndAddressUsed, // Is C++ definition inline definition whose
                                // address was taken.
     mergeSameNameAndSize,   // Another atom with different size is error
+    mergeLargest,           // Choose the largest one from atoms with the same
+                            // name
     mergeByContent,         // Merge with other constants with same content.
   };
 
Index: lib/Core/SymbolTable.cpp
===================================================================
--- lib/Core/SymbolTable.cpp
+++ lib/Core/SymbolTable.cpp
@@ -102,13 +102,14 @@
   MCR_Error
 };
 
-static MergeResolution mergeCases[][5] = {
-  // no          tentative      weak          weakAddress   sameNameAndSize
-  {MCR_Error,    MCR_First,     MCR_First,    MCR_First,    MCR_SameSize}, // no
-  {MCR_Second,   MCR_Largest,   MCR_Second,   MCR_Second,   MCR_SameSize}, // tentative
-  {MCR_Second,   MCR_First,     MCR_First,    MCR_Second,   MCR_SameSize}, // weak
-  {MCR_Second,   MCR_First,     MCR_First,    MCR_First,    MCR_SameSize}, // weakAddress
-  {MCR_SameSize, MCR_SameSize,  MCR_SameSize, MCR_SameSize, MCR_SameSize}, // sameSize
+static MergeResolution mergeCases[][6] = {
+  // no          tentative      weak          weakAddress   sameNameAndSize largest
+  {MCR_Error,    MCR_First,     MCR_First,    MCR_First,    MCR_SameSize,   MCR_Largest},  // no
+  {MCR_Second,   MCR_Largest,   MCR_Second,   MCR_Second,   MCR_SameSize,   MCR_Largest},  // tentative
+  {MCR_Second,   MCR_First,     MCR_First,    MCR_Second,   MCR_SameSize,   MCR_Largest},  // weak
+  {MCR_Second,   MCR_First,     MCR_First,    MCR_First,    MCR_SameSize,   MCR_Largest},  // weakAddress
+  {MCR_SameSize, MCR_SameSize,  MCR_SameSize, MCR_SameSize, MCR_SameSize,   MCR_SameSize}, // sameSize
+  {MCR_Largest,  MCR_Largest,   MCR_Largest,  MCR_Largest,  MCR_SameSize,   MCR_Largest},  // largest
 };
 
 static MergeResolution mergeSelect(DefinedAtom::Merge first,
@@ -148,19 +149,22 @@
     case MCR_Second:
       useNew = true;
       break;
-    case MCR_Largest:
-      useNew = true;
+    case MCR_Largest: {
+      uint64_t existingSize = ((DefinedAtom*)existing)->size();
+      uint64_t newSize = ((DefinedAtom*)&newAtom)->size();
+      useNew = (newSize >= existingSize);
       break;
+    }
     case MCR_SameSize: {
-      uint64_t sa = ((DefinedAtom*)existing)->size();
-      uint64_t sb = ((DefinedAtom*)&newAtom)->size();
-      if (sa == sb) {
+      uint64_t existingSize = ((DefinedAtom*)existing)->size();
+      uint64_t newSize = ((DefinedAtom*)&newAtom)->size();
+      if (existingSize == newSize) {
         useNew = true;
         break;
       }
       llvm::errs() << "Size mismatch: "
-                   << existing->name() << " (" << sa << ") "
-                   << newAtom.name() << " (" << sb << ")\n";
+                   << existing->name() << " (" << existingSize << ") "
+                   << newAtom.name() << " (" << newSize << ")\n";
       // fallthrough
     }
     case MCR_Error:
Index: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
===================================================================
--- lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
+++ lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
@@ -253,8 +253,9 @@
     return DefinedAtom::mergeByContent;
   case llvm::COFF::IMAGE_COMDAT_SELECT_SAME_SIZE:
     return DefinedAtom::mergeSameNameAndSize;
-  case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:
   case llvm::COFF::IMAGE_COMDAT_SELECT_LARGEST:
+    return DefinedAtom::mergeLargest;
+  case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:
   case llvm::COFF::IMAGE_COMDAT_SELECT_NEWEST:
     // FIXME: These attributes has more complicated semantics than the regular
     // weak symbol. These are mapped to mergeAsWeakAndAddressUsed for now
Index: lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
===================================================================
--- lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
+++ lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
@@ -340,6 +340,7 @@
     io.enumCase(value, "by-content",   lld::DefinedAtom::mergeByContent);
     io.enumCase(value, "same-name-and-size",
                 lld::DefinedAtom::mergeSameNameAndSize);
+    io.enumCase(value, "largest", lld::DefinedAtom::mergeLargest);
   }
 };
 
Index: test/pecoff/Inputs/merge-largest1.obj.yaml
===================================================================
--- /dev/null
+++ test/pecoff/Inputs/merge-largest1.obj.yaml
@@ -0,0 +1,25 @@
+---
+header:
+  Machine:         IMAGE_FILE_MACHINE_I386
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     00112233
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    NumberOfAuxSymbols: 1
+    AuxiliaryData:   0700000000000000C979F796000006000000
+  - Name:            "_foo"
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...
Index: test/pecoff/Inputs/merge-largest2.obj.yaml
===================================================================
--- /dev/null
+++ test/pecoff/Inputs/merge-largest2.obj.yaml
@@ -0,0 +1,25 @@
+---
+header:
+  Machine:         IMAGE_FILE_MACHINE_I386
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     0011223344556677
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    NumberOfAuxSymbols: 1
+    AuxiliaryData:   0700000000000000C979F796000006000000
+  - Name:            "_foo"
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...
Index: test/pecoff/merge-largest.test
===================================================================
--- /dev/null
+++ test/pecoff/merge-largest.test
@@ -0,0 +1,22 @@
+# RUN: yaml2obj %p/Inputs/merge-largest1.obj.yaml > %t1.obj
+# RUN: yaml2obj %p/Inputs/merge-largest2.obj.yaml > %t2.obj
+#
+# RUN: lld -flavor link /out:%t.exe /subsystem:console /opt:noref /force \
+# RUN:   -- %t1.obj %t2.obj > %t.log 2>&1
+# RUN: echo foo >> %t.log
+# RUN: FileCheck -check-prefix=OK %s < %t.log
+#
+# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=READOBJ %s
+
+OK-NOT: duplicate symbol error
+
+READOBJ:      Format: COFF-i386
+READOBJ-NEXT: Arch: i386
+READOBJ-NEXT: AddressSize: 32bit
+READOBJ-NEXT: Sections [
+READOBJ-NEXT:   Section {
+READOBJ-NEXT:     Number: 1
+READOBJ-NEXT:     Name: .text (2E 74 65 78 74 00 00 00)
+READOBJ-NEXT:     VirtualSize: 0x8
+READOBJ-NEXT:     VirtualAddress: 0x1000
+READOBJ-NEXT:     RawDataSize: 8
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3011.1.patch
Type: text/x-patch
Size: 7444 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140307/22b22e47/attachment.bin>


More information about the llvm-commits mailing list