Fix off-by-one errors in limit checking for ldr/str instructions.
The LDR/STR encoder in the thumb assembler had an off-by-one
error for limit checking for immediates. This resulted in an
assertion failure for things like 'ldr rx,[ry,#128]'
Bug: 15876206
Change-Id: Ic866212e2feae94e0bd4c753724898d84f5cb944
diff --git a/compiler/utils/arm/assembler_thumb2.cc b/compiler/utils/arm/assembler_thumb2.cc
index 30aa625..604f59e 100644
--- a/compiler/utils/arm/assembler_thumb2.cc
+++ b/compiler/utils/arm/assembler_thumb2.cc
@@ -1260,23 +1260,23 @@
int32_t offset = ad.GetOffset();
// The 16 bit SP relative instruction can only have a 10 bit offset.
- if (rn == SP && offset > 1024) {
+ if (rn == SP && offset >= (1 << 10)) {
must_be_32bit = true;
}
if (byte) {
// 5 bit offset, no shift.
- if (offset > 32) {
+ if (offset >= (1 << 5)) {
must_be_32bit = true;
}
} else if (half) {
// 6 bit offset, shifted by 1.
- if (offset > 64) {
+ if (offset >= (1 << 6)) {
must_be_32bit = true;
}
} else {
// 7 bit offset, shifted by 2.
- if (offset > 128) {
+ if (offset >= (1 << 7)) {
must_be_32bit = true;
}
}
@@ -1312,7 +1312,7 @@
CHECK_GE(offset, 0);
if (sp_relative) {
// SP relative, 10 bit offset.
- CHECK_LT(offset, 1024);
+ CHECK_LT(offset, (1 << 10));
CHECK_EQ((offset & 0b11), 0);
encoding |= rd << 8 | offset >> 2;
} else {
@@ -1322,15 +1322,15 @@
if (byte) {
// 5 bit offset, no shift.
- CHECK_LT(offset, 32);
+ CHECK_LT(offset, (1 << 5));
} else if (half) {
// 6 bit offset, shifted by 1.
- CHECK_LT(offset, 64);
+ CHECK_LT(offset, (1 << 6));
CHECK_EQ((offset & 0b1), 0);
offset >>= 1;
} else {
// 7 bit offset, shifted by 2.
- CHECK_LT(offset, 128);
+ CHECK_LT(offset, (1 << 7));
CHECK_EQ((offset & 0b11), 0);
offset >>= 2;
}
@@ -1344,7 +1344,7 @@
if (ad.GetRegister() == PC) {
// PC relative literal encoding.
int32_t offset = ad.GetOffset();
- if (must_be_32bit || offset < 0 || offset > (1 << 10) || !load) {
+ if (must_be_32bit || offset < 0 || offset >= (1 << 10) || !load) {
int32_t up = B23;
if (offset < 0) {
offset = -offset;
diff --git a/compiler/utils/assembler_thumb_test.cc b/compiler/utils/assembler_thumb_test.cc
index 1e3e569..68cb656 100644
--- a/compiler/utils/assembler_thumb_test.cc
+++ b/compiler/utils/assembler_thumb_test.cc
@@ -1358,6 +1358,41 @@
delete assembler;
}
+TEST(Thumb2AssemblerTest, LoadStoreLimits) {
+ arm::Thumb2Assembler* assembler = static_cast<arm::Thumb2Assembler*>(Assembler::Create(kThumb2));
+
+ __ ldr(R0, Address(R4, 124)); // 16 bit.
+ __ ldr(R0, Address(R4, 128)); // 32 bit.
+
+ __ ldrb(R0, Address(R4, 31)); // 16 bit.
+ __ ldrb(R0, Address(R4, 32)); // 32 bit.
+
+ __ ldrh(R0, Address(R4, 62)); // 16 bit.
+ __ ldrh(R0, Address(R4, 64)); // 32 bit.
+
+ __ ldrsb(R0, Address(R4, 31)); // 32 bit.
+ __ ldrsb(R0, Address(R4, 32)); // 32 bit.
+
+ __ ldrsh(R0, Address(R4, 62)); // 32 bit.
+ __ ldrsh(R0, Address(R4, 64)); // 32 bit.
+
+ __ str(R0, Address(R4, 124)); // 16 bit.
+ __ str(R0, Address(R4, 128)); // 32 bit.
+
+ __ strb(R0, Address(R4, 31)); // 16 bit.
+ __ strb(R0, Address(R4, 32)); // 32 bit.
+
+ __ strh(R0, Address(R4, 62)); // 16 bit.
+ __ strh(R0, Address(R4, 64)); // 32 bit.
+
+ size_t cs = __ CodeSize();
+ std::vector<uint8_t> managed_code(cs);
+ MemoryRegion code(&managed_code[0], managed_code.size());
+ __ FinalizeInstructions(code);
+ dump(managed_code, "LoadStoreLimits");
+ delete assembler;
+}
+
#undef __
} // namespace arm
} // namespace art
diff --git a/compiler/utils/assembler_thumb_test_expected.cc.inc b/compiler/utils/assembler_thumb_test_expected.cc.inc
index c2e7fe8..3943e37 100644
--- a/compiler/utils/assembler_thumb_test_expected.cc.inc
+++ b/compiler/utils/assembler_thumb_test_expected.cc.inc
@@ -4799,6 +4799,25 @@
" 18: f8cf 07ff str.w r0, [pc, #2047] ; 81b <LoadStoreLiteral+0x81b>\n",
nullptr
};
+const char* LoadStoreLimitsResults[] = {
+ " 0: 6fe0 ldr r0, [r4, #124] ; 0x7c\n",
+ " 2: f8d4 0080 ldr.w r0, [r4, #128] ; 0x80\n",
+ " 6: 7fe0 ldrb r0, [r4, #31]\n",
+ " 8: f894 0020 ldrb.w r0, [r4, #32]\n",
+ " c: 8fe0 ldrh r0, [r4, #62] ; 0x3e\n",
+ " e: f8b4 0040 ldrh.w r0, [r4, #64] ; 0x40\n",
+ " 12: f994 001f ldrsb.w r0, [r4, #31]\n",
+ " 16: f994 0020 ldrsb.w r0, [r4, #32]\n",
+ " 1a: f9b4 003e ldrsh.w r0, [r4, #62] ; 0x3e\n",
+ " 1e: f9b4 0040 ldrsh.w r0, [r4, #64] ; 0x40\n",
+ " 22: 67e0 str r0, [r4, #124] ; 0x7c\n",
+ " 24: f8c4 0080 str.w r0, [r4, #128] ; 0x80\n",
+ " 28: 77e0 strb r0, [r4, #31]\n",
+ " 2a: f884 0020 strb.w r0, [r4, #32]\n",
+ " 2e: 87e0 strh r0, [r4, #62] ; 0x3e\n",
+ " 30: f8a4 0040 strh.w r0, [r4, #64] ; 0x40\n",
+ nullptr
+};
std::map<std::string, const char**> test_results;
void setup_results() {
test_results["SimpleMov"] = SimpleMovResults;
@@ -4845,4 +4864,5 @@
test_results["Shifts"] = ShiftsResults;
test_results["LoadStoreRegOffset"] = LoadStoreRegOffsetResults;
test_results["LoadStoreLiteral"] = LoadStoreLiteralResults;
+ test_results["LoadStoreLimits"] = LoadStoreLimitsResults;
}