[llvm] [X86] Invalid assembly given inverted meaning (PR #190460)
Takashi Idobe via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 4 06:29:39 PDT 2026
https://github.com/Takashiidobe created https://github.com/llvm/llvm-project/pull/190460
Relates: https://github.com/llvm/llvm-project/issues/96427
The issue shows that `lea rax, [rax - 8 * rdx]` would be assembled as `lea rax, [rax + 8 * rdx]`. This is invalid according to binutils: https://godbolt.org/z/z6jrzasYv.
The first commit fixes the case in the issue by rejecting `[rax - 8 * rdx]` completely with an error message, and has the caret point to the minus sign for the diagnostic.
So for that case we get:
```
test.s:2:15: error: scale factor in address cannot be negative
lea rax, [rax - 8 * rdx]
^
```
Instead of
```
leaq (%rax,%rdx,8), %rax # which is incorrectly assembled
```
There are two extra cases I fixed the diagnostic for:
```
test.s:4:21: error: unknown token in expression
lea rax, [rax - rdx * 8]
^
test.s:5:20: error: unknown token in expression
lea rax, [rax - rdx]
^
```
The first case doesn't make sense to me (why is the * an invalid token in a lea? You can change this to:
```
lea rax, [rax + rdx * 8]
```
and now * is valid. So is the * valid or not? Or is it the - sign?
I changed the error message to point to the - sign since I think that gives you more information about what's actually invalid here.
```
test.s:4:15: error: scale factor in address cannot be negative
lea rax, [rax - rdx * 8]
^
```
For the second case:
```
test.s:5:20: error: unknown token in expression
lea rax, [rax - rdx]
^
```
The right bracket being unknown is bizarre to say the least. binutils' default "invalid use of register" is much better, but we can do one better since the - sign is incorrect here and should be a + sign so I changed it to:
```
test.s:5:15: error: scale factor in address cannot be negative
lea rax, [rax - rdx]
^
```
I'm not too happy about how the code looks but other error diagnostics to special case had the same shape so I followed convention here.
>From 59bd8e613b1d1ada47e9fd60e5ad02fc9e3c91ca Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Fri, 3 Apr 2026 16:13:31 -0400
Subject: [PATCH 1/3] Fix case of scale * register and scale * register with an
existing index being accepted by llvm-mc
---
.../lib/Target/X86/AsmParser/X86AsmParser.cpp | 65 ++++++++++++++-----
1 file changed, 49 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 9d8b74284a3de..9d42b7ed9d42f 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -453,6 +453,8 @@ class X86AsmParser : public MCTargetAsmParser {
short BracCount = 0;
bool MemExpr = false;
bool BracketUsed = false;
+ bool NegativeAdditiveTerm = false;
+ SMLoc NegativeAdditiveTermLoc;
bool OffsetOperator = false;
bool AttachToOperandIdx = false;
bool IsPIC = false;
@@ -504,6 +506,11 @@ class X86AsmParser : public MCTargetAsmParser {
void setPIC() { IsPIC = true; }
bool hadError() const { return State == IES_ERROR; }
+ SMLoc getLocForNegativeScaleError(StringRef ErrMsg, SMLoc DefaultLoc) const {
+ return ErrMsg == "scale factor in address cannot be negative"
+ ? NegativeAdditiveTermLoc
+ : DefaultLoc;
+ }
const InlineAsmIdentifierInfo &getIdentifierInfo() const { return Info; }
bool regsUseUpError(StringRef &ErrMsg) {
@@ -693,6 +700,8 @@ class X86AsmParser : public MCTargetAsmParser {
case IES_OFFSET:
State = IES_PLUS;
IC.pushOperator(IC_PLUS);
+ NegativeAdditiveTerm = false;
+ NegativeAdditiveTermLoc = SMLoc();
if (CurrState == IES_REGISTER && PrevState != IES_MULTIPLY) {
// If we already have a BaseReg, then assume this is the IndexReg with
// no explicit scale.
@@ -710,7 +719,7 @@ class X86AsmParser : public MCTargetAsmParser {
PrevState = CurrState;
return false;
}
- bool onMinus(StringRef &ErrMsg) {
+ bool onMinus(SMLoc MinusLoc, StringRef &ErrMsg) {
IntelExprState CurrState = State;
switch (State) {
default:
@@ -744,8 +753,11 @@ class X86AsmParser : public MCTargetAsmParser {
// push minus operator if it is not a negate operator
if (CurrState == IES_REGISTER || CurrState == IES_RPAREN ||
CurrState == IES_INTEGER || CurrState == IES_RBRAC ||
- CurrState == IES_OFFSET)
+ CurrState == IES_OFFSET) {
IC.pushOperator(IC_MINUS);
+ NegativeAdditiveTerm = true;
+ NegativeAdditiveTermLoc = MinusLoc;
+ }
else if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
// We have negate operator for Scale: it's illegal
ErrMsg = "Scale can't be negative";
@@ -819,6 +831,10 @@ class X86AsmParser : public MCTargetAsmParser {
if (PrevState == IES_INTEGER) {
if (IndexReg)
return regsUseUpError(ErrMsg);
+ if (NegativeAdditiveTerm) {
+ ErrMsg = "scale factor in address cannot be negative";
+ return true;
+ }
State = IES_REGISTER;
IndexReg = Reg;
// Get the scale and replace the 'Scale * Register' with '0'.
@@ -901,6 +917,10 @@ class X86AsmParser : public MCTargetAsmParser {
// Index Register - Register * Scale
if (IndexReg)
return regsUseUpError(ErrMsg);
+ if (NegativeAdditiveTerm) {
+ ErrMsg = "scale factor in address cannot be negative";
+ return true;
+ }
IndexReg = TmpReg;
Scale = TmpInt;
if (checkScale(Scale, ErrMsg))
@@ -1969,7 +1989,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
if (!Val->evaluateAsAbsolute(Res, getStreamer().getAssemblerPtr()))
return Error(ValueLoc, "expected absolute value");
if (SM.onInteger(Res, ErrMsg))
- return Error(ValueLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, ValueLoc),
+ ErrMsg);
break;
}
[[fallthrough]];
@@ -2016,7 +2037,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
if (Tok.is(AsmToken::Identifier)) {
if (!ParseRegister(Reg, IdentLoc, End, /*RestoreOnFailure=*/true)) {
if (SM.onRegister(Reg, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
break;
}
if (Parser.isParsingMasm()) {
@@ -2027,7 +2049,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
if (!Field.empty() &&
!MatchRegisterByName(Reg, ID, IdentLoc, IDEndLoc)) {
if (SM.onRegister(Reg, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
AsmFieldInfo Info;
SMLoc FieldStartLoc = SMLoc::getFromPointer(Field.data());
@@ -2036,7 +2059,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
else if (SM.onPlus(ErrMsg))
return Error(getTok().getLoc(), ErrMsg);
else if (SM.onInteger(Info.Offset, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
SM.setTypeInfo(Info.Type);
End = consumeToken();
@@ -2075,7 +2099,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
if (unsigned OpKind = IdentifyIntelInlineAsmOperator(Identifier)) {
if (int64_t Val = ParseIntelInlineAsmOperator(OpKind)) {
if (SM.onInteger(Val, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
} else {
return true;
}
@@ -2089,7 +2114,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
return true;
else if (SM.onIdentifierExpr(Val, Identifier, Info, FieldInfo.Type,
true, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
break;
}
if (Parser.isParsingMasm()) {
@@ -2098,7 +2124,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
if (ParseMasmOperator(OpKind, Val))
return true;
if (SM.onInteger(Val, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
break;
}
if (!getParser().lookUpType(Identifier, FieldInfo.Type)) {
@@ -2122,7 +2149,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
EndDot = parseOptionalToken(AsmToken::Dot);
}
if (SM.onInteger(FieldInfo.Offset, ErrMsg))
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
break;
}
}
@@ -2130,7 +2158,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
return Error(Tok.getLoc(), "Unexpected identifier!");
} else if (SM.onIdentifierExpr(Val, Identifier, Info, FieldInfo.Type,
false, ErrMsg)) {
- return Error(IdentLoc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, IdentLoc),
+ ErrMsg);
}
break;
}
@@ -2155,15 +2184,18 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
AsmTypeInfo Type;
if (SM.onIdentifierExpr(Val, Identifier, Info, Type,
isParsingMSInlineAsm(), ErrMsg))
- return Error(Loc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, Loc),
+ ErrMsg);
End = consumeToken();
} else {
if (SM.onInteger(IntVal, ErrMsg))
- return Error(Loc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, Loc),
+ ErrMsg);
}
} else {
if (SM.onInteger(IntVal, ErrMsg))
- return Error(Loc, ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, Loc),
+ ErrMsg);
}
break;
}
@@ -2172,8 +2204,9 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
return Error(getTok().getLoc(), ErrMsg);
break;
case AsmToken::Minus:
- if (SM.onMinus(ErrMsg))
- return Error(getTok().getLoc(), ErrMsg);
+ if (SM.onMinus(getTok().getLoc(), ErrMsg))
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, getTok().getLoc()),
+ ErrMsg);
break;
case AsmToken::Tilde: SM.onNot(); break;
case AsmToken::Star: SM.onStar(); break;
>From 62aee3a3013a2969653d232baaaed80ce13a484e Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Sat, 4 Apr 2026 08:42:31 -0400
Subject: [PATCH 2/3] add extra cases, register * scale and reg - reg cases
which should also be rejected
---
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp | 17 +++++++++++++----
.../X86/inline-asm-intel-negative-scale.ll | 14 ++++++++++++++
llvm/test/MC/X86/intel-syntax-invalid-scale.s | 10 ++++++++++
3 files changed, 37 insertions(+), 4 deletions(-)
create mode 100644 llvm/test/CodeGen/X86/inline-asm-intel-negative-scale.ll
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 9d42b7ed9d42f..68dc21f872a5b 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -820,6 +820,7 @@ class X86AsmParser : public MCTargetAsmParser {
State = IES_ERROR;
break;
case IES_PLUS:
+ case IES_MINUS:
case IES_LPAREN:
case IES_LBRAC:
State = IES_REGISTER;
@@ -829,12 +830,12 @@ class X86AsmParser : public MCTargetAsmParser {
case IES_MULTIPLY:
// Index Register - Scale * Register
if (PrevState == IES_INTEGER) {
- if (IndexReg)
- return regsUseUpError(ErrMsg);
if (NegativeAdditiveTerm) {
ErrMsg = "scale factor in address cannot be negative";
return true;
}
+ if (IndexReg)
+ return regsUseUpError(ErrMsg);
State = IES_REGISTER;
IndexReg = Reg;
// Get the scale and replace the 'Scale * Register' with '0'.
@@ -915,12 +916,12 @@ class X86AsmParser : public MCTargetAsmParser {
State = IES_INTEGER;
if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
// Index Register - Register * Scale
- if (IndexReg)
- return regsUseUpError(ErrMsg);
if (NegativeAdditiveTerm) {
ErrMsg = "scale factor in address cannot be negative";
return true;
}
+ if (IndexReg)
+ return regsUseUpError(ErrMsg);
IndexReg = TmpReg;
Scale = TmpInt;
if (checkScale(Scale, ErrMsg))
@@ -1025,6 +1026,10 @@ class X86AsmParser : public MCTargetAsmParser {
} else {
if (IndexReg)
return regsUseUpError(ErrMsg);
+ if (NegativeAdditiveTerm) {
+ ErrMsg = "scale factor in address cannot be negative";
+ return true;
+ }
IndexReg = TmpReg;
Scale = 0;
}
@@ -1090,6 +1095,10 @@ class X86AsmParser : public MCTargetAsmParser {
} else {
if (IndexReg)
return regsUseUpError(ErrMsg);
+ if (NegativeAdditiveTerm) {
+ ErrMsg = "scale factor in address cannot be negative";
+ return true;
+ }
IndexReg = TmpReg;
Scale = 0;
}
diff --git a/llvm/test/CodeGen/X86/inline-asm-intel-negative-scale.ll b/llvm/test/CodeGen/X86/inline-asm-intel-negative-scale.ll
new file mode 100644
index 0000000000000..6f6f758b21241
--- /dev/null
+++ b/llvm/test/CodeGen/X86/inline-asm-intel-negative-scale.ll
@@ -0,0 +1,14 @@
+; RUN: not llc -mtriple=x86_64-unknown-linux-gnu %s -o /dev/null 2>&1 | FileCheck %s
+
+; Verify that inline asm with inteldialect correctly rejects a negative scale
+; factor in an address expression (e.g. [rax - 8 * rdx]) instead of silently
+; inverting the sign and producing [rax + 8*rdx].
+
+; CHECK: error: scale factor in address cannot be negative
+
+define i64 @test_neg_scale(i64 %0) {
+ %result = call i64 asm sideeffect alignstack inteldialect
+ "xor rax, rax\0Alea rax, [rax - 8 * rdx]",
+ "=&{ax},{dx},~{dirflag},~{fpsr},~{flags},~{memory}"(i64 %0)
+ ret i64 %result
+}
diff --git a/llvm/test/MC/X86/intel-syntax-invalid-scale.s b/llvm/test/MC/X86/intel-syntax-invalid-scale.s
index 702f08a2f176f..225d33c324ca3 100644
--- a/llvm/test/MC/X86/intel-syntax-invalid-scale.s
+++ b/llvm/test/MC/X86/intel-syntax-invalid-scale.s
@@ -13,3 +13,13 @@
lea rax, [rdi + rdx*-8]
// CHECK: error: scale factor in address must be 1, 2, 4 or 8
lea rax, [rdi + -1*rdx]
+// CHECK: error: scale factor in address cannot be negative
+ lea rax, [rax - 8 * rdx]
+// CHECK: error: scale factor in address cannot be negative
+ lea rax, [rax - 2 * rdx]
+// CHECK: error: scale factor in address cannot be negative
+ lea rax, [rdx * 4 + rax - 2 * rcx]
+// CHECK: error: scale factor in address cannot be negative
+ lea rax, [rax - rdx * 8]
+// CHECK: error: scale factor in address cannot be negative
+ lea rax, [rax - rdx]
>From 970e1db8e4cfde9ee00ebba58df6199f38877c03 Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Sat, 4 Apr 2026 09:27:18 -0400
Subject: [PATCH 3/3] fix last test case and add caret checks
---
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp | 11 ++++++-----
llvm/test/MC/X86/intel-syntax-invalid-scale.s | 18 ++++++++++++------
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 68dc21f872a5b..4a1b899e4cee9 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -830,12 +830,12 @@ class X86AsmParser : public MCTargetAsmParser {
case IES_MULTIPLY:
// Index Register - Scale * Register
if (PrevState == IES_INTEGER) {
+ if (IndexReg)
+ return regsUseUpError(ErrMsg);
if (NegativeAdditiveTerm) {
ErrMsg = "scale factor in address cannot be negative";
return true;
}
- if (IndexReg)
- return regsUseUpError(ErrMsg);
State = IES_REGISTER;
IndexReg = Reg;
// Get the scale and replace the 'Scale * Register' with '0'.
@@ -916,12 +916,12 @@ class X86AsmParser : public MCTargetAsmParser {
State = IES_INTEGER;
if (PrevState == IES_REGISTER && CurrState == IES_MULTIPLY) {
// Index Register - Register * Scale
+ if (IndexReg)
+ return regsUseUpError(ErrMsg);
if (NegativeAdditiveTerm) {
ErrMsg = "scale factor in address cannot be negative";
return true;
}
- if (IndexReg)
- return regsUseUpError(ErrMsg);
IndexReg = TmpReg;
Scale = TmpInt;
if (checkScale(Scale, ErrMsg))
@@ -2235,7 +2235,8 @@ bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
break;
case AsmToken::RBrac:
if (SM.onRBrac(ErrMsg)) {
- return Error(Tok.getLoc(), ErrMsg);
+ return Error(SM.getLocForNegativeScaleError(ErrMsg, Tok.getLoc()),
+ ErrMsg);
}
break;
case AsmToken::LParen: SM.onLParen(); break;
diff --git a/llvm/test/MC/X86/intel-syntax-invalid-scale.s b/llvm/test/MC/X86/intel-syntax-invalid-scale.s
index 225d33c324ca3..03880a4fd1f2d 100644
--- a/llvm/test/MC/X86/intel-syntax-invalid-scale.s
+++ b/llvm/test/MC/X86/intel-syntax-invalid-scale.s
@@ -13,13 +13,19 @@
lea rax, [rdi + rdx*-8]
// CHECK: error: scale factor in address must be 1, 2, 4 or 8
lea rax, [rdi + -1*rdx]
-// CHECK: error: scale factor in address cannot be negative
+// CHECK: [[#@LINE+3]]:19: error: scale factor in address cannot be negative
+// CHECK-NEXT: lea rax, [rax - 8 * rdx]
+// CHECK-NEXT: ^
lea rax, [rax - 8 * rdx]
-// CHECK: error: scale factor in address cannot be negative
+// CHECK: [[#@LINE+3]]:19: error: scale factor in address cannot be negative
+// CHECK-NEXT: lea rax, [rax - 2 * rdx]
+// CHECK-NEXT: ^
lea rax, [rax - 2 * rdx]
-// CHECK: error: scale factor in address cannot be negative
- lea rax, [rdx * 4 + rax - 2 * rcx]
-// CHECK: error: scale factor in address cannot be negative
+// CHECK: [[#@LINE+3]]:19: error: scale factor in address cannot be negative
+// CHECK-NEXT: lea rax, [rax - rdx * 8]
+// CHECK-NEXT: ^
lea rax, [rax - rdx * 8]
-// CHECK: error: scale factor in address cannot be negative
+// CHECK: [[#@LINE+3]]:19: error: scale factor in address cannot be negative
+// CHECK-NEXT: lea rax, [rax - rdx]
+// CHECK-NEXT: ^
lea rax, [rax - rdx]
More information about the llvm-commits
mailing list