[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