[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