[PECOFF] Support a new type of weak symbol.

Summary:
COMDAT_SELECT_SAME_SIZE is a COMDAT type that I presume exist only in COFF.
The semantics of the type is that linker should merge such COMDAT sections if
their sizes are the same. Otherwise it's an error.

Reviewers: Bigcheese, shankarke, kledzik

CC: llvm-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D2996

llvm-svn: 203308

lld/include/lld/Core/DefinedAtom.h

@@ -93,12 +93,13 @@ public:
   enum Merge {
     mergeNo,                // Another atom with same name is error
     mergeAsTentative,       // Is ANSI C tentative definition, can be coalesced
-    mergeAsWeak,            // is C++ inline definition that was not inlined,
+    mergeAsWeak,            // Is C++ inline definition that was not inlined,
                             // but address was not taken, so atom can be hidden
                             // by linker
-    mergeAsWeakAndAddressUsed,// is C++ definition inline definition whose
+    mergeAsWeakAndAddressUsed, // Is C++ definition inline definition whose
-                              // address was taken.
+                               // address was taken.
-    mergeByContent          // merge with other constants with same content
+    mergeByContent,         // Merge with other constants with same content
+    mergeSameNameAndSize,   // Another atom with different size is error
   };
 
   enum ContentType {

lld/lib/Core/SymbolTable.cpp

@@ -98,27 +98,17 @@ enum MergeResolution {
   MCR_First,
   MCR_Second,
   MCR_Largest,
+  MCR_SameSize,
   MCR_Error
 };
 
-static MergeResolution mergeCases[4][4] = {
+static MergeResolution mergeCases[][5] = {
-  // no        tentative     weak       weakAddressUsed
+  // no          tentative      weak          weakAddress   sameNameAndSize
-  {
+  {MCR_Error,    MCR_First,     MCR_First,    MCR_First,    MCR_SameSize}, // no
-    // first is no
+  {MCR_Second,   MCR_Largest,   MCR_Second,   MCR_Second,   MCR_SameSize}, // tentative
-    MCR_Error,  MCR_First,   MCR_First, MCR_First
+  {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
-    // first is tentative
-    MCR_Second, MCR_Largest, MCR_Second, MCR_Second
-  },
-  {
-    // first is weak
-    MCR_Second, MCR_First,   MCR_First, MCR_Second
-  },
-  {
-    // first is weakAddressUsed
-    MCR_Second, MCR_First,   MCR_First, MCR_First
-  }
 };
 
 static MergeResolution mergeSelect(DefinedAtom::Merge first,
@@ -126,7 +116,7 @@ static MergeResolution mergeSelect(DefinedAtom::Merge first,
   return mergeCases[first][second];
 }
 
-void SymbolTable::addByName(const Atom & newAtom) {
+void SymbolTable::addByName(const Atom &newAtom) {
   StringRef name = newAtom.name();
   assert(!name.empty());
   const Atom *existing = this->findByName(name);
@@ -149,7 +139,7 @@ void SymbolTable::addByName(const Atom & newAtom) {
     assert(existing->definition() == Atom::definitionRegular);
     assert(newAtom.definition() == Atom::definitionRegular);
     switch (mergeSelect(((DefinedAtom*)existing)->merge(),
-                        ((DefinedAtom*)(&newAtom))->merge())) {
+                        ((DefinedAtom*)&newAtom)->merge())) {
     case MCR_First:
       useNew = false;
       break;
@@ -159,6 +149,18 @@ void SymbolTable::addByName(const Atom & newAtom) {
     case MCR_Largest:
       useNew = true;
       break;
+    case MCR_SameSize: {
+      uint64_t sa = ((DefinedAtom*)existing)->size();
+      uint64_t sb = ((DefinedAtom*)&newAtom)->size();
+      if (sa == sb) {
+        useNew = true;
+        break;
+      }
+      llvm::errs() << "Size mismatch: "
+                   << existing->name() << " (" << sa << ") "
+                   << newAtom.name() << " (" << sb << ")\n";
+      // fallthrough
+    }
     case MCR_Error:
       llvm::errs() << "Duplicate symbols: "
                    << existing->name()

lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp

@@ -249,8 +249,10 @@ DefinedAtom::Merge getMerge(const coff_aux_section_definition *auxsym) {
     return DefinedAtom::mergeNo;
   case llvm::COFF::IMAGE_COMDAT_SELECT_ANY:
     return DefinedAtom::mergeAsWeakAndAddressUsed;
-  case llvm::COFF::IMAGE_COMDAT_SELECT_SAME_SIZE:
   case llvm::COFF::IMAGE_COMDAT_SELECT_EXACT_MATCH:
+    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:
   case llvm::COFF::IMAGE_COMDAT_SELECT_NEWEST:

lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp

@@ -337,7 +337,9 @@ template <> struct ScalarEnumerationTraits<lld::DefinedAtom::Merge> {
     io.enumCase(value, "as-weak",      lld::DefinedAtom::mergeAsWeak);
     io.enumCase(value, "as-addressed-weak",
                                    lld::DefinedAtom::mergeAsWeakAndAddressUsed);
-    io.enumCase(value, "by-content",  lld::DefinedAtom::mergeByContent);
+    io.enumCase(value, "by-content",   lld::DefinedAtom::mergeByContent);
+    io.enumCase(value, "same-name-and-size",
+                lld::DefinedAtom::mergeSameNameAndSize);
   }
 };
 

lld/test/pecoff/Inputs/merge-same-size1.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:     FFFFFFFFFFFFFF
+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:   0700000000000000C979F796000003000000
+  - Name:            "_foo"
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

lld/test/pecoff/Inputs/merge-same-size2.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:     AAAAAAAAAAAAAA
+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:   0700000000000000C979F796000003000000
+  - Name:            "_foo"
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

lld/test/pecoff/merge-same-size.test

@@ -0,0 +1,24 @@
+# RUN: yaml2obj %p/Inputs/merge-same-size1.obj.yaml > %t1.obj
+# RUN: yaml2obj %p/Inputs/merge-same-size2.obj.yaml > %t2.obj
+#
+# RUN: lld -flavor link /out:%t.exe /subsystem:console /opt:noref /force \
+# RUN:   -- %t1.obj %t2.obj 2>&1 > %t.log
+#
+# FileCheck complains if the input files is empty, so add a dummy line.
+# RUN: echo foo >> %t.log
+# RUN: FileCheck -check-prefix=STDERR %s < %t.log
+#
+# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=READOBJ %s
+
+STDERR-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: 0x7
+READOBJ-NEXT:     VirtualAddress: 0x1000
+READOBJ-NEXT:     RawDataSize: 7