[PATCH] D84131: [ELF] Support -r --gc-sections

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 22 00:59:36 PDT 2020


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with one remaining nit.



================
Comment at: lld/ELF/InputFiles.cpp:946
 
-    // Relocation sections processed by the linker are usually removed
-    // from the output, so returning `nullptr` for the normal case.
-    // However, if -emit-relocs is given, we need to leave them in the output.
-    // (Some post link analysis tools need this information.)
-    if (config->emitRelocs) {
-      InputSection *relocSec = make<InputSection>(*this, sec, name);
-      // We will not emit relocation section if target was discarded.
-      target->dependentSections.push_back(relocSec);
-      return relocSec;
-    }
-    return nullptr;
+    // Relocation sections are usually removed from the output, so returning
+    // `nullptr` for the normal case. However, if -r or --emit-relocs is
----------------
returning -> return


================
Comment at: lld/ELF/InputFiles.cpp:951-955
+      // We want to add a dependency to target, similar like we do for
+      // -emit-relocs below. This is useful for the case when linker script
+      // contains the "/DISCARD/". It is perhaps uncommon to use a script with
+      // -r, but we faced it in the Linux kernel and have to handle such case
+      // and not to crash.
----------------
MaskRay wrote:
> jhenderson wrote:
> > similar like we -> like we
> > for the case when linker script -> when a linker script
> > the "/DISCARD/" -> a "/DISCARD/" output section
> > have to handle such case and not to crash -> have to handle such cases without crashing
> The two `if` can be merged. I merged them.
Makes sense!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84131/new/

https://reviews.llvm.org/D84131





More information about the llvm-commits mailing list