[PATCH] [PECOFF] Support a new type of weak symbol.

Rui Ueyama ruiu at google.com
Thu Mar 6 17:04:22 PST 2014


  Addressed Nick's comments.

Hi shankarke, Bigcheese, kledzik,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2996?vs=7616&id=7619#toc

BRANCH
  master

ARCANIST PROJECT
  lld

Files:
  include/lld/Core/DefinedAtom.h
  lib/Core/SymbolTable.cpp
  lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
  lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  test/pecoff/Inputs/merge-same-size1.obj.yaml
  test/pecoff/Inputs/merge-same-size2.obj.yaml
  test/pecoff/merge-same-size.test

Index: include/lld/Core/DefinedAtom.h
===================================================================
--- include/lld/Core/DefinedAtom.h
+++ include/lld/Core/DefinedAtom.h
@@ -93,12 +93,13 @@
   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
-                              // address was taken.
-    mergeByContent          // merge with other constants with same content
+    mergeAsWeakAndAddressUsed, // Is C++ definition inline definition whose
+                               // address was taken.
+    mergeByContent,         // Merge with other constants with same content
+    mergeSameSize,          // Another atom with different size is error
   };
 
   enum ContentType {
Index: lib/Core/SymbolTable.cpp
===================================================================
--- lib/Core/SymbolTable.cpp
+++ lib/Core/SymbolTable.cpp
@@ -101,32 +101,26 @@
   MCR_Error
 };
 
-static MergeResolution mergeCases[4][4] = {
-  // no        tentative     weak       weakAddressUsed
-  {
-    // first is no
-    MCR_Error,  MCR_First,   MCR_First, MCR_First
-  },
-  {
-    // 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 mergeCases[][5] = {
+  // no        tentative    weak        weakAddress sameSize
+  {MCR_Error,  MCR_First,   MCR_First,  MCR_First,  MCR_First},  // no
+  {MCR_Second, MCR_Largest, MCR_Second, MCR_Second, MCR_Second}, // tentative
+  {MCR_Second, MCR_First,   MCR_First,  MCR_Second, MCR_Second}, // weak
+  {MCR_Second, MCR_First,   MCR_First,  MCR_First,  MCR_First},  // weakAddress
+  {MCR_Second, MCR_Second,  MCR_Second, MCR_Second, MCR_First},  // sameSize
 };
 
-static MergeResolution mergeSelect(DefinedAtom::Merge first,
-                                   DefinedAtom::Merge second) {
-  return mergeCases[first][second];
+static MergeResolution mergeSelect(const DefinedAtom *first,
+                                   const DefinedAtom *second) {
+  if (first->merge() == DefinedAtom::mergeSameSize ||
+      second->merge() == DefinedAtom::mergeSameSize) {
+    if (first->size() != second->size())
+      return MCR_Error;
+  }
+  return mergeCases[first->merge()][second->merge()];
 }
 
-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);
@@ -148,8 +142,7 @@
   case NCR_DupDef:
     assert(existing->definition() == Atom::definitionRegular);
     assert(newAtom.definition() == Atom::definitionRegular);
-    switch (mergeSelect(((DefinedAtom*)existing)->merge(),
-                        ((DefinedAtom*)(&newAtom))->merge())) {
+    switch (mergeSelect((DefinedAtom*)existing, (DefinedAtom*)&newAtom)) {
     case MCR_First:
       useNew = false;
       break;
Index: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
===================================================================
--- lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
+++ lib/ReaderWriter/PECOFF/ReaderCOFF.cpp
@@ -249,8 +249,10 @@
     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::mergeSameSize;
   case llvm::COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE:
   case llvm::COFF::IMAGE_COMDAT_SELECT_LARGEST:
   case llvm::COFF::IMAGE_COMDAT_SELECT_NEWEST:
Index: lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
===================================================================
--- lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
+++ lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
@@ -337,7 +337,8 @@
     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-size",      lld::DefinedAtom::mergeSameSize);
   }
 };
 
Index: test/pecoff/Inputs/merge-same-size1.obj.yaml
===================================================================
--- /dev/null
+++ 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
+...
Index: test/pecoff/Inputs/merge-same-size2.obj.yaml
===================================================================
--- /dev/null
+++ 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
+...
Index: test/pecoff/merge-same-size.test
===================================================================
--- /dev/null
+++ test/pecoff/merge-same-size.test
@@ -0,0 +1,12 @@
+# 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 %s < %t.log
+
+CHECK-NOT: duplicate symbol error
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2996.2.patch
Type: text/x-patch
Size: 7516 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140306/9aa45570/attachment.bin>


More information about the llvm-commits mailing list