[PATCH] D156505: [AArch64][ELF] Support R_AARCH64_AUTH_ABS64 static relocation

Daniil Kovalev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 7 00:53:42 PDT 2023


kovdan01 added a comment.

@MaskRay Fixed the issues you've mentioned. Would be glad to see your review on new version of the patch.



================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:7555
+  EndLoc = Parser.getTok().getEndLoc();
+  if (parseToken(AsmToken::RParen,
+                 "expected ')' at the end of @AUTH expression"))
----------------
MaskRay wrote:
> The error message is not tested.
> 
> I think a simple `"expected ')'"` is sufficient. The integrated assembler marks the column that an error occurs, so it's obvious that this message applies to `@AUTH`.
> I think a simple "expected ')'" is sufficient. The integrated assembler marks the column that an error occurs, so it's obvious that this message applies to @AUTH.

It makes sense. Deleted similar redundant info from other error messages as well.


================
Comment at: test/MC/AArch64/arm64-ptrauth-elf-reloc.s:5
+// RUN: llvm-mc -triple=arm64-linux -filetype=obj < %s | \
+// RUN: llvm-readobj --expand-relocs --sections --section-relocations --section-data - | \
+// RUN: FileCheck %s --check-prefix=RELOC
----------------
MaskRay wrote:
> `llvm-readobj` is quite verbose. Tabular output from `llvm-readelf -S -r` may look better. Use `-x sectionname` for content.
Thanks, llvm-readelf's output definitely looks better in our case


================
Comment at: test/MC/AArch64/arm64-ptrauth-elf-reloc.s:114
+.quad 0
+
+// ASM:          .xword _g2 at AUTH(da,5,addr)
----------------
MaskRay wrote:
> A test involving more modifiers like `a at PLT@AUTH(ib,0)` may be interesting to add.
Explicitly marked such combinations as unsupported. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156505



More information about the llvm-commits mailing list