[PATCH] D37626: [ELF] Scan .eh_frame sections precisely in order to eliminate unused LSDAs and personality routines.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 3 inline comments as done.
ikudrin added inline comments.


================
Comment at: ELF/MarkLive.cpp:180-182
+  const unsigned CieFirstRelI = Cie.FirstRelocation;
+  if (CieFirstRelI == (unsigned)-1)
+    return;
----------------
ruiu wrote:
> Is this code reachable? The GC traverses a graph consists of nodes (sections, fdes, cies, etc.) and edges (relocations). I wonder who can point to this dangling cie.
It's not a dangling CIE. It just doesn't refer to a personality function, which is not prohibited.
And the edge from an FDE to a CIE is not a relocation, but an indirect link through ID field in the FDE.


================
Comment at: ELF/MarkLive.cpp:349
   // Mark all reachable sections.
-  while (!Q.empty())
-    forEachSuccessor<ELFT>(*Q.pop_back_val(), Enqueue);
+  while (!Q.empty()) {
+    while (!Q.empty())
----------------
ruiu wrote:
> How many times does this loop usually repeat? Since this loop scans all EhInputSections on every iteration, I'm a bit worried about its performance.
In case of programs which do not use EH, like `clang` or `lld`, there will be only one iteration.
A typical executable with EH on Linux, where a personal routine is located in a DSO, will require two iterations. And a new flag `ExecInQueue` helps to avoid scanning EH frames for the second time.
If a personality routine is added from a library, it'll require two full iterations, but hardly more, because all references should be satisfied at that point.


================
Comment at: ELF/SyntheticSections.cpp:447-462
+    if (!Piece.Live)
+      continue;
+
     size_t Offset = Piece.InputOff;
     uint32_t ID = read32<E>(Piece.data().data() + 4);
     if (ID == 0) {
       OffsetToCie[Offset] = addCie(Piece, Rels);
----------------
ruiu wrote:
> It is odd that you had to check use `Live` and `isFdeLive`. If GC works correctly, all live sections are marked as such, so you don't need to use `isFdeLive`, no? I believe how this should work.
When `--gc-sections` is not specified, we mark all pieces alive by default. In that case `markLive()` is not called and we have to check the state here to discard FDEs for merged COMDAT sections, for example.


https://reviews.llvm.org/D37626