[PATCH] D85242: [AArch64] [Windows] Error out on unsupported symbol locations

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo updated this revision to Diff 283174.
mstorsjo retitled this revision from "[AArch64] [Windows] Error out on some ELF style GOT relative relocations" to "[AArch64] [Windows] Error out on unsupported symbol locations".
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Using getSymbolLoc(), moved the check to the top of the function, doing the check of symbol location regardless of relocation type (as only VK_ABS and VK_SECREL are supported), changed a preexisting report_fatal_error for unsupported relocation types into an error with a source location, as it can be triggered with custom assembly input.

I've managed to a rather large corpus of code (VLC, including around 100 third party library dependencies) with this patch, so it looks fairly safe that no other symbol locations than VK_ABS and VK_SECREL are being used in successfully built code so far.


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

https://reviews.llvm.org/D85242

Files:
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64WinCOFFObjectWriter.cpp
  llvm/test/MC/AArch64/coff-relocations-diags.s


Index: llvm/test/MC/AArch64/coff-relocations-diags.s
===================================================================
--- /dev/null
+++ llvm/test/MC/AArch64/coff-relocations-diags.s
@@ -0,0 +1,43 @@
+// RUN: not llvm-mc -triple aarch64-win32 -filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
+
+  adrp x0, :got:symbol
+  // CHECK: error: symbol kind :got: unsupported on COFF targets
+  // CHECK-NEXT: adrp x0, :got:symbol
+  // CHECK-NEXT: ^
+
+  ldr x0, [x0, :got_lo12:symbol]
+  // CHECK: error: symbol kind :got_lo12: unsupported on COFF targets
+  // CHECK-NEXT: ldr x0, [x0, :got_lo12:symbol]
+  // CHECK-NEXT: ^
+
+  adrp x0, :tlsdesc:symbol
+  // CHECK: error: symbol kind :tlsdesc: unsupported on COFF targets
+  // CHECK-NEXT: adrp x0, :tlsdesc:symbol
+  // CHECK-NEXT: ^
+  add x0, x0, :tlsdesc_lo12:symbol
+  // CHECK: error: symbol kind :tlsdesc_lo12: unsupported on COFF targets
+  // CHECK-NEXT: add x0, x0, :tlsdesc_lo12:symbol
+  // CHECK-NEXT: ^
+
+  adrp x0, :gottprel:symbol
+  // CHECK: error: symbol kind :gottprel: unsupported on COFF targets
+  // CHECK-NEXT: adrp x0, :gottprel:symbol
+  // CHECK-NEXT: ^
+  ldr x0, [x0, :gottprel_lo12:symbol]
+  // CHECK: error: symbol kind :gottprel_lo12: unsupported on COFF targets
+  // CHECK-NEXT: ldr x0, [x0, :gottprel_lo12:symbol]
+  // CHECK-NEXT: ^
+
+  add x0, x0, #:dtprel_hi12:symbol, lsl #12
+  // CHECK: error: symbol kind :dtprel_hi12: unsupported on COFF targets
+  // CHECK-NEXT: add x0, x0, #:dtprel_hi12:symbol, lsl #12
+  // CHECK-NEXT: ^
+  add x0, x0, :dtprel_lo12:symbol
+  // CHECK: error: symbol kind :dtprel_lo12: unsupported on COFF targets
+  // CHECK-NEXT: add x0, x0, :dtprel_lo12:symbol
+  // CHECK-NEXT: ^
+
+  movz x0, #:abs_g0:symbol
+  // CHECK: error: relocation type :abs_g0: unsupported on COFF targets
+  // CHECK-NEXT: movz x0, #:abs_g0:symbol
+  // CHECK-NEXT: ^
Index: llvm/lib/Target/AArch64/MCTargetDesc/AArch64WinCOFFObjectWriter.cpp
===================================================================
--- llvm/lib/Target/AArch64/MCTargetDesc/AArch64WinCOFFObjectWriter.cpp
+++ llvm/lib/Target/AArch64/MCTargetDesc/AArch64WinCOFFObjectWriter.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/MC/MCAsmBackend.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCFixup.h"
 #include "llvm/MC/MCFixupKindInfo.h"
@@ -48,10 +49,33 @@
                                       : Target.getSymA()->getKind();
   const MCExpr *Expr = Fixup.getValue();
 
+  if (const AArch64MCExpr *A64E = dyn_cast<AArch64MCExpr>(Expr)) {
+    AArch64MCExpr::VariantKind RefKind = A64E->getKind();
+    switch (AArch64MCExpr::getSymbolLoc(RefKind)) {
+    case AArch64MCExpr::VK_ABS:
+    case AArch64MCExpr::VK_SECREL:
+      // Supported
+      break;
+    default:
+      Ctx.reportError(Fixup.getLoc(), "symbol kind " +
+                                          A64E->getVariantKindName() +
+                                          " unsupported on COFF targets");
+      return COFF::IMAGE_REL_ARM64_ABSOLUTE; // Dummy return value
+    }
+  }
+
   switch (static_cast<unsigned>(Fixup.getKind())) {
   default: {
-    const MCFixupKindInfo &Info = MAB.getFixupKindInfo(Fixup.getKind());
-    report_fatal_error(Twine("unsupported relocation type: ") + Info.Name);
+    if (const AArch64MCExpr *A64E = dyn_cast<AArch64MCExpr>(Expr)) {
+      Ctx.reportError(Fixup.getLoc(), "relocation type " +
+                                          A64E->getVariantKindName() +
+                                          " unsupported on COFF targets");
+    } else {
+      const MCFixupKindInfo &Info = MAB.getFixupKindInfo(Fixup.getKind());
+      Ctx.reportError(Fixup.getLoc(), Twine("relocation type ") + Info.Name +
+                                          " unsupported on COFF targets");
+    }
+    return COFF::IMAGE_REL_ARM64_ABSOLUTE; // Dummy return value
   }
 
   case FK_Data_4:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85242.283174.patch
Type: text/x-patch
Size: 3960 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200805/78b2201e/attachment.bin>