[PATCH] D62609: [LLD][ELF][AArch64] Support for BTI and PAC

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: ELF/Arch/AArch64.cpp:543
+
+void AArch64BtiPac::writePltBti(uint8_t *Buf, uint64_t GotPltEntryAddr,
+                                uint64_t PltEntryAddr, int32_t Index,
----------------
peter.smith wrote:
> MaskRay wrote:
> > Have you considered the alternative: rather than define 4 writePlt*, define a unified one:
> > 
> > ```
> > if Bti
> >   write bti c
> > write adrp/ldr/add
> > if Pac
> >   write autiax16x17
> > write br x17
> > write padding nop(s)
> > ```
> I did try that at first, the disadvantage, particularly for the BTI is that the offsets for the relocations change and it gets a bit messy. Not overly so, but more difficult to follow on first reading. I'm happy to do that if you'd prefer?
I do prefer a unified PLT as because this is a big chunk of repetition (4x). While reading the code, I couldn't help comparing the 4 functions just to find the difference is bti/autia1716. Putting them in one place may help.

But I'd like to wait for others to comment on this.


================
Comment at: ELF/Driver.cpp:1607
 // This function returns the merged feature flags. If 0, we cannot enable CET.
-template <class ELFT> static uint32_t getX86Features() {
-  if (Config->EMachine != EM_386 && Config->EMachine != EM_X86_64)
+// This is also the case with AARCH64's BTI and PAC which use the a similar
+// GNU_PROPERTY_AARCH64_FEATURE_1_AND mechanism.
----------------
`the a`, typo?


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

https://reviews.llvm.org/D62609