-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Mips] Fix clang crashes when assembling invalid MIPS beql instructions with --arch=mips #156413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-backend-mips Author: None (yingopq) ChangesFrom clang version 4, mips append new instruction BeqImm and BEQLImmMacro, the second operand of instruction format is imm64:$imm. When Mips process Fix #151453. Full diff: https://github.com/llvm/llvm-project/pull/156413.diff 1 Files Affected:
diff --git a/llvm/lib/Target/Mips/MipsInstrInfo.td b/llvm/lib/Target/Mips/MipsInstrInfo.td
index a124e84e9ca5f..0d6fa25fb8025 100644
--- a/llvm/lib/Target/Mips/MipsInstrInfo.td
+++ b/llvm/lib/Target/Mips/MipsInstrInfo.td
@@ -855,7 +855,15 @@ def calltarget : Operand<iPTR> {
let PrintMethod = "printJumpOperand";
}
-def imm64: Operand<i64>;
+def ConstantImmAsmOperandClass : AsmOperandClass {
+ let Name = "ConstantImm";
+ let PredicateMethod = "isConstantImm";
+ let RenderMethod = "addImmOperands";
+}
+
+def imm64: Operand<i64> {
+ let ParserMatchClass = ConstantImmAsmOperandClass;
+}
def simm19_lsl2 : Operand<i32> {
let EncoderMethod = "getSimm19Lsl2Encoding";
|
3d53abf
to
4cfe280
Compare
4cfe280
to
5c8d18d
Compare
5c8d18d
to
5afae40
Compare
@arsenm Can you help review this pr? This issue has been tested OK in the comments section of the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test
5afae40
to
4928c42
Compare
# RUN: not llvm-mc %s -triple=mips -mcpu=mips32 2>&1 | FileCheck %s | ||
|
||
# CHECK: error: invalid operand for instruction | ||
beql $t0, ($t0), 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changed at least 2 opcodes, right? Should test all of them (missing bne?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
…ns with --arch=mips From clang version 4, mips append new instruction BeqImm and BEQLImm, the second operand format of instruction is imm64:$imm. 1.When Mips process `beql $t0, ($t0), 1`, it think the second operand was an imm, so match success. Then mips backend process expandBranchImm, check the second operand `$t0` was not imm, reported asserts. We can strengthen the second operand matching restrictions. 2.Similarly, when Mips process `beql $t0, (1), 1`, it think the second was an imm. so match success. Then mips backend process expandBranchImm, check the third operand `1` was not expression, reported asserts. Permit the third operand of `beql` to be imm. Fix llvm#151453.
4928c42
to
9fd2966
Compare
From clang version 4, mips append new instruction BeqImm and BEQLImmMacro, the second operand of instruction format is imm64:$imm. When Mips process
beql $t0, ($t0), 1
, it think the second operand was an imm, so match success. Then mips backend process expandBranchImm, check the Operand(1) was not imm, reported failure.We can strengthen the instruction matching restrictions.
Fixes #151453.