[X86] Correct the number of bits in a fixup

[dougk]

Summary

The intended behavior on a too-large relative branch is an assembler crash.
We don't need to do the right thing, because the only branch that can't
be relaxed is J[ER]CXZ, and only hand-written code uses that.

[niravd]

LGTM.

[joerg]

Are you sure about the summary? An assert on user-input is never the intended behavior.

[dougk]

We can change it from an assert to report_fatal_error, but we can't actually distinguish at this point whether it was in fact a user error versus whether it was an internal bug.
I guess I agree that dumping core is a bit sad for the user error case. Would you prefer it always to say this instead?

clang -cc1as: fatal error: error in backend: Value does not fit in fixup

[joerg]

Yeah, that's better. Can we easily get at least the section as well? I really hate having to debug instances like this by running gdb :)

[dougk]

it looks like we need an MCAssembler on which to call getContext() and then Ctx->reportError(Fixup.getLoc(), "mumble").
Some of the other AsmBackends have started to have this plumbing added in.
However most of them actually guard the emission of the message with "if (Ctx)" because we usually don't have the context apparently, so you really can't get a nice source location anyway.
Without this change, we'll silently emit bogus code, so I think we should land it as-is, but do change it from assert to report_fatal_error. Are you ok with that?

[joerg]

Sure, including the sector would be just a QoI issue.

[dougk]

this changes harms the .byte,.short, etc assembler directives. niravd will take a look

lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

void applyFixup(const MCFixup &Fixup, char *Data, unsigned DataSize,
 
     assert(Fixup.getOffset() + Size <= DataSize &&
            "Invalid fixup offset!");
 
     // Check that uppper bits are either all zeros or all ones.
     // Specifically ignore overflow/underflow as long as the leakage is
     // limited to the lower bits. This is to remain compatible with
     // other assemblers.
-    assert(isIntN(Size * 8 + 1, Value) &&
+    assert(isIntN(Size * 8, Value) &&
            "Value does not fit in the Fixup field");
 
     for (unsigned i = 0; i != Size; ++i)
       Data[Fixup.getOffset() + i] = uint8_t(Value >> (i * 8));
   }
 
   bool mayNeedRelaxation(const MCInst &Inst) const override;
 

test/MC/X86/reloc-1-byte.s

+// RUN: not --crash llvm-mc -triple x86_64-linux-gnu -filetype=obj %s 2>%t.err
+// RUN: FileCheck < %t.err %s
+
+  .code64
+        .text
+        jrcxz 1f
+        .fill 128
+1:
+        ret
+
+// CHECK:   "Value does not fit in the Fixup field"