[PATCH] D29273: [ELF] - Added support of linkerscript's "/DISCARD/" for --emit-relocs

[Rafael Avila de Espindola via llvm-commits]

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
> -    if (Config->EmitRelocs)
> -      return make<InputSection<ELFT>>(this, &Sec, Name);
> +    if (Config->EmitRelocs) {
> +      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);
> +      cast<InputSection<ELFT>>(Target)->DependentSection = RelocSec;
> +      return RelocSec;
> +    }

This might be overriding DependentSection, no? If the section has both
relocations and a metadata section:

------------------------------------
.section .foo,"a"
.quad zed
.section .bar,"am", at progbits,.foo
.quad 0
-----------------------------------

Note also that the issue is not specific to relocations, we currently
crash with

test.s:
---------------------------------------
.section .foo,"a"
.quad 0
.section .bar,"am", at progbits,.foo
.quad 0
---------------------------------------
test.script
---------------------------------------
SECTIONS { /DISCARD/ : { *(.foo) } }
---------------------------------------

which I think your patch fixes too. Can you add the test?

Also, can we chain dependent sections? We can write them:

-------------------------------------
.section .foo,"a"
.quad 0
.section .bar,"am", at progbits,.foo
.quad 0
.section .zed,"am", at progbits,.bar
.quad 0
-------------------------------------

and we should either reject them or handle them, but not crash.

What I would suggest is:

* Create a new review that just changes DependentSection to a
  TinyPtrVector. That should fix the handling of

.section foo,"a"
.quad 0
.section .bar,"am", at progbits,foo
.quad 0
.section .zed,"am", at progbits,foo
.quad 0

We currently gc .bar since we just remember the last dependent section.

* With that in your change to put reloc sections as dependent sections
  when using -emit-relocs is reasonable. I am not convinced about the
  direct logic in LinkerScript. It seems that we should just run the
  marklive algorithm, but with the majority of sections already marked
  live so that it only has to check SHF_LINK_ORDER and relocation
  sections.

Cheers,
Rafael