Add more merging/coalescing test cases and make core linking work for them

llvm-svn: 147130

lld/include/lld/Core/Atom.h

@@ -32,7 +32,7 @@ class File;
 ///
 /// Here are some example attribute sets for common atoms. If a particular
 /// attribute is not listed, the default values are:  definition=regular,
-/// sectionChoice=basedOnContent, scope=translationUnit, mergeDups=false, 
+/// sectionChoice=basedOnContent, scope=translationUnit, mergeDupes=false, 
 /// autoHide=false, internalName=false, deadStrip=normal
 ///
 ///  C function:  void foo() {} <br>
@@ -61,23 +61,23 @@ class File;
 ///
 ///  Non-inlined C++ inline method:  inline void Foo::doit() {} <br>
 ///    name=_ZN3Foo4doitEv, type=code, perm=r_x, scope=global, 
-///    mergeDups=true, autoHide=true
+///    mergeDupes=true, autoHide=true
 ///
 ///  Non-inlined C++ inline method whose address is taken:  
 ///     inline void Foo::doit() {} <br>
-///    name=_ZN3Foo4doitEv, type=code, perm=r_x, scope=global, mergeDups=true
+///    name=_ZN3Foo4doitEv, type=code, perm=r_x, scope=global, mergeDupes=true
 ///
 ///  literal c-string:  "hello" <br>
 ///    name=L0, internalName=true, type=cstring, perm=r__, 
-///    scope=linkageUnit, mergeDups=true
+///    scope=linkageUnit, mergeDupes=true
 ///
 ///  literal double:  1.234 <br>
 ///    name=L0, internalName=true, type=literal8, perm=r__, 
-///    scope=linkageUnit, mergeDups=true
+///    scope=linkageUnit, mergeDupes=true
 ///
 ///  constant:  { 1,2,3 } <br>
 ///    name=L0, internalName=true, type=constant, perm=r__, 
-///    scope=linkageUnit, mergeDups=true
+///    scope=linkageUnit, mergeDupes=true
 ///
 ///  Pointer to initializer function:  <br>
 ///    name=_init, internalName=true, type=initializer, perm=rw_l,
@@ -336,6 +336,7 @@ public:
       , ContentType ct
       , SectionChoice sc
       , bool internalName
+      , bool md
       , DeadStripKind ds
       , bool IsThumb
       , bool IsAlias
@@ -347,6 +348,7 @@ public:
     , _internalName(internalName)
     , _deadStrip(ds)
     , _mode(modeOrdinal)
+    , _mergeDuplicates(md)
     , _thumb(IsThumb)
     , _alias(IsAlias)
     , _contentType(ct)

lld/include/lld/Core/Resolver.h

@@ -33,6 +33,7 @@ public:
   Resolver(Platform &plat, const InputFiles &inputs)
     : _platform(plat)
     , _inputFiles(inputs)
+    , _symbolTable(plat)
     , _haveLLVMObjs(false)
     , _addToFinalSection(false)
     , _completedInitialObjectFiles(false) {}

lld/include/lld/Core/SymbolTable.h

@@ -19,6 +19,7 @@ namespace llvm { class StringRef; }
 namespace lld {
 
 class Atom;
+class Platform;
 
 /// The SymbolTable class is responsible for coalescing atoms.
 ///
@@ -27,6 +28,8 @@ class Atom;
 /// if an atom has been coalesced away.
 class SymbolTable {
 public:
+      SymbolTable(Platform& plat);
+
   /// @brief add atom to symbol table
   void add(const Atom &);
 
@@ -52,6 +55,7 @@ private:
 
   void addByName(const Atom &);
 
+  Platform&  _platform;
   AtomToAtom _replacedAtoms;
   NameToAtom _nameTable;
 };

lld/include/lld/Core/UndefinedAtom.h

@@ -27,6 +27,7 @@ public:
           , Atom::typeUnknown
           , Atom::sectionBasedOnContent
           , false
+          , false
           , deadStripNormal
           , false
           , false

lld/include/lld/Core/YamlWriter.h

@@ -17,7 +17,7 @@
 namespace lld {
 namespace yaml {
 
-void writeObjectText(File *, llvm::raw_ostream &);
+void writeObjectText(lld::File &, llvm::raw_ostream &);
 
 } // namespace yaml
 } // namespace lld

lld/include/lld/Platform/Platform.h

@@ -67,6 +67,12 @@ public:
   /// @brief for debugging dead code stripping, -why_live
   virtual bool printWhyLive(llvm::StringRef name) = 0;
 
+  /// When core linking finds a duplicate definition, the platform
+  /// can either print an error message and terminate or return with
+  /// which atom the linker should use.
+  virtual const Atom& handleMultipleDefinitions(const Atom& def1, 
+                                                const Atom& def2) = 0;
+
   /// @brief print out undefined symbol error messages in platform specific way
   virtual void errorWithUndefines(const std::vector<const Atom *>& undefs,
                                   const std::vector<const Atom *>& all) = 0;

lld/lib/Core/SymbolTable.cpp

@@ -24,6 +24,10 @@
 
 namespace lld {
 
+SymbolTable::SymbolTable(Platform& plat)
+  : _platform(plat) {
+}
+
 void SymbolTable::add(const Atom &atom) {
   assert(atom.scope() != Atom::scopeTranslationUnit);
   if ( !atom.internalName() ) {
@@ -39,6 +43,7 @@ enum NameCollisionResolution {
   NCR_Second,
   NCR_Weak,
   NCR_Larger,
+  NCR_Dup,
   NCR_Error
 };
 
@@ -46,7 +51,7 @@ static NameCollisionResolution cases[6][6] = {
   //regular     weak         tentative   absolute    undef      sharedLib
   {
     // first is regular
-    NCR_Error,  NCR_First,   NCR_First,  NCR_Error,  NCR_First, NCR_First
+    NCR_Dup,    NCR_First,   NCR_First,  NCR_Error,  NCR_First, NCR_First
   },
   {
     // first is weak
@@ -75,26 +80,44 @@ static NameCollisionResolution collide(Atom::Definition first,
   return cases[first][second];
 }
 
-void SymbolTable::addByName(const Atom &atom) {
-  llvm::StringRef name = atom.name();
+void SymbolTable::addByName(const Atom & newAtom) {
+  llvm::StringRef name = newAtom.name();
   const Atom *existing = this->findByName(name);
   if (existing == NULL) {
     // name is not in symbol table yet, add it associate with this atom
-    _nameTable[name] = &atom;
+    _nameTable[name] = &newAtom;
   } else {
     // name is already in symbol table and associated with another atom
-    switch (collide(existing->definition(), atom.definition())) {
+    switch (collide(existing->definition(), newAtom.definition())) {
     case NCR_First:
       // using first, just add new to _replacedAtoms
-      _replacedAtoms[&atom] = existing;
+      _replacedAtoms[&newAtom] = existing;
       break;
     case NCR_Second:
       // using second, update tables
-      _nameTable[name] = &atom;
-      _replacedAtoms[existing] = &atom;
+      _nameTable[name] = &newAtom;
+      _replacedAtoms[existing] = &newAtom;
+      break;
+    case NCR_Dup:
+      if ( existing->mergeDuplicates() && newAtom.mergeDuplicates() ) {
+          // using existing atom, add new atom to _replacedAtoms
+        _replacedAtoms[&newAtom] = existing;
+      }
+      else {
+        const Atom& use = _platform.handleMultipleDefinitions(*existing, newAtom);
+        if ( &use == existing ) {
+          // using existing atom, add new atom to _replacedAtoms
+          _replacedAtoms[&newAtom] = existing;
+        }
+        else {
+          // using new atom, update tables
+          _nameTable[name] = &newAtom;
+          _replacedAtoms[existing] = &newAtom;
+        }
+      }
       break;
     default:
-      llvm::report_fatal_error("unhandled switch clause");
+      llvm::report_fatal_error("SymbolTable::addByName(): unhandled switch clause");
     }
   }
 }

lld/lib/Core/YamlReader.cpp

@@ -137,6 +137,8 @@ void YAML::parse(llvm::MemoryBuffer *mb, std::vector<const Entry *> &entries) {
         state = inTriplePeriod;
       } else if (c == '\n') {
         // ignore empty lines
+      } else if (c == '\t') {
+        llvm::report_fatal_error("TAB character found in yaml file");
       } else {
         return;
       }
@@ -244,13 +246,14 @@ public:
           , ContentType ct
           , SectionChoice sc
           , bool intn
+          , bool md
           , DeadStripKind dsk
           , bool tb
           , bool al
           , Alignment a
           , YAMLFile& f
           , const char *n)
-    : Atom(ord, d, s, ct, sc, intn, dsk, tb, al, a)
+    : Atom(ord, d, s, ct, sc, intn, md, dsk, tb, al, a)
     , _file(f)
     , _name(n)
     , _size(0)
@@ -311,6 +314,7 @@ public:
   void setType(const char *n);
   void setAlign2(const char *n);
   void setDefinition(const char *n);
+  void setMergeDuplicates(const char *n);
 
   void setFixupKind(const char *n);
   void setFixupOffset(const char *n);
@@ -328,7 +332,7 @@ private:
   Atom::Definition _def;
   Atom::SectionChoice _sectionChoice;
   bool _internalName;
-  bool _userVisibleName;
+  bool _mergeDuplicates;
   Atom::DeadStripKind _dontDeadStrip;
   bool _thumb;
   bool _alias;
@@ -341,7 +345,8 @@ YAMLAtomState::YAMLAtomState()
   , _align(0, 0)
   , _type(Atom::typeData)
   , _scope(Atom::scopeGlobal)
-  , _userVisibleName(true)
+  , _internalName(false)
+  , _mergeDuplicates(false)
   , _dontDeadStrip(Atom::deadStripNormal)
   , _thumb(false)
   , _alias(false) {
@@ -354,8 +359,8 @@ YAMLAtomState::YAMLAtomState()
 
 void YAMLAtomState::makeAtom(YAMLFile& f) {
   Atom *a = new YAMLAtom(_ordinal, _def, _scope, _type, _sectionChoice,
-                         _internalName, _dontDeadStrip, _thumb, _alias,
-                         _align, f, _name);
+                         _internalName, _mergeDuplicates, _dontDeadStrip, 
+                         _thumb, _alias, _align, f, _name);
 
   f._atoms.push_back(a);
   ++_ordinal;
@@ -369,6 +374,7 @@ void YAMLAtomState::makeAtom(YAMLFile& f) {
   _def              = Atom::definitionRegular;
   _sectionChoice    = Atom::sectionBasedOnContent;
   _internalName     = false;
+  _mergeDuplicates  = false;
   _dontDeadStrip    = Atom::deadStripNormal;
   _thumb            = false;
   _alias            = false;
@@ -419,12 +425,23 @@ void YAMLAtomState::setDefinition(const char *s) {
     _def = Atom::definitionRegular;
   else if (strcmp(s, "tentative") == 0)
     _def = Atom::definitionTentative;
+  else if (strcmp(s, "weak") == 0)
+    _def = Atom::definitionWeak;
   else if (strcmp(s, "absolute") == 0)
     _def = Atom::definitionAbsolute;
   else
     llvm::report_fatal_error("bad definition value");
 }
 
+void YAMLAtomState::setMergeDuplicates(const char *s) {
+  if (strcmp(s, "true") == 0)
+    _mergeDuplicates = true;
+  else if (strcmp(s, "false") == 0)
+    _mergeDuplicates = false;
+  else
+    llvm::report_fatal_error("bad merge-duplicates value");
+}
+
 void YAMLAtomState::setFixupKind(const char *s) {
   if (strcmp(s, "pcrel32") == 0)
     _ref.kind = 1;
@@ -525,6 +542,9 @@ llvm::error_code parseObjectText( llvm::MemoryBuffer *mb
         } else if (strcmp(entry->key, "definition") == 0) {
           atomState.setDefinition(entry->value);
           haveAtom = true;
+        } else if (strcmp(entry->key, "merge-duplicates") == 0) {
+          atomState.setMergeDuplicates(entry->value);
+          haveAtom = true;
         } else if (strcmp(entry->key, "fixups") == 0) {
           inFixups = true;
         }

lld/lib/Core/YamlWriter.cpp

@@ -28,11 +28,22 @@ public:
 
   virtual void doFile(const class File &) { }
   virtual void doAtom(const class Atom &atom) {
-    _out << "    - name:        " << atom.name() << "\n";
-    _out << "      internal-name:" << atom.internalName() << "\n";
-    _out << "      definition:  " << definitionString(atom.definition()) <<"\n";
-    _out << "      scope:       " << scopeString(atom.scope()) << "\n";
-    _out << "      type:        " << typeString(atom.contentType()) << "\n";
+    _out << "    - name:             " << atom.name() << "\n";
+    
+    if ( atom.internalName() )
+      _out << "      internal-name:     true\n";
+      
+    if ( atom.definition() != Atom::definitionRegular )
+      _out << "      definition:       " << definitionString(atom.definition()) <<"\n";
+      
+    if ( atom.scope() != Atom::scopeTranslationUnit )
+      _out << "      scope:            " << scopeString(atom.scope()) << "\n";
+      
+    _out << "      type:             " << typeString(atom.contentType()) << "\n";
+    
+    if ( atom.mergeDuplicates() )
+      _out << "      merge-duplicates: true\n";
+      
     if (atom.referencesBegin() != atom.referencesEnd()) {
       _out << "      fixups:\n";
       for (Reference::iterator it = atom.referencesBegin(),
@@ -76,6 +87,8 @@ private:
     switch (def) {
     case Atom::definitionRegular:
       return "regular";
+    case Atom::definitionWeak:
+      return "weak";
     case Atom::definitionTentative:
       return "tentative";
     case Atom::definitionAbsolute:
@@ -88,11 +101,11 @@ private:
   llvm::raw_ostream &_out;
 };
 
-void writeObjectText(File *file, llvm::raw_ostream &out) {
+void writeObjectText(File &file, llvm::raw_ostream &out) {
   Handler h(out);
   out << "---\n";
   out << "atoms:\n";
-  file->forEachAtom(h);
+  file.forEachAtom(h);
   out << "...\n";
 }
 

lld/test/inline-coalesce.objtxt

@@ -0,0 +1,34 @@
+# RUN: lld-core %s | FileCheck %s
+
+#
+# Test that non-inlined inlined functions are silently coalesced
+#
+
+---
+atoms:
+    - name:              _inlineFunc
+      scope:             global
+      definition:        regular
+      type:              code
+      merge-duplicates:  true
+---
+atoms:
+    - name:              _inlineFunc
+      scope:             global
+      definition:        regular
+      type:              code
+      merge-duplicates:  true
+---
+atoms:
+    - name:              _inlineFunc
+      scope:             global
+      definition:        regular
+      type:              code
+      merge-duplicates:  true
+...
+
+
+# CHECK:       name: _inlineFunc
+# CHECK:       merge-duplicates: true
+# CHECK-NOT:   name: _inlineFunc
+# CHECK:       ...

lld/test/multiple-def-error.objtxt

@@ -0,0 +1,18 @@
+# RUN:  lld-core %s  2>&1  |  grep "multiply defined" 
+
+#
+# Test that multiple definitions cause an error
+#
+
+---
+atoms:
+    - name:         _foo
+      definition:   regular
+      type:         data
+---
+atoms:
+    - name:         _foo
+      definition:   regular
+      type:         data
+...
+

lld/test/tent-merge.objtxt

@@ -21,4 +21,4 @@ atoms:
 
 
 # CHECK:        name: _foo
-# CHECK-NEXT:   definition: regular
+# CHECK-NOT:   definition: tentative

lld/test/weak-coalesce.objtxt

@@ -0,0 +1,28 @@
+# RUN: lld-core %s | FileCheck %s
+
+#
+# Test that weak definitions are coalesced away in favor of a regular definition
+#
+
+---
+atoms:
+    - name:         _foo
+      definition:   weak
+      type:         data
+---
+atoms:
+    - name:         _foo
+      definition:   regular
+      type:         data
+---
+atoms:
+    - name:         _foo
+      definition:   weak
+      type:         data
+...
+
+
+# CHECK:       name: _foo
+# CHECK-NOT:   definition: weak
+# CHECK-NOT:   name: _foo
+# CHECK:       ...

lld/tools/lld-core/lld-core.cpp

@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lld/Core/InputFiles.h"
+#include "lld/Core/Atom.h"
 #include "lld/Core/Resolver.h"
 #include "lld/Core/YamlReader.h"
 #include "lld/Core/YamlWriter.h"
@@ -19,6 +20,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/system_error.h"
+#include "llvm/Support/ErrorHandling.h"
 
 #include <vector>
 
@@ -118,6 +120,13 @@ public:
     return false;
   }
 
+  virtual const Atom& handleMultipleDefinitions(const Atom& def1, 
+                                                const Atom& def2) {
+    llvm::report_fatal_error("symbol '" 
+                            + llvm::Twine(def1.name()) 
+                            + "' multiply defined");
+  }
+
   // print out undefined symbol error messages in platform specific way
   virtual void errorWithUndefines(const std::vector<const Atom *> &undefs,
                                   const std::vector<const Atom *> &all) {}
@@ -185,6 +194,6 @@ int main(int argc, const char *argv[]) {
   // write new atom graph out as YAML doc
   std::string errorInfo;
   llvm::raw_fd_ostream out("-", errorInfo);
-  yaml::writeObjectText(&outFile, out);
+  yaml::writeObjectText(outFile, out);
   return 0;
 }