[lld][LoongArch] Print error when encountering R_LARCH_ALIGN

[SixWeining]

GAS (from binutils 2.41) defaults to emit R_LARCH_ALIGN for the .align directive for later linker relaxation in ld. But lld hasn't implemented relaxation on LoongArch currently. So we print a more valuable error than the default when encountering R_LARCH_ALIGN.

Similar to https://reviews.llvm.org/D71820.

[llvmbot]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Changes

GAS (from binutils 2.41) defaults to emit R_LARCH_ALIGN for the .align directive for later linker relaxation in ld. But lld hasn't unimplemented relaxation on LoongArch currently. So we print a more valuable error than the default when encountering R_LARCH_ALIGN.

Similar to https://reviews.llvm.org/D71820.

---

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

2 Files Affected:

• (modified) lld/ELF/Arch/LoongArch.cpp (+7)
• (added) lld/test/ELF/loongarch-reloc-align.s (+15)

diff --git a/lld/ELF/Arch/LoongArch.cpp b/lld/ELF/Arch/LoongArch.cpp
index 04ddb4682917b4b..8f7cd7db9233e88 100644
--- a/lld/ELF/Arch/LoongArch.cpp
+++ b/lld/ELF/Arch/LoongArch.cpp
@@ -521,6 +521,13 @@ RelExpr LoongArch::getRelExpr(const RelType type, const Symbol &s,
   case R_LARCH_RELAX:
     // LoongArch linker relaxation is not implemented yet.
     return R_NONE;
+  case R_LARCH_ALIGN:
+    // Not just a hint; always padded to the worst-case number of NOPs, so may
+    // not currently be aligned, and without linker relaxation support we can't
+    // delete NOPs to realign.
+    errorOrWarn(getErrorLocation(loc) + "relocation R_LARCH_ALIGN requires "
+                "unimplemented linker relaxation; recompile with -mno-relax");
+    return R_NONE;
 
   // Other known relocs that are explicitly unimplemented:
   //
diff --git a/lld/test/ELF/loongarch-reloc-align.s b/lld/test/ELF/loongarch-reloc-align.s
new file mode 100644
index 000000000000000..8e4eeca3d8e0a25
--- /dev/null
+++ b/lld/test/ELF/loongarch-reloc-align.s
@@ -0,0 +1,15 @@
+# REQUIRES: loongarch
+
+# RUN: llvm-mc --filetype=obj --triple=loongarch64 %s -o %t.o
+# RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
+
+# CHECK: relocation R_LARCH_ALIGN requires unimplemented linker relaxation
+
+.global _start
+_start:
+    addi.d $t0, $t0, 1
+1:
+    nop
+    .reloc 1b, R_LARCH_ALIGN, 12
+    nop
+    nop

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[SixWeining]

@xen0n @xry111 What do you think about this? Seems I cannot find you in the Reviewers popup.

[MaskRay]

Err, binutils defaulting to -mrelax and copying R_RISCV_ALIGN is unfortunate. I've mentioned it could be probelamtic: loongson/LoongArch-Documentation#77

For most align directives, not respecting the required alignment due to linker not support -mrelax isn't really an issue. Printing an error would likely be more disruptive. Perhaps the choice is just to silently ignore.

Now with GitHub pull requests, it's difficult to become a blocking reviewer, but I'll request changes for now.

[SixWeining]

For most align directives, not respecting the required alignment due to linker not support -mrelax isn't really an issue. Printing an error would likely be more disruptive. Perhaps the choice is just to silently ignore.

Thanks. It makes sense. Ignoring this reloc doesn’t cause functionality issues but performance issues (thinking about the align directives emitted by function address alignment or loop alignment).

I will update this PR by ignoring this reloc.

[xry111]

But in various cases the .align directives are necessary for correct function. For example in kernel there is some duff-device-like constructions based on jump labels with equal distances guaranteed by .align, and in libffi we use .align to ensure an alignment to page boundary.

[jrtc27]

Ignoring alignment is not an option. Same as why I made it an error for RISC-V.

[MaskRay]

There are certainly places where alignment directives are mandatory, but I feel that reporting an error would make lld not usable for many many benign cases. Consider the scenario that if your crt1.o or crtbeginS.o have R_LARCH_ALIGN.

This interop is exactly why I warned in loongson/LoongArch-Documentation#77 . At the very least, binutils should not enable -mrelax by default. Reporting the error is now no better than relaxing R_LARCH_ALIGN.

[xry111]

There are certainly places where alignment directives are mandatory, but I feel that reporting an error would make lld not usable for many many benign cases. Consider the scenario that if your crt1.o or crtbeginS.o have R_LARCH_ALIGN.

This interop is exactly why I warned in loongson/LoongArch-Documentation#77 . At the very least, binutils should not enable -mrelax by default. Reporting the error is now no better than relaxing R_LARCH_ALIGN.

It is better because detecting the issue at link time is better than producing a subtly buggy link unit which will blow up at runtime.

Meanwhile I think (1) we should make lld support R_LARCH_ALIGN (even if we still ignore R_LARCH_RELAX); (2) in Glibc & GCC we can disable relax for crt*.o and lib*_nonshared.a.

[SixWeining]

So implementing R_LARCH_ALIGN is the best choice. Let me close this PR. New PRs for relaxing will be sent in next few days.

[SixWeining]

So implementing R_LARCH_ALIGN is the best choice. Let me close this PR. New PRs for relaxing will be sent in next few days.

Part of the PRs for relaxation: #72191 #72190