[PATCH] D29273: [ELF] - Added partial support for --emit-relocs (no --gc-section case)

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputFiles.cpp:409-410
+      if (!isa<InputSection<ELFT>>(Target))
+        fatal(toString(this) + ": --emit-relocs for relocations sections "
+                               "pointing to .eh_frame is not supported");
+      InputSection<ELFT> *RelocSec = make<InputSection<ELFT>>(this, &Sec, Name);
----------------
ruiu wrote:
> So, why? Please explain for those who will read this code.
Will "pointing to .eh_frame is not supported due to an implementation limitation" sound fine for you ?
The only reason for that place is that I want to prepare support for this feature in following patch to simplify review of this.
It is also used in linux kernel, so this fatal is kinda temporarily stub.


================
Comment at: ELF/LinkerScript.cpp:266
+
+    // If we discard a section, we also should discard a dependent section.
+    InputSection<ELFT> *IS = dyn_cast<InputSection<ELFT>>(S);
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > Please describe not what we are doing but why we are doing.
> > Done.
> I still do not understand why you need this. Well, I do understand what you are trying to do here and what your intention is, but I wonder why you specifically have to care about this case.
> 
> Let's say you have sections A and B. A defines symbol X, and B has an undefined symbol X. Naturally, the B's undefined symbol will be resolved using A. But, what if you discard section A using a linker script? What will happen? Isn't this essentially the same situation as this?
Linux kernel contains exactly the situation I show in testcase.
It discards all debug sections:

```
/DISCARD/ : { *(.debug*) }
```

And without that code we just crash for this situation. Because we will try to emit .rela.debug* and crash.
That is the only real situation I care here about.


https://reviews.llvm.org/D29273