Skip to content

Commit 155a483

Browse files
Rot127Rot127
authored
Fix cstest_py and add negative tests. (#2807)
* Fix faulty _check(). * Add negative test case for cstest_py * Enable full leak report and fix leaks. * Add negative tests to cmake and CI. * Fix typos
1 parent 9d4fb1a commit 155a483

File tree

12 files changed

+231
-17
lines changed

12 files changed

+231
-17
lines changed

‎.github/workflows/CITest.yml‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,16 @@ jobs:
147147
run: |
148148
sh suite/run_invalid_cstool.sh
149149
150+
- name: cstest negatives
151+
if: startsWith(matrix.config.build-system, 'cmake')
152+
run: |
153+
ctest --test-dir build --output-on-failure -R NegativeTests1
154+
ctest --test-dir build --output-on-failure -R NegativeTests2
155+
ctest --test-dir build --output-on-failure -R NegativeTests3
156+
ctest --test-dir build --output-on-failure -R NegativeTests4
157+
ctest --test-dir build --output-on-failure -R NegativeTests5
158+
ctest --test-dir build --output-on-failure -R NegativeTests6
159+
150160
- name: cstest MC
151161
if: startsWith(matrix.config.build-system, 'cmake')
152162
run: |
@@ -177,9 +187,12 @@ jobs:
177187
run: |
178188
sudo apt-get -y update
179189
sudo apt-get -y install valgrind
180-
valgrind cstest tests
190+
valgrind --leak-check=full cstest tests/details
191+
valgrind --leak-check=full cstest tests/features
192+
valgrind --leak-check=full cstest tests/issues
193+
valgrind --leak-check=full cstest tests/MC
181194
182-
- name: Comaptibility header generation
195+
- name: Compatibility header generation
183196
# clang-format-17 is only available in Ubuntu 24.04
184197
# Hence the check only happens for it.
185198
if: startsWith(matrix.config.build-system, 'cmake') && startsWith(matrix.config.os, 'ubuntu-24.04')

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

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ def print_evaluate(self):
137137
log.error(
138138
"Inconsistent statistics: total != successful + failed + skipped\n"
139139
)
140+
exit(-1)
140141

141142
if self.errors != 0:
142143
log.error("Failed with errors\n")
@@ -236,28 +237,41 @@ def compare(self, actual_insns: list[CsInsn], bits: int) -> TestResult:
236237
return TestResult.FAILED
237238

238239
for a_insn, e_insn in zip(actual_insns, self.insns):
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))
240+
if not compare_asm_text(a_insn, e_insn.get("asm_text"), bits):
241+
log.error(f"Failed instruction: {a_insn}")
242+
return TestResult.FAILED
245243

246-
_check(compare_str(a_insn.mnemonic, e_insn.get("mnemonic"), "mnemonic"))
244+
if not compare_str(a_insn.mnemonic, e_insn.get("mnemonic"), "mnemonic"):
245+
log.error(f"Failed instruction: {a_insn}")
246+
return TestResult.FAILED
247247

248-
_check(compare_str(a_insn.op_str, e_insn.get("op_str"), "op_str"))
248+
if not compare_str(a_insn.op_str, e_insn.get("op_str"), "op_str"):
249+
log.error(f"Failed instruction: {a_insn}")
250+
return TestResult.FAILED
249251

250-
_check(compare_enum(a_insn.id, e_insn.get("id"), "id"))
252+
if not compare_enum(a_insn.id, e_insn.get("id"), "id"):
253+
log.error(f"Failed instruction: {a_insn}")
254+
return TestResult.FAILED
251255

252-
_check(compare_tbool(a_insn.is_alias, e_insn.get("is_alias"), "is_alias"))
256+
if not compare_tbool(a_insn.is_alias, e_insn.get("is_alias"), "is_alias"):
257+
log.error(f"Failed instruction: {a_insn}")
258+
return TestResult.FAILED
253259

254-
_check(compare_tbool(a_insn.illegal, e_insn.get("illegal"), "illegal"))
260+
if not compare_tbool(a_insn.illegal, e_insn.get("illegal"), "illegal"):
261+
log.error(f"Failed instruction: {a_insn}")
262+
return TestResult.FAILED
255263

256-
_check(compare_uint32(a_insn.size, e_insn.get("size"), "size"))
264+
if not compare_uint32(a_insn.size, e_insn.get("size"), "size"):
265+
log.error(f"Failed instruction: {a_insn}")
266+
return TestResult.FAILED
257267

258-
_check(compare_enum(a_insn.alias_id, e_insn.get("alias_id"), "alias_id"))
268+
if not compare_enum(a_insn.alias_id, e_insn.get("alias_id"), "alias_id"):
269+
log.error(f"Failed instruction: {a_insn}")
270+
return TestResult.FAILED
259271

260-
_check(compare_details(a_insn, e_insn.get("details")))
272+
if not compare_details(a_insn, e_insn.get("details")):
273+
log.error(f"Failed instruction: {a_insn}")
274+
return TestResult.FAILED
261275

262276
return TestResult.SUCCESS
263277

@@ -401,7 +415,6 @@ def run_tests(self):
401415
self.stats.add_failing_file(tf.path)
402416
self.stats.add_test_case_data_point(result)
403417
log.info(result)
404-
print()
405418
self.stats.print_evaluate()
406419

407420

‎suite/cstest/CMakeLists.txt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,40 @@ add_test(FeaturesTests
8888
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
8989
)
9090

91+
# These tests must be run independently. Otherwise we won't detect when one succeeds.
92+
# Because cstest will always return not 0 from the first failing test onwards.
93+
# But we need to ensure all of them fail always.
94+
add_test(NegativeTests1
95+
cstest ${PROJECT_SOURCE_DIR}/tests/negative/should_fail_I.yaml
96+
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
97+
)
98+
set_property(TEST NegativeTests1 PROPERTY WILL_FAIL TRUE)
99+
add_test(NegativeTests2
100+
cstest ${PROJECT_SOURCE_DIR}/tests/negative/should_fail_II.yaml
101+
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
102+
)
103+
set_property(TEST NegativeTests2 PROPERTY WILL_FAIL TRUE)
104+
add_test(NegativeTests3
105+
cstest ${PROJECT_SOURCE_DIR}/tests/negative/should_fail_III.yaml
106+
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
107+
)
108+
set_property(TEST NegativeTests3 PROPERTY WILL_FAIL TRUE)
109+
add_test(NegativeTests4
110+
cstest ${PROJECT_SOURCE_DIR}/tests/negative/should_fail_IV.yaml
111+
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
112+
)
113+
set_property(TEST NegativeTests4 PROPERTY WILL_FAIL TRUE)
114+
add_test(NegativeTests5
115+
cstest ${PROJECT_SOURCE_DIR}/tests/negative/should_fail_V.yaml
116+
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
117+
)
118+
set_property(TEST NegativeTests5 PROPERTY WILL_FAIL TRUE)
119+
add_test(NegativeTests6
120+
cstest ${PROJECT_SOURCE_DIR}/tests/negative/should_fail_VI.yaml
121+
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
122+
)
123+
set_property(TEST NegativeTests6 PROPERTY WILL_FAIL TRUE)
124+
91125

92126
if(CAPSTONE_INSTALL)
93127
install(TARGETS cstest EXPORT capstone-targets DESTINATION ${CMAKE_INSTALL_BINDIR})

‎suite/cstest/src/cstest.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,20 @@ void print_test_run_stats(const TestRunStats *stats)
7474
printf("\n");
7575
}
7676

77+
void cleanup_test_files()
78+
{
79+
if (!test_files) {
80+
return;
81+
}
82+
for (size_t k = 0; k < file_count; ++k) {
83+
cs_mem_free(test_files[0][k]);
84+
}
85+
if (test_files[0]) {
86+
cs_mem_free(test_files[0]);
87+
}
88+
cs_mem_free(test_files);
89+
}
90+
7791
int main(int argc, const char **argv)
7892
{
7993
if (argc < 2 || strcmp(argv[1], "-h") == 0 ||
@@ -87,6 +101,7 @@ int main(int argc, const char **argv)
87101
get_tfiles(argc, argv);
88102
if (!*test_files || file_count == 0) {
89103
fprintf(stderr, "Arguments are invalid. No files found.\n");
104+
cleanup_test_files();
90105
exit(EXIT_FAILURE);
91106
}
92107

@@ -95,6 +110,7 @@ int main(int argc, const char **argv)
95110
TestRunResult res = cstest_run_tests(*test_files, file_count, &stats);
96111

97112
print_test_run_stats(&stats);
113+
cleanup_test_files();
98114
if (res == TEST_RUN_ERROR) {
99115
fprintf(stderr, "[!] An error occured.\n");
100116
exit(EXIT_FAILURE);

‎suite/cstest/src/test_case.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ void test_expected_compare(csh *handle, TestExpected *expected, cs_insn *insns,
248248

249249
#define CS_TEST_FAIL(msg) \
250250
_print_insn(handle, &insns[i]); \
251+
cs_free(insns, insns_count); \
251252
fail_msg(msg);
252253

253254
if (!compare_asm_text(asm_text, expec_data->asm_text,

‎suite/run_tests.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,22 @@
1818
f"cstest_py {root_dir}/tests/features/",
1919
]
2020

21+
expected_to_fail = [
22+
f"cstest_py {root_dir}/tests/negative/",
23+
]
24+
2125
for test in tests:
2226
logger.info(f'Running {test}')
2327
logger.info("#######################")
2428
subprocess.run(test.split(" "), check=True)
2529
logger.info("-----------------------")
30+
31+
for test in expected_to_fail:
32+
logger.info(f'Running {test}')
33+
logger.info("#######################")
34+
try:
35+
subprocess.run(test.split(" "), check=True)
36+
logger.error("Test was expected to fail.")
37+
exit(-1)
38+
except Exception as e:
39+
logger.info("-----------------------")

‎tests/negative/should_fail_I.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
test_cases:
2+
-
3+
input:
4+
name: "Fail because of too few input bytes."
5+
bytes: [ 0x00 ]
6+
arch: "sparc"
7+
options: [ CS_OPT_DETAIL, CS_MODE_BIG_ENDIAN ]
8+
address: 0x1000
9+
expected:
10+
insns:
11+
-
12+
asm_text: "cmp %g1, %g2"
13+
details:
14+
sparc:
15+
operands:
16+
-
17+
type: SPARC_OP_REG
18+
reg: g1
19+
-
20+
type: SPARC_OP_REG
21+
reg: g2

‎tests/negative/should_fail_II.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
test_cases:
2+
-
3+
input:
4+
name: "Fails due to mismatching asm text"
5+
bytes: [ 0x80, 0xa0, 0x40, 0x02 ]
6+
arch: "sparc"
7+
options: [ CS_OPT_DETAIL, CS_MODE_BIG_ENDIAN ]
8+
address: 0x1000
9+
expected:
10+
insns:
11+
-
12+
asm_text: "cmp %g1, %g3"
13+
details:
14+
sparc:
15+
operands:
16+
-
17+
type: SPARC_OP_REG
18+
reg: g1
19+
-
20+
type: SPARC_OP_REG
21+
reg: g2

‎tests/negative/should_fail_III.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
test_cases:
2+
-
3+
input:
4+
name: "Fails due to missing detail operand"
5+
bytes: [ 0x80, 0xa0, 0x40, 0x02 ]
6+
arch: "sparc"
7+
options: [ CS_OPT_DETAIL, CS_MODE_BIG_ENDIAN ]
8+
address: 0x1000
9+
expected:
10+
insns:
11+
-
12+
asm_text: "cmp %g1, %g2"
13+
details:
14+
sparc:
15+
operands:
16+
-
17+
type: SPARC_OP_REG
18+
reg: g1

‎tests/negative/should_fail_IV.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
test_cases:
2+
-
3+
input:
4+
name: "Fails due to non existent architecture name."
5+
bytes: [ 0x00, 0xa0, 0x40, 0x02 ]
6+
arch: "not_existant"
7+
options: [ CS_OPT_DETAIL, CS_MODE_BIG_ENDIAN ]
8+
address: 0x1000
9+
expected:
10+
insns:
11+
-
12+
asm_text: "cmp %g1, %g2"
13+
details:
14+
sparc:
15+
operands:
16+
-
17+
type: SPARC_OP_REG
18+
reg: g1
19+
-
20+
type: SPARC_OP_REG
21+
reg: g2

0 commit comments

Comments
 (0)