[PATCH] [core] add support to resolve section groups.
    kledzik at apple.com 
    kledzik at apple.com
       
    Tue Mar 25 11:58:59 PDT 2014
    
    
  
  Shankar, can you put your phabricator comment from Mar 25, 11:31 AM  somewhere in the sources, either as a source code comment on in the design docs. It really helps explain how ELF group COMDAT is modeled in lld.
================
Comment at: lib/Core/Resolver.cpp:221-223
@@ +220,5 @@
+            (r->kindValue() == lld::Reference::kindGroupChild)) {
+          const DefinedAtom *target = llvm::dyn_cast<DefinedAtom>(r->target());
+          _atoms.push_back(target);
+          _symbolTable.add(*target);
+        } // Add group child atoms
----------------
It is possible there might be some atoms that are not DefinedAtoms as child atoms.  The code should either properly handle that or assert that it does not.  This downcast to DefinedAtom will return nullptr and cause a later crash.
================
Comment at: lib/ReaderWriter/Native/WriterNative.cpp:41-42
@@ -40,1 +40,4 @@
       this->addIVarsForDefinedAtom(*defAtom);
+      // For handling section groups, lld hides the group child references
+      // lets write all the group child atoms, when we write the parent atom.
+      if (defAtom->contentType() == DefinedAtom::typeGroupComdat) {
----------------
The comment should be that we are trying to process all atoms, but the defined() iterator does not return group children.  So, when a group parent is found, we need to handle each child atom.
================
Comment at: lib/ReaderWriter/Native/WriterNative.cpp:48
@@ +47,3 @@
+          if (r->kindValue() == lld::Reference::kindGroupChild)
+            this->addIVarsForDefinedAtom(*((DefinedAtom *)r->target()));
+        } // end loop iteration
----------------
Again the child might be some other kind of atom than DefineAtom.
http://llvm-reviews.chandlerc.com/D3182
    
    
More information about the llvm-commits
mailing list