capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.
http://www.capstone-engine.org
7.59k stars 1.56k forks source link

cstest does not test multiple instruction lines per issue #1861

Open adamjseitz opened 2 years ago

adamjseitz commented 2 years ago

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
kabeor commented 2 years ago

Can you make a pr for this? :)

Rot127 commented 1 year ago

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 commented 1 year ago

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);