[PATCH] D23312: [Thumb] Validate branch target for CBZ/CBNZ instructions.
Oliver Stannard via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 9 07:34:07 PDT 2016
olista01 added inline comments.
================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6689
@@ +6688,3 @@
+ case ARM::tCBNZ: {
+ if (!static_cast<ARMOperand &>(*Operands[2]).isUnsignedOffset<5, 1>())
+ return Error(Operands[2]->getStartLoc(), "branch target out of range");
----------------
These instructions accept a 6-bit immediate (shifted by one bit), not 5-bit. This should allow all even numbers in the range 0 to 126 (inclusive at both ends).
================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:582
@@ +581,3 @@
+ // CB instructions cannot branch backwards with a negative offset
+ if (Ctx && ((int64_t)Value < 0 || Value > 0x1f)) {
+ Ctx->reportError(Fixup.getLoc(), "out of range pc-relative fixup value");
----------------
We should also be checking that the low bit is clear (and see my comment above about the immediate being 6 bits).
================
Comment at: test/MC/ARM/thumb-diagnostics.s:241
@@ +240,3 @@
+
+ cbz r0, #-4
+ cbnz r0, #-8
----------------
It would be better to test as close to the bounds as possible, and add some tests for valid immediates close to the boundary. I think these should cover it:
-2 (invalid)
0 (lowest valid)
17 (invalid due to alignment)
126 (highest valid)
128 (invalid)
https://reviews.llvm.org/D23312
More information about the llvm-commits
mailing list