[PATCH] [core] add support to resolve section groups.

Shankar Kalpathi Easwaran shankarke at gmail.com
Tue Mar 25 12:21:23 PDT 2014

  I will add the comment in the Resolver and in the design doc as well.

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
kledzik at apple.com wrote:
> 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.
Yes, I will assert if the target is not a defined atom.

Comment at: lib/ReaderWriter/Native/WriterNative.cpp:41-42
@@ -40,1 +40,4 @@
+      // 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) {
kledzik at apple.com wrote:
> 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.
Will change the comment.

Comment at: lib/ReaderWriter/Native/WriterNative.cpp:48
@@ +47,3 @@
+          if (r->kindValue() == lld::Reference::kindGroupChild)
+            this->addIVarsForDefinedAtom(*((DefinedAtom *)r->target()));
+        } // end loop iteration
kledzik at apple.com wrote:
> Again the child might be some other kind of atom than DefineAtom.
Will assert if the child is not defined.


More information about the llvm-commits mailing list