Fix #3965 (#3969)

This fixes a reachable assert(). The operands used to calculate the jump target, used whatever was in the union of the operand value field. This also means that the previous calculation of jump targets were simply incorrect. It nontheless lead to the right results because MEMDISP() seemed to be 0 all the time.
rizinorg · Nov 10, 2023 · 839411c · 839411c
1 parent eabd3e7
commit 839411c
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 9 deletions.
diff --git a/librz/analysis/fcn.c b/librz/analysis/fcn.c
@@ -1175,7 +1175,7 @@ static RzAnalysisBBEndCause run_basic_block_analysis(RzAnalysisTaskItem *item, R
 						};
 						if (op.ireg) {
 							rz_analysis_walkthrough_jmptbl(analysis, fcn, bb, &params);
-						} else { // op.reg
+						} else if (RZ_STR_EQ(analysis->arch_target->arch, "arm")) {
 							rz_analysis_walkthrough_arm_jmptbl_style(analysis, fcn, bb, &params);
 						}
 						// check if op.jump and op.fail contain jump table location

diff --git a/librz/analysis/p/analysis_arm_cs.c b/librz/analysis/p/analysis_arm_cs.c
@@ -55,10 +55,10 @@ inline static const char *ARMCondCodeToString(arm_cc cc) {
 #endif
 
 typedef struct arm_cs_context_t {
-	RzArmITContext it;
-	csh handle;
-	int omode;
-	int obits;
+	RzArmITContext it; ///< Save IT values between instruction disassembly.
+	csh handle; ///< The Capstone handle used.
+	int omode; ///< Capstone mode flags.
+	int obits; ///< Architecture bits.
 } ArmCSContext;
 
 static const char *shift_type_name(arm_shifter type) {
@@ -1210,13 +1210,16 @@ jmp $$ + 4 + ( [delta] * 2 )
 		if (REGID(0) == ARM_REG_PC) {
 			op->type = RZ_ANALYSIS_OP_TYPE_UJMP;
 			if (REGID(1) == ARM_REG_PC && insn->detail->arm.cc != CS_ARMCC(AL)) {
-				// op->type = RZ_ANALYSIS_OP_TYPE_RCJMP;
 				op->type = RZ_ANALYSIS_OP_TYPE_UCJMP;
 				op->fail = addr + op->size;
-				op->jump = ((addr & ~3LL) + (thumb ? 4 : 8) + MEMDISP(1)) & UT64_MAX;
-				op->ptr = (addr & ~3LL) + (thumb ? 4 : 8) + MEMDISP(1);
 				op->refptr = 4;
-				op->reg = rz_str_get_null(cs_reg_name(handle, INSOP(2).reg));
+				op->reg = rz_str_get_null(cs_reg_name(handle, INSOP(0).reg));
+				// add pc, pc, <offset|ireg> is considered a jump table start.
+				// op->ptr points to the start of the table.
+				op->ptr = (addr & ~3LL) + (thumb ? 4 : 8);
+				if (ISIMM(2)) {
+					op->jump = ((addr & ~3LL) + (thumb ? 4 : 8) + INSOP(2).imm) & UT64_MAX;
+				}
 				break;
 			}
 		}

diff --git a/test/db/analysis/arm b/test/db/analysis/arm
@@ -1239,3 +1239,45 @@ EXPECT=<<EOF
 94
 EOF
 RUN
+
+NAME=add/adc pc, pc issue #3965
+FILE==
+ARGS=-a arm -b 32 -e cfg.bigendian=false
+CMDS=<<EOF
+wx ffff8f024fffaf02
+pd 2
+aa
+ags
+EOF
+EXPECT=<<EOF
+            0x00000000      addeq pc, pc, 0x3fc                        ; [0x8:4]=0
+            0x00000004      adceq pc, pc, 0x13c                        ; [0xc:4]=0
+        .-----------.
+        |  0x0      |
+        `-----------'
+              t f
+              | |
+    .---------' |
+    |           '---.
+    |               |
+.----------.    .-----------.
+|  0x404   |    |  0x4      |
+`----------'    `-----------'
+                        f t
+                        | |
+                        | |
+              .---------' |
+              |           |
+          .----------.    |
+          |  0x8     |    |
+          `----------'    |
+              v           |
+              |           |
+              '------.    |
+                     | .--'
+                     | |
+               .-----------.
+               |  0x148    |
+               `-----------'
+EOF
+RUN