[PATCH] sectiongroup support

[kledzik at apple.com]

  The symbol level collision detection seems to be conflicting with group level removal.  Is this a fair statement of group comdat:

  If two .o files each contain a group named "foo", the linker will pick one of the groups to use (first one?).  All atoms in the first group will be used and all atoms in the other group will be discarded.

  If that is what is supposed to happen then, lld should not be doing any symbol level collision detection on atoms that are in a group.

  The algorithm to do this gets tricky if you want to support multi-threaded symbol table updates, because the "first (in command line order)" file may be parsed last, so no group picking can be done until all files are parsed.

  Here is a possible algorithm:
  1) Require that any object file that vends group atoms, supplies the parent group atom before its children when iterated the defined atoms.
  2) When a group atom is added to the symbol table, its references will be scanned and a pointer to each child will be added to a _groupChild set in the symbol table.
  3) When an atom is added by name, a quick test is done _groupChild.count(atom) to see if it is a child.  If it is a child, early exit from the add method.
  4) Once the .o  files are added and we start lookup to see if an undefined symbols remain, then we deal with groups.  We find all groups used and add their children, and find groups not used and mark their children as being replaced.

  I'm not sure how this will interact with static library loading.  It is the remaining undefines that drive searching archives.  But there may be a remaining undefine because the definition is in a group and group processing is delayed to allow multi-threading...


================
Comment at: lib/Core/Resolver.cpp:445-458
@@ +444,16 @@
+          (iterDefAtom->contentType() == DefinedAtom::typeGroupComdat)) {
+        std::vector<std::string> groupMemberDifference;
+        if (!verifyGroupMembers(iterDefAtom, replacedDefAtom,
+                                groupMemberDifference)) {
+          llvm::errs() << "New group member exists in " << iterDefAtom->name()
+                       << ":" << iterDefAtom->file().path()
+                       << " which is not found in group "
+                       << replacedDefAtom->name() << ":"
+                       << replacedDefAtom->file().path() << "\n";
+          llvm::errs() << "Group difference (new members)"
+                       << "\n";
+          for (auto mem : groupMemberDifference)
+            llvm::errs() << mem << " ";
+          llvm::errs() << "\n";
+          llvm::report_fatal_error("group resolution error");
+        }
----------------
Shankar Kalpathi Easwaran wrote:
> kledzik at apple.com wrote:
> > I don't think we need to compare the members (unless there is some command line option for debugging group comdat coalescing).  You can have files compiled with different optimizations which may lead to different inlining and thus different symbols exposed for the same group.
> could this be a warning instead of a error to notify the user ?
What does the ELF linker do? 

================
Comment at: lib/Core/SymbolTable.cpp:197-198
@@ +196,4 @@
+            return;
+          if (signatureAtomForExisting->name() == signatureAtomForNew->name())
+            useNew = false;
+          break;
----------------
Shankar Kalpathi Easwaran wrote:
> kledzik at apple.com wrote:
> > At this point we may not know which group will be used, so we may want to punt and determine the replacement part later.
> Do we regard the atom as just a duplicate atom, if the signatures dont match and leave it to the replacement to decide atoms in the group to be coalesced ?
Hmm.  We can't fallback on the replacement because the group replacement may later override that.  For the same reason we can't make any replacement decision in the NCR_DupDef case if one is in a group.  

See overall comment.


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