capstone icon indicating copy to clipboard operation
capstone copied to clipboard

cstest does not test multiple instruction lines per issue

Open adamjseitz opened this issue 4 years ago • 1 comments

The cstest regression test suite specifies tests using something like this:

!# issue 1661 M68K invalid transfer direction in MOVEC instruction
!# CS_ARCH_M68K, CS_MODE_BIG_ENDIAN | CS_MODE_M68K_040, None
0x4E,0x7A,0x00,0x02 == movec cacr, d0

Some tests attempt to specify multiple instructions to decode and check in a single issue, e.g.:

!# issue 1827 x16 lcall seg:off format
!# CS_ARCH_X86, CS_MODE_16, CS_OPT_DETAIL
0x33,0xc0 == xor ax, ax
0xba,0x5a,0xff == mov dx, 0xff5a

cstest only actually tests the first such instruction. I tried changing some of them to be incorrect, and after the first one, they do not fail. For example, changing the above test to this does not fail:

!# issue 1827 x16 lcall seg:off format
!# CS_ARCH_X86, CS_MODE_16, CS_OPT_DETAIL
0x33,0xc0 == xor ax, ax
0xba,0x5a,0xff == mov dx, 0xff5b

adamjseitz avatar Mar 22 '22 13:03 adamjseitz

Can you make a pr for this? :)

kabeor avatar Mar 23 '22 11:03 kabeor

This is a hotfix for the problem:

[deleted]

@kabeor I would not merge this because it might leads to unknown behavior. E.g. if more than one setup line (like this# CS_ARCH_ARM, CS_MODE_THUMB, None) exist in the file).

Rot127 avatar Apr 03 '23 13:04 Rot127

I was sloppy and made false assumptions how the functions work.

Here is the actual working patch:

diff --git a/suite/cstest/src/main.c b/suite/cstest/src/main.c
index 88e1b328..68ef1f38 100644
--- a/suite/cstest/src/main.c
+++ b/suite/cstest/src/main.c
@@ -119,20 +119,19 @@ static int getDetail;
 static int mc_mode;
 static int e_flag;
 
-static int setup_MC(void **state)
-{
+static int setup_state(void **state) {
 	csh *handle;
 	char **list_params;	
 	int size_params;
 	int arch, mode;
-	int i, index, tmp_counter;
+	int i, index;
 
 	if (failed_setup) {
 		fprintf(stderr, "[  ERROR   ] --- Invalid file to setup\n");
 		return -1;
 	}
 
-	tmp_counter = 0;
+	int tmp_counter = 0;
 	while (tmp_counter < size_lines && list_lines[tmp_counter][0] != '#')
 		tmp_counter++;
 
@@ -181,18 +180,13 @@ static int setup_MC(void **state)
 		failed_setup = 1;
 		return -1;
 	}
-	
-	for (i = 0; i < ARR_SIZE(options); ++i) {
-		if (strstr(list_params[2], options[i].str)) {
-			if (cs_option(*handle, options[i].first_value, options[i].second_value) != CS_ERR_OK) {
-				fprintf(stderr, "[  ERROR   ] --- Option is not supported for this arch/mode\n");
-				failed_setup = 1;
-				return -1;
-			}
-		}
-	}
-
 	*state = (void *)handle;
+	free_strs(list_params, size_params);
+	return 0;
+}
+
+static int setup_MC(void **state)
+{
 	counter++;
 	if (e_flag == 0)
 		while (counter < size_lines && strncmp(list_lines[counter], "0x", 2))
@@ -201,7 +195,6 @@ static int setup_MC(void **state)
 		while (counter < size_lines && strncmp(list_lines[counter], "// 0x", 5))
 			counter++;
 
-	free_strs(list_params, size_params);
 	return 0;
 }
 
@@ -213,7 +206,7 @@ static void test_MC(void **state)
 		test_single_MC((csh *)*state, mc_mode, list_lines[counter]);
 }
 
-static int teardown_MC(void **state)
+static int teardown_state(void **state)
 {
 	cs_close(*state);
 	free(*state);
@@ -391,13 +384,13 @@ static void test_file(const char *filename)
 				tmp = (char *)malloc(sizeof(char) * 100);
 				sprintf(tmp, "Line %d", i+1);
 				tests = (struct CMUnitTest *)realloc(tests, sizeof(struct CMUnitTest) * (number_of_tests + 1));
-				tests[number_of_tests] = (struct CMUnitTest)cmocka_unit_test_setup_teardown(test_MC, setup_MC, teardown_MC);
+				tests[number_of_tests] = (struct CMUnitTest)cmocka_unit_test_setup_teardown(test_MC, setup_MC, NULL);
 				tests[number_of_tests].name = tmp;
 				number_of_tests ++;
 			}
 		}
 
-		_cmocka_run_group_tests("Testing MC", tests, number_of_tests, NULL, NULL);
+		_cmocka_run_group_tests("Testing MC", tests, number_of_tests, setup_state, teardown_state);
 	}
 
 	printf("[+] DONE: %s\n", filename);

Rot127 avatar Apr 06 '23 10:04 Rot127