Re: [Qemu-devel] test-qapi-visit causes clang -fsanitize=undefined warning

[Kevin Wolf <kwolf@redhat.com>]

Am 10.03.2014 um 19:21 hat Markus Armbruster geschrieben:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> > On 10 March 2014 13:36, Markus Armbruster <armbru@redhat.com> wrote:
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> Turns out my clang installation doesn't support -fsanitize=undefined: it
> >> lacks libclang_rt.san-x86_64.a.
> >>
> >> Test works fine without -fsanitize=undefined.  I set a breakpoint on
> >> visit_type_UserDefA_fields, and there's no null pointer to be found.
> >
> > Yes, there is. This is gdb on a gcc-compiled version of
> > the test:
> >
> > /visitor/input-strict/fail/union-flat:
> > Breakpoint 1, visit_type_UserDefA_fields (m=0x5555557a8ed0,
> > obj=0x5555557a8d88, errp=0x7fffffffdfb0)
> >     at tests/test-qapi-visit.c:460
> > 460     {
> > (gdb) print *obj
> > $3 = (UserDefA *) 0x0
> > (gdb) bt
> > #0  visit_type_UserDefA_fields (m=0x5555557a8ed0, obj=0x5555557a8d88,
> > errp=0x7fffffffdfb0)
> >     at tests/test-qapi-visit.c:460
> > #1  0x000055555555b2a2 in visit_type_UserDefFlatUnion
> > (m=0x5555557a8ed0, obj=0x7fffffffdfd8, name=0x0, errp=0x0)
> >     at tests/test-qapi-visit.c:654
> > #2  0x0000555555558c0b in qapi_free_UserDefFlatUnion
> > (obj=0x5555557a8d80) at tests/test-qapi-types.c:368
> > #3  0x000055555555cc47 in test_validate_fail_union_flat
> > (data=0x5555557a8de0, unused=0x7fffffffe3f0)
> >     at
> > /home/petmay01/linaro/qemu-from-laptop/qemu/tests/test-qmp-input-strict.c:241
> >
> > *obj is NULL, which is what clang is complaining about.
> 
> I think what happens here is the visitor fails half-way through
> constructing the type, and returns a UserDefFlatUnion with kind
> USER_DEF_UNION_KIND_A and data.a null, violating the data type's
> invariant.

This is one of the cases that I found during my blockdev-add testing. I
wasn't sure whether introduing something like a KIND_UNDEFINED or adding
a new visitor callback like .struct_abort was the better way of fixing
this. And then I ran out of time.

Please keep me CCed when you send patches for this.

> We either have to relax the invariant, or fix the error paths.  I'm
> looking into the latter.

Yes, I think that's preferable.

Kevin