[PATCH] D74537: [LLD][ELF][AArch64] Fix plt generation for PAC/BTI.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/AArch64.cpp:597
+  btiHeader = (config->andFeatures & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) ||
+              config->forceBTI;
   // A BTI (Branch Target Indicator) Plt Entry is only required if the
----------------
danielkiss wrote:
> peter.smith wrote:
> > Unless I missed something, the || config->forceBTI shouldn't be necessary due to forceBTI always adding GNU_PROPERTY_AARCH64_FEATURE_1_BTI to all objects if it isn't there already.
> > 
> > ```
> > // Driver.cpp
> > if (config->forceBTI && !(features & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) {
> >       warn(toString(f) + ": -z force-bti: file does not have "
> >                          "GNU_PROPERTY_AARCH64_FEATURE_1_BTI property");
> >       features |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
> >     }
> > ```
> lld/test/ELF/aarch64-feature-btipac.s:156 will fail without it because in that test the AArch64BtiPac::AArch64BtiPac() runs before the  getAndFeatures(). 
> I guess the PLT generation for the passed so runs before the flags are processed in the driver.
See https://reviews.llvm.org/D74537#1875597

The change should be dropped.


================
Comment at: lld/ELF/Arch/AArch64.cpp:693
 static TargetInfo *getTargetInfo() {
-  if (config->andFeatures & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI |
-                             GNU_PROPERTY_AARCH64_FEATURE_1_PAC)) {
+  if ((config->andFeatures & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) ||
+      config->pacPlt) {
----------------
The change should be dropped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74537