Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PAC][MC][AArch64] Fix error message for AUTH_ABS64 reloc with ILP32 #89563

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

kovdan01
Copy link
Contributor

The LP64 eqv: should say that the equivalent is AUTH_ABS64 rather than ABS64 when trying to emit an AUTH absolute reloc with ILP32.

The `LP64 eqv:` should say that the equivalent is `AUTH_ABS64` rather
than `ABS64` when trying to emit an AUTH absolute reloc with ILP32.
@kovdan01 kovdan01 marked this pull request as ready for review April 22, 2024 02:07
@llvmbot llvmbot added backend:AArch64 mc Machine (object) code labels Apr 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 22, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-aarch64

Author: Daniil Kovalev (kovdan01)

Changes

The LP64 eqv: should say that the equivalent is AUTH_ABS64 rather than ABS64 when trying to emit an AUTH absolute reloc with ILP32.


Full diff: https://github.com/llvm/llvm-project/pull/89563.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp (+8-8)
  • (modified) llvm/test/MC/AArch64/ilp32-diagnostics.s (+8)
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
index 6e074b6a63c41d..abbf20257edcc0 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
@@ -211,18 +211,18 @@ unsigned AArch64ELFObjectWriter::getRelocType(MCContext &Ctx,
               Target.getAccessVariant() == MCSymbolRefExpr::VK_GOTPCREL)
                  ? ELF::R_AARCH64_GOTPCREL32
                  : R_CLS(ABS32);
-    case FK_Data_8:
+    case FK_Data_8: {
+      bool IsAuth = (RefKind == AArch64MCExpr::VK_AUTH ||
+                     RefKind == AArch64MCExpr::VK_AUTHADDR);
       if (IsILP32) {
         Ctx.reportError(Fixup.getLoc(),
-                        "ILP32 8 byte absolute data "
-                        "relocation not supported (LP64 eqv: ABS64)");
+                        Twine("ILP32 8 byte absolute data "
+                              "relocation not supported (LP64 eqv: ") +
+                            (IsAuth ? "AUTH_ABS64" : "ABS64") + Twine(')'));
         return ELF::R_AARCH64_NONE;
-      } else {
-        if (RefKind == AArch64MCExpr::VK_AUTH ||
-            RefKind == AArch64MCExpr::VK_AUTHADDR)
-          return ELF::R_AARCH64_AUTH_ABS64;
-        return ELF::R_AARCH64_ABS64;
       }
+      return (IsAuth ? ELF::R_AARCH64_AUTH_ABS64 : ELF::R_AARCH64_ABS64);
+    }
     case AArch64::fixup_aarch64_add_imm12:
       if (RefKind == AArch64MCExpr::VK_DTPREL_HI12)
         return R_CLS(TLSLD_ADD_DTPREL_HI12);
diff --git a/llvm/test/MC/AArch64/ilp32-diagnostics.s b/llvm/test/MC/AArch64/ilp32-diagnostics.s
index 65c9e4ea5a1c56..4ca15f160418da 100644
--- a/llvm/test/MC/AArch64/ilp32-diagnostics.s
+++ b/llvm/test/MC/AArch64/ilp32-diagnostics.s
@@ -8,6 +8,14 @@
 
         .xword sym+16
 // CHECK-ERROR: error: ILP32 8 byte absolute data relocation not supported (LP64 eqv: ABS64)
+// CHECK-ERROR: ^
+
+        .xword sym@AUTH(da,42)
+// CHECK-ERROR: error: ILP32 8 byte absolute data relocation not supported (LP64 eqv: AUTH_ABS64)
+// CHECK-ERROR: ^
+
+        .xword sym@AUTH(da,42,addr)
+// CHECK-ERROR: error: ILP32 8 byte absolute data relocation not supported (LP64 eqv: AUTH_ABS64)
 // CHECK-ERROR: ^
 
         movz x7, #:abs_g3:some_label

@kovdan01
Copy link
Contributor Author

AArch64ELFObjectWriter::getRelocType contains many else's after return. If nobody minds, I'll submit a subsequent PR removing use of else after return in this function.

if (IsILP32) {
Ctx.reportError(Fixup.getLoc(),
"ILP32 8 byte absolute data "
"relocation not supported (LP64 eqv: ABS64)");
Twine("ILP32 8 byte absolute data "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8-byte absolute data relocation is not supported in ILP32?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the spec, it's not - see no "ELF32 code" for R_<CLS>_ABS64 in the table here https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#static-data-relocations

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I was suggesting a candidate diagnostic message

8-byte absolute data relocation is not supported in ILP32

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like that error messages across the function use some convention, so it's probably better to stick with it (or change the messages everywhere at once)

@kovdan01 kovdan01 merged commit da57609 into llvm:main Apr 23, 2024
9 checks passed
kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Apr 23, 2024
After llvm#89563, we do not use else after return in code corresponding to
`R_AARCH64_AUTH_ABS64` reloc in `getRelocType`. This patch removes use
of else after return in other places in `getRelocType`.
kovdan01 added a commit that referenced this pull request Apr 24, 2024
…9818)

After #89563, we do not use else after return in code corresponding to
`R_AARCH64_AUTH_ABS64` reloc in `getRelocType`. This patch removes use
of else after return in other places in `getRelocType`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 mc Machine (object) code
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants