Skip to content

Commit 8cdfe4c

Browse files
hainesthainest
authored
Add instruction details to cstest logging (#2787)
* Update cstest.py logging * Update cstest.c logging * Change exception to log+return in TestExpected.compare The prior change in TestCase.test converts the exception to TestResult.FAILED which doesn't preserve the status here. * Restore calling sequence in TestCase.test The transformation didn't preserve error reporting. It also is unnecessary now that TestExpected.compare returns a TestResult again. * Convert exception to return for length check in TestExpected.compare The callsite in TestCase.test no longer converts exceptions to a TestResult. * Rename local variable 'postfix' to 'prefix' in test_input_stringify This better reflects what it actually is.
1 parent ddf472b commit 8cdfe4c

File tree

2 files changed

+75
-46
lines changed

2 files changed

+75
-46
lines changed

‎bindings/python/cstest_py/src/cstest_py/cstest.py‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -234,37 +234,31 @@ def compare(self, actual_insns: list[CsInsn], bits: int) -> TestResult:
234234
f"{len(actual_insns)} != {len(self.insns):#x}"
235235
)
236236
return TestResult.FAILED
237+
237238
for a_insn, e_insn in zip(actual_insns, self.insns):
238-
if not compare_asm_text(
239-
a_insn,
240-
e_insn.get("asm_text"),
241-
bits,
242-
):
243-
return TestResult.FAILED
239+
def _check(res: bool):
240+
if not res:
241+
log.error(f"Failed instruction: {a_insn}")
242+
return TestResult.FAILED
243+
244+
_check(compare_asm_text(a_insn, e_insn.get("asm_text"), bits))
245+
246+
_check(compare_str(a_insn.mnemonic, e_insn.get("mnemonic"), "mnemonic"))
244247

245-
if not compare_str(a_insn.mnemonic, e_insn.get("mnemonic"), "mnemonic"):
246-
return TestResult.FAILED
248+
_check(compare_str(a_insn.op_str, e_insn.get("op_str"), "op_str"))
247249

248-
if not compare_str(a_insn.op_str, e_insn.get("op_str"), "op_str"):
249-
return TestResult.FAILED
250+
_check(compare_enum(a_insn.id, e_insn.get("id"), "id"))
250251

251-
if not compare_enum(a_insn.id, e_insn.get("id"), "id"):
252-
return TestResult.FAILED
252+
_check(compare_tbool(a_insn.is_alias, e_insn.get("is_alias"), "is_alias"))
253253

254-
if not compare_tbool(a_insn.is_alias, e_insn.get("is_alias"), "is_alias"):
255-
return TestResult.FAILED
254+
_check(compare_tbool(a_insn.illegal, e_insn.get("illegal"), "illegal"))
256255

257-
if not compare_tbool(a_insn.illegal, e_insn.get("illegal"), "illegal"):
258-
return TestResult.FAILED
256+
_check(compare_uint32(a_insn.size, e_insn.get("size"), "size"))
259257

260-
if not compare_uint32(a_insn.size, e_insn.get("size"), "size"):
261-
return TestResult.FAILED
258+
_check(compare_enum(a_insn.alias_id, e_insn.get("alias_id"), "alias_id"))
262259

263-
if not compare_enum(a_insn.alias_id, e_insn.get("alias_id"), "alias_id"):
264-
return TestResult.FAILED
260+
_check(compare_details(a_insn, e_insn.get("details")))
265261

266-
if not compare_details(a_insn, e_insn.get("details")):
267-
return TestResult.FAILED
268262
return TestResult.SUCCESS
269263

270264

‎suite/cstest/src/test_case.c‎

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ TestInput *test_input_clone(TestInput *test_input)
5555
return ti;
5656
}
5757

58-
char *test_input_stringify(const TestInput *test_input, const char *postfix)
58+
char *test_input_stringify(const TestInput *test_input, const char *prefix)
5959
{
6060
size_t msg_len = 2048;
6161
char *msg = cs_mem_calloc(sizeof(char), msg_len);
@@ -75,11 +75,12 @@ char *test_input_stringify(const TestInput *test_input, const char *postfix)
7575
}
7676
}
7777
str_append_no_realloc(opt_seq, sizeof(opt_seq), "]");
78-
cs_snprintf(msg, msg_len,
79-
"%sTestInput { arch: %s, options: %s, addr: 0x%" PRIx64
80-
", bytes: %s }",
81-
postfix, test_input->arch, opt_seq, test_input->address,
82-
byte_seq);
78+
cs_snprintf(
79+
msg, msg_len,
80+
"%sTestInput { name: %s, arch: %s, options: %s, addr: 0x%" PRIx64
81+
", bytes: %s }",
82+
prefix, test_input->name, test_input->arch, opt_seq,
83+
test_input->address, byte_seq);
8384
cs_mem_free(byte_seq);
8485
return msg;
8586
}
@@ -213,6 +214,17 @@ static bool ids_match(uint32_t actual, const char *expected)
213214
return true;
214215
}
215216

217+
static void _print_insn(csh *handle, cs_insn *insn)
218+
{
219+
cm_print_error("Failed instruction: %s %s", insn->mnemonic,
220+
insn->op_str);
221+
cm_print_error("(0x%x", insn->bytes[0]);
222+
for (int i = 1; i < insn->size; i++) {
223+
cm_print_error(", 0x%x", insn->bytes[i]);
224+
}
225+
cm_print_error("%s", ")\n");
226+
}
227+
216228
/// Compares the decoded instructions @insns against the @expected values and returns the result.
217229
void test_expected_compare(csh *handle, TestExpected *expected, cs_insn *insns,
218230
size_t insns_count, size_t arch_bits)
@@ -233,53 +245,76 @@ void test_expected_compare(csh *handle, TestExpected *expected, cs_insn *insns,
233245
str_append_no_realloc(asm_text, sizeof(asm_text),
234246
insns[i].op_str);
235247
}
248+
249+
#define CS_TEST_FAIL(msg) \
250+
_print_insn(handle, &insns[i]); \
251+
fail_msg(msg);
252+
236253
if (!compare_asm_text(asm_text, expec_data->asm_text,
237254
arch_bits)) {
238-
fail_msg("asm-text mismatch\n");
255+
CS_TEST_FAIL("asm-text mismatch\n");
239256
}
240257

241258
// Not mandatory fields. If not initialized they should still match.
242259
if (expec_data->id) {
243-
assert_true(ids_match((uint32_t)insns[i].id,
244-
expec_data->id));
260+
if (!ids_match((uint32_t)insns[i].id, expec_data->id)) {
261+
CS_TEST_FAIL("ids mismatch");
262+
}
245263
}
246264
if (expec_data->is_alias != 0) {
247265
if (expec_data->is_alias > 0) {
248-
assert_true(insns[i].is_alias);
266+
if (!insns[i].is_alias) {
267+
CS_TEST_FAIL("should be an alias");
268+
}
249269
} else {
250-
assert_false(insns[i].is_alias);
270+
if (insns[i].is_alias) {
271+
CS_TEST_FAIL("should not be an alias");
272+
}
251273
}
252274
}
253275
if (expec_data->illegal != 0) {
254276
if (expec_data->illegal > 0) {
255-
assert_true(insns[i].illegal);
277+
if (!insns[i].illegal) {
278+
CS_TEST_FAIL("should be illegal");
279+
}
256280
} else {
257-
assert_false(insns[i].illegal);
281+
if (insns[i].illegal) {
282+
CS_TEST_FAIL("should not be illegal");
283+
}
258284
}
259285
}
260286
if (expec_data->size) {
261-
assert_int_equal(insns[i].size, expec_data->size);
287+
if (insns[i].size != expec_data->size) {
288+
CS_TEST_FAIL("size mismatch");
289+
}
262290
}
263291
if (expec_data->alias_id) {
264-
assert_true(ids_match((uint32_t)insns[i].alias_id,
265-
expec_data->alias_id));
292+
if (!ids_match((uint32_t)insns[i].alias_id,
293+
expec_data->alias_id)) {
294+
CS_TEST_FAIL("alias ids mismatch");
295+
}
266296
}
267297
if (expec_data->mnemonic) {
268-
assert_string_equal(insns[i].mnemonic,
269-
expec_data->mnemonic);
298+
if (strcmp(insns[i].mnemonic, expec_data->mnemonic) !=
299+
0) {
300+
CS_TEST_FAIL("mnemonics don't match");
301+
}
270302
}
271303
if (expec_data->op_str) {
272-
assert_string_equal(insns[i].op_str,
273-
expec_data->op_str);
304+
if (strcmp(insns[i].op_str, expec_data->op_str) != 0) {
305+
CS_TEST_FAIL("operands don't match");
306+
}
274307
}
275308
if (expec_data->details) {
276309
if (!insns[i].detail) {
277-
fprintf(stderr, "detail is NULL\n");
278-
assert_non_null(insns[i].detail);
310+
CS_TEST_FAIL("detail is NULL");
311+
}
312+
if (!test_expected_detail(handle, &insns[i],
313+
expec_data->details)) {
314+
CS_TEST_FAIL("test_expected_detail");
279315
}
280-
assert_true(test_expected_detail(handle, &insns[i],
281-
expec_data->details));
282316
}
317+
#undef CS_TEST_FAIL
283318
}
284319
}
285320

0 commit comments

Comments
 (0)