[PATCH] D38613: [ELF] - Change how we deal with unused synthetic sections.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
Herald added a subscriber: emaste.

It turns out our code that handles unused synthetic sections is
a bit overcomplicated.
I think we set output section `Live` bit too early and do things in 
`removeUnusedSyntheticSections` that we can skip.

I suggest to set `Live` bit of output section early only if it is
known that it is live. Then we can update this flag after we get
final information about synthetic section status (empty/non-empty).
For input sections that are empty we could just disable them,
setting their `Live` flag to false.

Approach I am suggesting is implemented in this patch.


https://reviews.llvm.org/D38613

Files:
  ELF/MapFile.cpp
  ELF/OutputSections.cpp
  ELF/Writer.cpp


Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -1168,37 +1168,16 @@
 // relocation that needs a .got, we don't want to emit .got.
 //
 // To deal with the above problem, this function is called after
-// scanRelocations is called to remove synthetic sections that turn
-// out to be empty.
-static void removeUnusedSyntheticSections() {
-  // All input synthetic sections that can be empty are placed after
-  // all regular ones. We iterate over them all and exit at first
-  // non-synthetic.
-  for (InputSectionBase *S : llvm::reverse(InputSections)) {
-    SyntheticSection *SS = dyn_cast<SyntheticSection>(S);
-    if (!SS)
-      return;
-    OutputSection *OS = SS->getParent();
-    if (!SS->empty() || !OS)
-      continue;
-
-    std::vector<BaseCommand *>::iterator Empty = OS->Commands.end();
-    for (auto I = OS->Commands.begin(), E = OS->Commands.end(); I != E; ++I) {
-      BaseCommand *B = *I;
-      if (auto *ISD = dyn_cast<InputSectionDescription>(B)) {
-        llvm::erase_if(ISD->Sections,
-                       [=](InputSection *IS) { return IS == SS; });
-        if (ISD->Sections.empty())
-          Empty = I;
-      }
+// scanRelocations to disable synthetic sections that turn out to be
+// empty. If we find that section should be kept, we must update output
+// section live bit to ensure it is emited.
+static void disableUnusedSyntheticSections() {
+  for (InputSectionBase *S : InputSections) {
+    if (SyntheticSection *SS = dyn_cast<SyntheticSection>(S)) {
+      SS->Live = !SS->empty();
+      if (OutputSection *OS = SS->getParent())
+        OS->Live |= SS->Live;
     }
-    if (Empty != OS->Commands.end())
-      OS->Commands.erase(Empty);
-
-    // If there are no other sections in the output section, remove it from the
-    // output.
-    if (OS->Commands.empty())
-      OS->Live = false;
   }
 }
 
@@ -1301,7 +1280,7 @@
     return;
 
   addPredefinedSections();
-  removeUnusedSyntheticSections();
+  disableUnusedSyntheticSections();
 
   sortSections();
   Script->removeEmptyCommands();
Index: ELF/OutputSections.cpp
===================================================================
--- ELF/OutputSections.cpp
+++ ELF/OutputSections.cpp
@@ -84,7 +84,16 @@
 
 void OutputSection::addSection(InputSection *S) {
   assert(S->Live);
-  Live = true;
+
+  // Adding regular sections should trigger live bit, so that we enable emiting
+  // this output section. Synthetic sections are exception, because we do not
+  // know their final status here yet. So we set Live flag here only if we
+  // definetely know we are going to keep this synthetic section.
+  if (SyntheticSection *SS = dyn_cast<SyntheticSection>(S))
+    Live |= !SS->empty();
+  else
+    Live = true;
+
   S->Parent = this;
   this->updateAlignment(S->Alignment);
 
Index: ELF/MapFile.cpp
===================================================================
--- ELF/MapFile.cpp
+++ ELF/MapFile.cpp
@@ -138,6 +138,8 @@
       if (!ISD)
         continue;
       for (InputSection *IS : ISD->Sections) {
+        if (!IS->Live)
+          continue;
         writeHeader(OS, OSec->Addr + IS->OutSecOff, IS->getSize(),
                     IS->Alignment);
         OS << indent(1) << toString(IS) << '\n';


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38613.117964.patch
Type: text/x-patch
Size: 3311 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171006/df2b6d30/attachment.bin>