[PATCH] D59780: Support Intel Control-flow Enforcement Technology

[Rui Ueyama via Phabricator via llvm-commits]

ruiu marked 10 inline comments as done.
ruiu added inline comments.


================
Comment at: lld/ELF/Arch/X86_64.cpp:209-210
+    write32le(Buf + 5, I * sizeof(object::ELF64LE::Rela));
+    write32le(Buf + 10, -getPltEntryOffset(I) - 14);
+    Buf += sizeof(Inst);
+  }
----------------
xiangzhangllvm wrote:
> It seems will always write the same place.
Buf is incremented on every loop iteration.


================
Comment at: lld/ELF/InputFiles.cpp:627
+    while (!Desc.empty()) {
+      if (Desc.size() < 8)
+        fatal(toString(Obj) + ": .note.gnu.property: section too short");
----------------
xiangzhangllvm wrote:
> Should be < 12 ? Desc must have at least 3 elements: type(4B), datasz(4B), data(4B)
Data can be empty.


================
Comment at: lld/ELF/InputFiles.cpp:631
+      uint32_t Type = *reinterpret_cast<const uint32_t *>(Desc.data());
+      uint32_t Size = *reinterpret_cast<const uint32_t *>(Desc.data() + 4) * 4;
+      if (Size == 0)
----------------
MaskRay wrote:
> xiangzhangllvm wrote:
> > The Size calculation here seems strange.  Why *4 ?
> > *(Desc.data()+4) is datasz, it is the feature size. 
> > I think it should be
> > uint32_t Size = *reinterpret_cast<const uint32_t *>(Desc.data() + 4) + 8 +Pading;
> > (for 32-bit the Pading is always 0, and 64-bit is 4, due to their alignments)
> Use `read32le`. `*reinterpret_cast<const uint32_t *>(Desc.data())` doesn't work when cross linking x86 modules on a big-endian architecture.
SIze is actually the size of the body, so I needed +8. Fixed.


================
Comment at: lld/test/ELF/Inputs/x86-64-cet4.s:3
+.long 4
+.long 16
+.long 5
----------------
xiangzhangllvm wrote:
> This should not be fixed if used for 32-bit tests.
> The standard style should be:
> 
> ```
>  .p2align x
>  .long 4
>  .long 4f -1f
>  .long 5
> ...
> 1:
>  .....
> .p2align x
> 4:
> ```
I don't want to calculate offset in the assembler as this is a test file. Something more obvious is preferable. I added i386-cet* test files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59780