[llvm] [MC][ARM] Fix crash when assembling Thumb 'movs r0, #foo'. (PR #115026)
Eli Friedman via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 14:41:29 PST 2024
================
@@ -8653,6 +8657,41 @@ bool ARMAsmParser::validateInstruction(MCInst &Inst,
return false;
}
+// After processInstruction has transformed an instruction being assembled into
+// a different opcode, do any further validity checks that the new opcode
+// depends on.
+//
+// `Inst` contains the final modified form of the instruction, but `Operands`
+// contains the parsed operands from the _original_ instruction, because
+// nothing has updated them (`processInstruction` received them as const).
+// `OrigOpcode` contains the original value of `Inst.getOpcode()`, which should
+// give enough context to know how to understand the original operands list.
+bool ARMAsmParser::revalidateInstruction(MCInst &Inst,
+ const OperandVector &Operands,
+ unsigned MnemonicOpsEndInd,
+ unsigned OrigOpcode) {
+ const unsigned NewOpcode = Inst.getOpcode();
+
+ if (OrigOpcode == ARM::t2ADDri && NewOpcode == ARM::tADDi8) {
+ // t2ADDri is the Thumb 32-bit immediate add instruction, for example
+ // 'add[s] r0,r1,#0xff00'. If its immediate argument isn't a constant
+ // requiring shifting, then processInstruction can turn it into tADDi8, the
+ // simpler 16-bit Thumb immediate add (provided all the other conditions
+ // for that transformation are met). That makes it too late for
+ // validateInstruction to do this check, which it would have done if it had
+ // known from the start that the instruction was tADDi8.
----------------
efriedma-quic wrote:
The important things here are:
- We don't crash
- If an instruction is encodable in Thumb1 mode, it also has the same encoding in Thumb2 mode.
If we have to reject certain cases because they would introduce some ambiguity, that's fine, especially if there's some easy way for the user to rewrite their code.
Given that, current version seems okay. Maybe we should do relaxation as part of processing fixups, but that's obviously a much bigger change.
https://github.com/llvm/llvm-project/pull/115026
More information about the llvm-commits
mailing list