[PATCH] D53864: [ELF] - Do not crash when -r output uses linker script with `/DISCARD/`

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 30 04:25:43 PDT 2018


grimar added inline comments.


================
Comment at: ELF/InputFiles.cpp:648
     // but just copy relocation sections to output.
-    if (Config->Relocatable)
-      return make<InputSection>(*this, Sec, Name);
+    if (Config->Relocatable) {
+      InputSection *RelocSec = make<InputSection>(*this, Sec, Name);
----------------
peter.smith wrote:
> This bit of code looks like it could be merged with if (Config->EmitRelocs) on line 692
> 
> By doing so we wouldn't be skipping the code at 672 for Config->Relocatable I don't think that this code has any side-effects that would affect a discarded section though.
I would prefer keeping early return, that is a bit simpler IMO. Let's see what others think.


================
Comment at: ELF/InputFiles.cpp:652
+      // -emit-relocs below. This is useful for case when linker script contains
+      // the "/DISCARD/". It is uncommon to have script with -r, but arm64 linux
+      // kernel use it and we have to handle such case and do not crash.
----------------
peter.smith wrote:
> My understanding is that it is all Linux kernel modules use that fragment of linker script and not just arm64.
Maybe it is just better to omit the name completely. It does not seem to be important that we observed this in Linux kernel, it could be anywhere.


https://reviews.llvm.org/D53864





More information about the llvm-commits mailing list