Re: [Xen-devel] [PATCH v2 10/12] fuzz/x86emul: update fuzzer

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jan 31, 2017 at 09:02:46AM -0700, Jan Beulich wrote:
> >>> On 31.01.17 at 16:51, <wei.liu2@xxxxxxxxxx> wrote:
> > On Tue, Jan 31, 2017 at 06:33:11AM -0700, Jan Beulich wrote:
> >> >>> On 31.01.17 at 12:08, <wei.liu2@xxxxxxxxxx> wrote:
> >> > +static int _fuzz_rep_read(const char *why, unsigned long *reps)
> >> > +{
> >> > +    int rc;
> >> > +    unsigned long bytes_read = 0;
> >> > +
> >> > +    rc = data_read(why, &bytes_read, sizeof(bytes_read));
> >> > +
> >> > +    if ( bytes_read < *reps )
> >> > +        *reps -= bytes_read;
> >> 
> >> I don't understand this: In the success case, *reps upon return
> >> indicates the number of iterations done, not the number of
> >> iterations left. So at least the variable naming deserves
> >> improvement.
> > 
> > OK, so this should be *reps = bytes_read.
> 
> And the comparison should then presumably become <= .
> 

Ah, yes.

> >> > +static void setup_fpu_exception_handler(void)
> >> > +{
> >> > +    /* FIXME - just disable exceptions for now */
> >> > +    unsigned long a;
> >> > +
> >> > +    asm volatile ( "fnclex");
> >> > +    a=0x37f;                    /* FCW_DEFAULT in Xen */
> >> > +    asm volatile ( "fldcw %0" :: "m" (a));
> >> > +    a=0x1f80;                   /* MXCSR_DEFAULT in Xen */
> >> > +    asm volatile ( "ldmxcsr %0" :: "m" (a) );
> >> > +}
> >> 
> >> While I see that the FCW value has changed, the strange local
> >> variable is still there. If you really want to keep it, please at least
> >> add the missing spaces around the = signs. But I'd prefer
> >> 
> >>     asm volatile ( "fldcw %0" :: "m" (0x37f /* FCW_DEFAULT in Xen */));
> >>     asm volatile ( "ldmxcsr %0" :: "m" (0x1f80 /* MXCSR_DEFAULT in Xen */) 
> > );
> >> 
> > 
> > This doesn't work.
> > 
> > x86-insn-emulator-fuzzer.c:445:5: error: memory input 0 is not directly 
> > addressable
> 
> Oh. Not sure why that is.
> 
> >> And then - doesn't the ABI require these settings to be in effect
> >> upon program startup anyway?
> >> 
> > 
> > I'm not sure about this -- reading AMD64 ABI Draft 0.99.8 doesn't reveal
> > much for me. But having the code arranged like this hasn't caused any
> > SIGFPE so far.
> > 
> > What do you suggest I do here?
> 
> Well, you can keep the code as is if there's uncertainty. The
> question was mainly from the perspective of whether the non-
> fuzzing test would need to do something similar. With the
> SSEn/AVX test blob I'm close to round up, I haven't run into
> any situation where the initial settings hadn't been the intended
> ones.
> 
> Jan

Right. I will leave the code as-is for now.

Wei.

> 

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