[PATCH] D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added a comment.

In D58102#1407746 <https://reviews.llvm.org/D58102#1407746>, @ruiu wrote:

> From the linker's perspective, .splt *is* PLT, which inter-module calls jump to. If .splt is enabled, .plt is no longer a PLT in the regular sense, but it just contains code to lazy-resolve symbols. So, I think you should handle .splt as the PLT despite its name throughout this patch if .splt is enabled. For example, In.Plt should be > the .splt and Sym.getPltVA() should return its address in .splt.
>  (I feel that the spec made a mistake when naming .splt -- it should've been .plt and the current "first" PLT should be .splt, but it's probably too late to complain.)


Hi , @ruiu:
I was refining the patch as your second suggestion, I got some troubles in this work. I found it hard to "reverse" the roles of the 2 PLTs in the source code.

1. We can not simply use Sym.getPltVA() instead of  Sym.getSPltVA(), because the first PLT entry is also needed somewhere, for example:

void X86_64<ELFT>::writeGotPlt(uint8_t *Buf, const Symbol &S) const {

  if (Config->SPltSectionEnable)
    // If the PLT is IBT-enabled, the entries in .got.plt also initially point to
    // the entries in the first PLT not the second PLT.
    write64le(Buf, S.getPltVA());
  else
    // Otherwise the entries in .got.plt initially point back to the
    // corresponding PLT entries with a fixed offset to skip the first
    // instruction.
    write64le(Buf, S.getPltVA() + 6);

}

2. If I "reverse" the definition of the 2 PLTs fully, how about the Non-CET case, use SPLT class to definite the plts? And I worry that it may affect the other targets' code.

I am also a little afraid that will take confusions of understanding the code when people refer the ABI files.

Could you please give me suggestions about these if you have some good ideals? 
And do you still concern the options way?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D58102