Re: [Qemu-devel] [PATCH] tests: add fuzzing to visitor tests

[Peter Maydell]

On 2 February 2013 12:40, Blue Swirl <address@hidden> wrote:
> On Wed, Jan 30, 2013 at 4:37 PM, Kevin Wolf <address@hidden> wrote:
>> Am 19.01.2013 17:01, schrieb Blue Swirl:
>>> Perform input tests on random data.
>>>
>>> Improvement to code coverage for qapi/string-input-visitor.c
>>> is about 3 percentage points.
>>>
>>> Signed-off-by: Blue Swirl <address@hidden>
>>
>> Does this test pass for you? It consistently segfaults for me.
>
> Yes, it works on x86_64, i386, arm and sparc64.

> The call to g_free() in the fuzz function looks suspect.

On MacOSX this g_free fails the malloc system's checks:
/string-visitor/input/fuzz: test-string-input-visitor(76691) malloc:
*** error for object 0x7fff8e606b00: pointer being freed was not
allocated

This happens because you're reusing the Error* without checking
or clearing it after each call. If it's handed an Error** that
indicates an error has already occurred, visit_type_str()
does nothing, and so in test_visitor_in_fuzz() nothing has
set sres, and we try to g_free() an uninitialized pointer.

This code should either:
(a) avoid passing the visitors anything that could provoke
an error, and g_assert(!error_is_set(&errp)) after each call
(b) if errors are ok, do
  if (error_is_set(&errp)) {
      error_free(errp);
      errp = NULL;
  }
after each call.

I don't know exactly what the semantics of visit_type_str()
are [my guess is "if no error, caller must g_free() string,
otherwise no string allocated"] -- if somebody who did know
was able to write some brief docstring comments for visitor.h
that might be nice :-)

-- PMM