Skip to content

Commit

Permalink
[X86][MC] no error diagnostic for out-of-range jrcxz/jecxz/jcxz
Browse files Browse the repository at this point in the history
Fix for PR24072:

X86 instructions jrcxz/jecxz/jcxz performs short jumps if rcx/ecx/cx register is 0
The maximum relative offset for a forward short jump is 127 Bytes (0x7F).
The maximum relative offset for a backward short jump is 128 Bytes (0x80).

Gnu assembler warns when the distance of the jump exceeds the maximum but llvm-as does not.

Patch by Konstantin Belochapka and Alexey Lapshin

Differential Revision: https://reviews.llvm.org/D70652
  • Loading branch information
avl-llvm committed Nov 26, 2019
1 parent 8f2b57d commit e73f78a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 7 deletions.
27 changes: 21 additions & 6 deletions llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
Expand Up @@ -12,6 +12,8 @@
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/BinaryFormat/MachO.h"
#include "llvm/MC/MCAsmBackend.h"
#include "llvm/MC/MCAssembler.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCDwarf.h"
#include "llvm/MC/MCELFObjectWriter.h"
#include "llvm/MC/MCExpr.h"
Expand All @@ -22,6 +24,7 @@
#include "llvm/MC/MCRegisterInfo.h"
#include "llvm/MC/MCSectionMachO.h"
#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/MC/MCValue.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/raw_ostream.h"
using namespace llvm;
Expand Down Expand Up @@ -114,12 +117,24 @@ class X86AsmBackend : public MCAsmBackend {

assert(Fixup.getOffset() + Size <= Data.size() && "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((Size == 0 || isIntN(Size * 8 + 1, Value)) &&
"Value does not fit in the Fixup field");
int64_t SignedValue = static_cast<int64_t>(Value);
if ((Target.isAbsolute() || IsResolved) &&
getFixupKindInfo(Fixup.getKind()).Flags &
MCFixupKindInfo::FKF_IsPCRel) {
// check that PC relative fixup fits into the fixup size.
if (Size > 0 && !isIntN(Size * 8, SignedValue))
Asm.getContext().reportError(
Fixup.getLoc(), "value of " + Twine(SignedValue) +
" is too large for field of " + Twine(Size) +
((Size == 1) ? " byte." : " bytes."));
} else {
// 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((Size == 0 || isIntN(Size * 8 + 1, SignedValue)) &&
"Value does not fit in the Fixup field");
}

for (unsigned i = 0; i != Size; ++i)
Data[Fixup.getOffset() + i] = uint8_t(Value >> (i * 8));
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/MC/MachO/reloc.s
Expand Up @@ -37,7 +37,7 @@ L0:
.text
_f0:
L1:
jmp 0xbabecafe
jmp 0x7abecafe
jmp L0
jmp L1
ret
Expand Down
26 changes: 26 additions & 0 deletions llvm/test/MC/X86/x86-jcxz-loop-fixup.s
@@ -0,0 +1,26 @@
# RUN: not llvm-mc -filetype=obj -triple=x86_64-linux-gnu %s 2>&1 | FileCheck %s

.balign 128
label00:
// CHECK: value of 253 is too large for field of 1 byte.
jecxz label01
// CHECK: value of 251 is too large for field of 1 byte.
jrcxz label01
// CHECK: value of 249 is too large for field of 1 byte.
loop label01
// CHECK: value of 247 is too large for field of 1 byte.
loope label01
// CHECK: value of 245 is too large for field of 1 byte.
loopne label01
.balign 256
label01:
// CHECK: value of -259 is too large for field of 1 byte.
jecxz label00
// CHECK: value of -261 is too large for field of 1 byte.
jrcxz label00
// CHECK: value of -263 is too large for field of 1 byte.
loop label00
// CHECK: value of -265 is too large for field of 1 byte.
loope label00
// CHECK: value of -267 is too large for field of 1 byte.
loopne label00

0 comments on commit e73f78a

Please sign in to comment.