Re: [Xen-devel] [PATCH v4 10/12] fuzz/x86_emulate: Add --rerun option to try to track down instability

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.10.17 at 19:52, <george.dunlap@xxxxxxxxxx> wrote:
> @@ -884,20 +891,146 @@ int LLVMFuzzerInitialize(int *argc, char ***argv)
>      return 0;
>  }
>  
> -int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
> +static void setup_fuzz_state(struct fuzz_state *state, const void *data_p, 
> size_t size)
>  {
> -    struct fuzz_state state = {
> -        .ops = all_fuzzer_ops,
> -    };
> -    struct x86_emulate_ctxt ctxt = {
> -        .data = &state,
> -        .regs = &state.regs,
> -        .addr_size = 8 * sizeof(void *),
> -        .sp_size = 8 * sizeof(void *),
> -    };
> +    memset(state, 0, sizeof(*state));
> +    state->corpus = data_p;
> +    state->data_num = size;
> +}
> +
> +static int runtest(struct fuzz_state *state) {
>      int rc;
>  
> -    if ( size <= fuzz_minimal_input_size() )
> +    struct x86_emulate_ctxt *ctxt = &state->ctxt;

Please don't leave a blank line between declarations.

> +static void compare_states(struct fuzz_state state[2])
> +{
> +    /* First zero any "internal" pointers */
> +    state[0].corpus = state[1].corpus = NULL;
> +    state[0].ctxt.data = state[1].ctxt.data = NULL;
> +    state[0].ctxt.regs = state[1].ctxt.regs = NULL;
> +
> +    if ( memcmp(&state[0], &state[1], sizeof(struct fuzz_state)) )
> +    {
> +        unsigned int i;
> +
> +        printf("State mismatch\n");
> +
> +        for ( i = 0; i < ARRAY_SIZE(state[0].cr); i++ )
> +            if ( state[0].cr[i] != state[1].cr[i] )
> +                printf("cr[%u]: %lx != %lx\n",
> +                       i, state[0].cr[i], state[1].cr[i]);
> +        
> +        for ( i = 0; i < ARRAY_SIZE(state[0].msr); i++ )
> +            if ( state[0].msr[i] != state[1].msr[i] )
> +                printf("msr[%u]: %lx != %lx\n",
> +                       i, state[0].msr[i], state[1].msr[i]);
> +        
> +        for ( i = 0; i < ARRAY_SIZE(state[0].segments); i++ )
> +            if ( memcmp(&state[0].segments[i], &state[1].segments[i],
> +                        sizeof(state[0].segments[0])) )
> +                printf("segments[%u]: [%x:%x:%x:%lx] != [%x:%x:%x:%lx]!\n", 
> i,
> +                       (unsigned)state[0].segments[i].sel,
> +                       (unsigned)state[0].segments[i].attr,
> +                       state[0].segments[i].limit,
> +                       state[0].segments[i].base,
> +                       (unsigned)state[1].segments[i].sel,
> +                       (unsigned)state[1].segments[i].attr,
> +                       state[1].segments[i].limit,
> +                       state[1].segments[i].base);
> +
> +        if ( state[0].data_num != state[1].data_num )
> +            printf("data_num: %lx !=  %lx\n", state[0].data_num,
> +                   state[1].data_num);
> +        if ( state[0].data_index != state[1].data_index )
> +            printf("data_index: %lx !=  %lx\n", state[0].data_index,
> +                   state[1].data_index);
> +
> +        if ( memcmp(&state[0].regs, &state[1].regs, sizeof(state[0].regs)) )
> +        {
> +            printf("registers differ!\n");
> +            /* Print If Not Equal */
> +#define PRINT_NE(elem)\
> +            if ( state[0].elem != state[1].elem ) \
> +                printf(#elem " differ: %lx != %lx\n", \
> +                       (unsigned long)state[0].elem, \
> +                       (unsigned long)state[1].elem)
> +            PRINT_NE(regs.r15);
> +            PRINT_NE(regs.r14);
> +            PRINT_NE(regs.r13);
> +            PRINT_NE(regs.r12);
> +            PRINT_NE(regs.rbp);
> +            PRINT_NE(regs.rbx);
> +            PRINT_NE(regs.r10);
> +            PRINT_NE(regs.r11);
> +            PRINT_NE(regs.r9);
> +            PRINT_NE(regs.r8);
> +            PRINT_NE(regs.rax);
> +            PRINT_NE(regs.rcx);
> +            PRINT_NE(regs.rdx);
> +            PRINT_NE(regs.rsi);
> +            PRINT_NE(regs.rdi);

Aren't these register fields all of the same type? If so, why do you
need to casts to unsigned long in the macro?

Additionally iirc Andrew had asked for eflags (and perhaps also
selector register values) to be printed separately for ease of
recognition - I support this request.

> +            for ( i = offsetof(struct cpu_user_regs, error_code) / 
> sizeof(unsigned);
> +                  i < sizeof(state[1].regs)/sizeof(unsigned); i++ )

Blanks around binary operators please (also elsewhere).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel