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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.



> Hi, Peter, very appreciate for your suggestion,  for the Splt, in fact, it is just the entry of the old PLT, you can see in this way: SPLT+PLT=(old)PLT, we split the (old)Plt to Plt +SPlt, one of the reasons is for performance that we want to put them into different areas (hot code, cold code) .
>  And in this patch, I used getSPltVA() instead of getPltVA when face outside. For example in  getRelocTargetVA():
> 
>   case R_PLT:
>     // When the CET Branch protection is enabled, the second PLT
>     // should be call to.
>     if (Config->SPltSectionEnable)
>       return Sym.getSPltVA() + A;
>     else
>       return Sym.getPltVA() + A;
>    
> 
> Maybe it's hard to use In.SpltSection::Enable to do these.
>  Any way I will check it.
>  Thank you very much!

I've checked that it does work, although at the moment it does leave a redundant .splt section in the binary. I think that this can be removed by accounting for this in the SPltSection::empty(), I've left a suggestion inline. I also note that there are no tests that check the contents of the .splt are correct. I think it is important to have those as it can be easy for it to be broken by some later change and no-one noticing it.



================
Comment at: ELF/SyntheticSections.h:700
+  size_t getSize() const override;
+  bool empty() const override { return Entries.empty(); }
+
----------------
If I've understood correctly if the .SPLT is disabled then this should return true, otherwise if you have any ifuncs then a redundant .splt section is produced. 


================
Comment at: test/ELF/x86-64-feature-1-and.s:57
+# NGPSPLT: SHSTK
+# DMPSPLT: splt
+
----------------
The test checks that the splt is created, but it doesn't check that the contents are correct. There should be at least one of these tests that disassembles the .plt and checks that it is loading the correct .got entry.

There also should be a test to show that when an ifunc plt is added then we revert to using the .plt, and that any existing .splt entries are not called.

For example:

```
.type foo STT_GNU_IFUNC
.globl ifunc
ifun:
 ret

 .globl func1
 .type func1, at function
func1:
        callq func2
        callq ifunc    // func2 will have been added to .splt, we don't want func3 to be.
        callq func3
``` 


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D58102