[PATCH] D158574: [AArch64][ELF] Support PAUTH ABI marking

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

I haven'ed looked closely, but a `<numeric>` issue makes this not buildable for me..



================
Comment at: lld/ELF/Config.h:151
   uint32_t andFeatures = 0;
+  SmallVector<uint8_t, 16> aarch64PauthAbiTag;
+  SmallVector<uint8_t, 16> gnuPropAarch64Pauth;
----------------
They make the offsets of following variables larger. move to the end.
`<xxx, 0>` is more common in lld. The inline element doesn't save.


================
Comment at: lld/ELF/Driver.cpp:2595
+static void getAarch64PauthInfo() {
+  if (config->emachine != EM_AARCH64)
+    return;
----------------
move the check to the caller


================
Comment at: lld/ELF/Driver.cpp:2612
+              (d1.empty() ? f2 : f1) +
+              " has one. Either all or no link units must have it.");
+
----------------
No trailing period. Please check the existing convention.


================
Comment at: lld/ELF/Driver.cpp:2615
+      if (!std::equal(d1.begin(), d1.end(), d2.begin(), d2.end()))
+        fatal("Incompatible values of aarch64 pauth compatibility info found"
+              "\n" + f1 + ": 0x" + toHex(ArrayRef(d1.data(), d1.size())) +
----------------
Please check the existing convention.




================
Comment at: lld/ELF/InputFiles.cpp:964
+static void parseAarch64PauthAbiTag(const InputSection &sec, ObjFile<ELFT> &f) {
+  if (config->emachine != EM_AARCH64)
+    return;
----------------
move the check to the caller


================
Comment at: lld/ELF/SyntheticSections.cpp:362
+
+  return 16 + std::accumulate(props.begin(), props.end(), 0,
+                              [](uint32_t sum, const Elf_Prop &e) {
----------------
this needs `<numeric>`


================
Comment at: lld/test/ELF/aarch64-feature-pauth.s:3
+
+# RUN: split-file %s %tsplit
+
----------------
`%tsplit` makes temporary output filenames unnecessarily long. Just use `RUN: rm -rf %t && split-file %s %t && cd %t`

please check the existing convention.


================
Comment at: lld/test/ELF/aarch64-feature-pauth.s:59
+
+# ERR6: ld.lld: error: {{.*}} has no aarch64 pauth compatibility info while {{.*}} has one. Either all or no link units must have it.
+
----------------
We omit messages before `error:` as the executable may have a different name, e.g. `ld.lld.exe`. Please follow the style of some recently added tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158574