[llvm] r308972 - X86 Asm uses assertions instead of proper diagnostic. This patch fixes that.
Andrew V. Tischenko via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 25 06:05:12 PDT 2017
Author: avt77
Date: Tue Jul 25 06:05:12 2017
New Revision: 308972
URL: http://llvm.org/viewvc/llvm-project?rev=308972&view=rev
Log:
X86 Asm uses assertions instead of proper diagnostic. This patch fixes that.
Differential Revision: https://reviews.llvm.org/D35115
Added:
llvm/trunk/test/MC/X86/intel-syntax-3.s
Modified:
llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
llvm/trunk/test/MC/X86/intel-syntax-invalid-scale.s
llvm/trunk/test/MC/X86/intel-syntax.s
Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp?rev=308972&r1=308971&r2=308972&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
+++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Tue Jul 25 06:05:12 2017
@@ -37,6 +37,14 @@
using namespace llvm;
+static bool checkScale(unsigned Scale, StringRef &ErrMsg) {
+ if (Scale != 1 && Scale != 2 && Scale != 4 && Scale != 8) {
+ ErrMsg = "scale factor in address must be 1, 2, 4 or 8";
+ return true;
+ }
+ return false;
+}
+
namespace {
static const char OpPrecedence[] = {
@@ -125,8 +133,8 @@ private:
int64_t popOperand() {
assert (!PostfixStack.empty() && "Poped an empty stack!");
ICToken Op = PostfixStack.pop_back_val();
- assert ((Op.first == IC_IMM || Op.first == IC_REGISTER)
- && "Expected and immediate or register!");
+ if (!(Op.first == IC_IMM || Op.first == IC_REGISTER))
+ return -1; // The invalid Scale value will be caught later by checkScale
return Op.second;
}
void pushOperand(InfixCalculatorTok Op, int64_t Val = 0) {
@@ -422,7 +430,7 @@ private:
}
PrevState = CurrState;
}
- void onPlus() {
+ bool onPlus(StringRef &ErrMsg) {
IntelExprState CurrState = State;
switch (State) {
default:
@@ -439,7 +447,10 @@ private:
if (!BaseReg) {
BaseReg = TmpReg;
} else {
- assert (!IndexReg && "BaseReg/IndexReg already set!");
+ if (IndexReg) {
+ ErrMsg = "BaseReg/IndexReg already set!";
+ return true;
+ }
IndexReg = TmpReg;
Scale = 1;
}
@@ -447,8 +458,9 @@ private:
break;
}
PrevState = CurrState;
+ return false;
}
- void onMinus() {
+ bool onMinus(StringRef &ErrMsg) {
IntelExprState CurrState = State;
switch (State) {
default:
@@ -475,7 +487,11 @@ private:
if (CurrState == IES_REGISTER || CurrState == IES_RPAREN ||
CurrState == IES_INTEGER || CurrState == IES_RBRAC)
IC.pushOperator(IC_MINUS);
- else
+ else if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
+ // We have negate operator for Scale: it's illegal
+ ErrMsg = "Scale can't be negative";
+ return true;
+ } else
IC.pushOperator(IC_NEG);
if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
// If we already have a BaseReg, then assume this is the IndexReg with
@@ -483,7 +499,10 @@ private:
if (!BaseReg) {
BaseReg = TmpReg;
} else {
- assert (!IndexReg && "BaseReg/IndexReg already set!");
+ if (IndexReg) {
+ ErrMsg = "BaseReg/IndexReg already set!";
+ return true;
+ }
IndexReg = TmpReg;
Scale = 1;
}
@@ -491,6 +510,7 @@ private:
break;
}
PrevState = CurrState;
+ return false;
}
void onNot() {
IntelExprState CurrState = State;
@@ -517,7 +537,8 @@ private:
}
PrevState = CurrState;
}
- void onRegister(unsigned Reg) {
+
+ bool onRegister(unsigned Reg, StringRef &ErrMsg) {
IntelExprState CurrState = State;
switch (State) {
default:
@@ -532,11 +553,16 @@ private:
case IES_MULTIPLY:
// Index Register - Scale * Register
if (PrevState == IES_INTEGER) {
- assert (!IndexReg && "IndexReg already set!");
+ if (IndexReg) {
+ ErrMsg = "BaseReg/IndexReg already set!";
+ return true;
+ }
State = IES_REGISTER;
IndexReg = Reg;
// Get the scale and replace the 'Scale * Register' with '0'.
Scale = IC.popOperand();
+ if (checkScale(Scale, ErrMsg))
+ return true;
IC.pushOperand(IC_IMM);
IC.popOperator();
} else {
@@ -545,6 +571,7 @@ private:
break;
}
PrevState = CurrState;
+ return false;
}
void onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName) {
PrevState = State;
@@ -583,13 +610,14 @@ private:
State = IES_INTEGER;
if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
// Index Register - Register * Scale
- assert (!IndexReg && "IndexReg already set!");
+ if (IndexReg) {
+ ErrMsg = "BaseReg/IndexReg already set!";
+ return true;
+ }
IndexReg = TmpReg;
Scale = TmpInt;
- if(Scale != 1 && Scale != 2 && Scale != 4 && Scale != 8) {
- ErrMsg = "scale factor in address must be 1, 2, 4 or 8";
+ if (checkScale(Scale, ErrMsg))
return true;
- }
// Get the scale and replace the 'Register * Scale' with '0'.
IC.popOperator();
} else {
@@ -885,8 +913,8 @@ static unsigned MatchRegisterName(String
/// }
-static bool CheckBaseRegAndIndexReg(unsigned BaseReg, unsigned IndexReg,
- StringRef &ErrMsg) {
+static bool CheckBaseRegAndIndexRegAndScale(unsigned BaseReg, unsigned IndexReg,
+ unsigned Scale, StringRef &ErrMsg) {
// If we have both a base register and an index register make sure they are
// both 64-bit or 32-bit registers.
// To support VSIB, IndexReg can be 128-bit or 256-bit registers.
@@ -925,7 +953,7 @@ static bool CheckBaseRegAndIndexReg(unsi
}
}
}
- return false;
+ return checkScale(Scale, ErrMsg);
}
bool X86AsmParser::ParseRegister(unsigned &RegNo,
@@ -1348,6 +1376,7 @@ bool X86AsmParser::ParseIntelNamedOperat
bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
MCAsmParser &Parser = getParser();
const AsmToken &Tok = Parser.getTok();
+ StringRef ErrMsg;
AsmToken::TokenKind PrevTK = AsmToken::Error;
bool Done = false;
@@ -1386,7 +1415,8 @@ bool X86AsmParser::ParseIntelExpression(
StringRef Identifier = Tok.getString();
UpdateLocLex = false;
if (TK != AsmToken::String && !ParseRegister(TmpReg, IdentLoc, End)) {
- SM.onRegister(TmpReg);
+ if (SM.onRegister(TmpReg, ErrMsg))
+ return Error(Tok.getLoc(), ErrMsg);
} else if (ParseIntelNamedOperator(Identifier, SM)) {
UpdateLocLex = true;
} else if (!isParsingInlineAsm()) {
@@ -1400,7 +1430,6 @@ bool X86AsmParser::ParseIntelExpression(
int64_t Val = ParseIntelOperator(OpKind,SM.getAddImmPrefix());
if (!Val)
return true;
- StringRef ErrMsg;
if (SM.onInteger(Val, ErrMsg))
return Error(IdentLoc, ErrMsg);
} else if (Identifier.find('.') != StringRef::npos &&
@@ -1443,7 +1472,6 @@ bool X86AsmParser::ParseIntelExpression(
break;
}
case AsmToken::Integer: {
- StringRef ErrMsg;
if (isParsingInlineAsm() && SM.getAddImmPrefix())
InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Tok.getLoc());
// Look for 'b' or 'f' following an Integer as a directional label
@@ -1474,8 +1502,14 @@ bool X86AsmParser::ParseIntelExpression(
}
break;
}
- case AsmToken::Plus: SM.onPlus(); break;
- case AsmToken::Minus: SM.onMinus(); break;
+ case AsmToken::Plus:
+ if (SM.onPlus(ErrMsg))
+ return Error(getTok().getLoc(), ErrMsg);
+ break;
+ case AsmToken::Minus:
+ if (SM.onMinus(ErrMsg))
+ return Error(getTok().getLoc(), ErrMsg);
+ break;
case AsmToken::Tilde: SM.onNot(); break;
case AsmToken::Star: SM.onStar(); break;
case AsmToken::Slash: SM.onDivide(); break;
@@ -1582,7 +1616,7 @@ X86AsmParser::ParseIntelBracExpression(u
Start, End, Size);
}
StringRef ErrMsg;
- if (CheckBaseRegAndIndexReg(BaseReg, IndexReg, ErrMsg)) {
+ if (CheckBaseRegAndIndexRegAndScale(BaseReg, IndexReg, Scale, ErrMsg)) {
Error(StartInBrac, ErrMsg);
return nullptr;
}
@@ -2292,7 +2326,7 @@ std::unique_ptr<X86Operand> X86AsmParser
}
StringRef ErrMsg;
- if (CheckBaseRegAndIndexReg(BaseReg, IndexReg, ErrMsg)) {
+ if (CheckBaseRegAndIndexRegAndScale(BaseReg, IndexReg, Scale, ErrMsg)) {
Error(BaseLoc, ErrMsg);
return nullptr;
}
Added: llvm/trunk/test/MC/X86/intel-syntax-3.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/intel-syntax-3.s?rev=308972&view=auto
==============================================================================
--- llvm/trunk/test/MC/X86/intel-syntax-3.s (added)
+++ llvm/trunk/test/MC/X86/intel-syntax-3.s Tue Jul 25 06:05:12 2017
@@ -0,0 +1,46 @@
+// RUN: not llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s 2> %t.err
+// RUN: FileCheck --check-prefix=CHECK-STDERR < %t.err %s
+
+_test:
+// CHECK-LABEL: _test:
+// CHECK: xorl %eax, %eax
+
+ xor EAX, EAX
+ ret
+
+.set number, 8
+.global _foo
+
+.text
+ .global main
+main:
+
+// CHECK-STDERR: error: unknown token in expression
+ lea RDX, [RAX * number + RBX + _foo]
+
+// CHECK-STDERR: error: unknown token in expression
+ lea RDX, [_foo + RAX * number + RBX]
+
+// CHECK-STDERR: error: unknown token in expression
+ lea RDX, [number + RAX * number + RCX]
+
+// CHECK-STDERR: error: unknown token in expression
+ lea RDX, [_foo + RAX * number]
+
+// CHECK-STDERR: error: unknown token in expression
+ lea RDX, [_foo + RAX * number + RBX]
+
+// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
+ lea RDX, [number * RAX + RBX + _foo]
+
+// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
+ lea RDX, [_foo + number * RAX + RBX]
+
+// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
+ lea RDX, [8 + number * RAX + RCX]
+
+// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
+lea RDX, [unknown_number * RAX + RBX + _foo]
+
+// CHECK-STDERR: error: BaseReg/IndexReg already set!
+lea RDX, [4 * RAX + 27 * RBX + _pat]
Modified: llvm/trunk/test/MC/X86/intel-syntax-invalid-scale.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/intel-syntax-invalid-scale.s?rev=308972&r1=308971&r2=308972&view=diff
==============================================================================
--- llvm/trunk/test/MC/X86/intel-syntax-invalid-scale.s (original)
+++ llvm/trunk/test/MC/X86/intel-syntax-invalid-scale.s Tue Jul 25 06:05:12 2017
@@ -9,3 +9,7 @@
lea rax, [rdi + rdx*32]
// CHECK: error: scale factor in address must be 1, 2, 4 or 8
lea rax, [rdi + rdx*16]
+// CHECK: error: Scale can't be negative
+ lea rax, [rdi + rdx*-8]
+// CHECK: error: scale factor in address must be 1, 2, 4 or 8
+ lea rax, [rdi + -1*rdx]
Modified: llvm/trunk/test/MC/X86/intel-syntax.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/intel-syntax.s?rev=308972&r1=308971&r2=308972&view=diff
==============================================================================
--- llvm/trunk/test/MC/X86/intel-syntax.s (original)
+++ llvm/trunk/test/MC/X86/intel-syntax.s Tue Jul 25 06:05:12 2017
@@ -6,7 +6,50 @@ _test:
xor EAX, EAX
ret
-_main:
+.set number, 8
+.global _foo
+
+.text
+ .global main
+main:
+
+// CHECK: leaq _foo(%rbx,%rax,8), %rdx
+ lea RDX, [8 * RAX + RBX + _foo]
+
+// CHECK: leaq _foo(%rbx,%rax,8), %rdx
+ lea RDX, [_foo + 8 * RAX + RBX]
+
+// CHECK: leaq 8(%rcx,%rax,8), %rdx
+ lea RDX, [8 + RAX * 8 + RCX]
+
+// CHECK: leaq 8(%rcx,%rax,8), %rdx
+ lea RDX, [number + 8 * RAX + RCX]
+
+// CHECK: leaq _foo(,%rax,8), %rdx
+ lea RDX, [_foo + RAX * 8]
+
+// CHECK: leaq _foo(%rbx,%rax,8), %rdx
+ lea RDX, [_foo + RAX * 8 + RBX]
+
+// CHECK: leaq 8(%rax), %rdx
+ lea RDX, [RAX - number]
+// CHECK: leaq -8(%rax), %rdx
+ lea RDX, [RAX - 8]
+
+// CHECK: leaq _foo(%rax), %rdx
+ lea RDX, [RAX + _foo]
+// CHECK: leaq 8(%rax), %rdx
+ lea RDX, [RAX + number]
+// CHECK: leaq 8(%rax), %rdx
+ lea RDX, [RAX + 8]
+
+// CHECK: leaq _foo(%rax), %rdx
+ lea RDX, [_foo + RAX]
+// CHECK: leaq 8(%rax), %rdx
+ lea RDX, [number + RAX]
+// CHECK: leaq 8(%rax), %rdx
+ lea RDX, [8 + RAX]
+
// CHECK: movl $257, -4(%rsp)
mov DWORD PTR [RSP - 4], 257
// CHECK: movl $258, 4(%rsp)
More information about the llvm-commits
mailing list