Re: [Qemu-devel] [PULL v3 00/53] Misc changes for 2017-01-12

[Paolo Bonzini <pbonzini@redhat.com>]

On 16/01/2018 15:22, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jan 16, 2018 at 2:54 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 16/01/2018 14:47, Peter Maydell wrote:
>>> On 16 January 2018 at 13:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> On 16/01/2018 13:06, Peter Maydell wrote:
>>>>>> ASAN is enabled by default if available when --enable-debug. We could
>>>>>> add more flags if that helps.
>>>>> Configure switches should work like this:
>>>>>  * default: use feature if present, but don't complain if not present
>>>>>    or not usable
>>>>>  * --enable-foo: use feature. if feature not present, complain and
>>>>>    fail configure
>>>>>  * --disable-foo: don't test for or use feature
>>>>>
>>>>
>>>> However, --enable-debug has never worked like this (the "default" part)...
>>>
>>> True, but -g, no optimization isn't really something we want to
>>> default to :-)
>>
>> Same for ASAN. :-)
>>
>>> I think the general principle that unless the user
>>> specifically said they cared about the address sanitizer we shouldn't
>>> complain if it happens not to work on this host is still a good one.
>>
>> Yes, I agree.
>>
>> So we need two options:
>>
>> * --enable-asan defaults to not used, but also fails configure if ASAN
>> is not available/usable.
> 
> and --enable-ubsan etc ...
> 
> I wish we would avoid the multiplication of configure options, and use
> good default values instead for --enable-debug. But if it's not
> possible, let's add more options. However, it would be great if ASAN
> can be enabled by default, it seems too few developers care, even
> though it should be strongly recommended.

We can add --enable-sanitizer then instead of being specific to ASAN.
It could also support --enable-sanitizer=asan,ubsan but that's for later.

>>
>> * if we want to have --enable-debug enable ASAN, it should however _not_
>> fail configure if ASAN is not available/usable.  (I am not sure anymore
>> it's a good idea).
>>
>> The questions are:
>>
>> * should fiber support be required for --enable-asan?  What is the
>> difference in the quality of the reports?
> 
> It's not required, but helps to detect more leaks. It also removes
> some warnings in some cases:

Ok, if it's not a flood of warnings it's okay.

> 
>> * if not, and assuming --enable-debug tries to enable ASAN, should
>> --enable-debug complain if fiber support is not required?  Should
>> --enable-debug enable ASAN if fiber support is not available?
> 
> I propose to simply print a warning during configure

Yes, it could do the same as --enable-asan.

>> * if --enable-debug does *not* try to enable ASAN, should test-debug add
>> --enable-asn?  (I think so).
> 
> The other way around?

I mean - IMO, test-debug should enable sanitizers even if
--enable-sanitizer and --enable-debug are completely separate.

Paolo