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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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 <= .

>> > +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


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