qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PULL v3 00/53] Misc changes for 2017-01-12
Date: Tue, 16 Jan 2018 15:22:36 +0100

Hi

On Tue, Jan 16, 2018 at 2:54 PM, Paolo Bonzini <address@hidden> wrote:
> On 16/01/2018 14:47, Peter Maydell wrote:
>> On 16 January 2018 at 13:41, Paolo Bonzini <address@hidden> 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.

>
> * 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:

Before:

address@hidden:~/src/qemu/build (asan *%)$ tests/test-coroutine -p
/basic/lifecycle
/basic/lifecycle: ==20781==WARNING: ASan doesn't fully support
makecontext/swapcontext functions and may produce false positives in
some cases!
==20781==WARNING: ASan is ignoring requested __asan_handle_no_return:
stack top: 0x7ffcb184d000; bottom 0x7ff6c4cfd000; size: 0x0005ecb50000
(25446121472)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
OK

After:

 tests/test-coroutine -p /basic/lifecycle
/basic/lifecycle: ==21110==WARNING: ASan doesn't fully support
makecontext/swapcontext functions and may produce false positives in
some cases!
OK


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

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

The other way around?

(tbh, I am not fond of all the configure options - if you need
fine-grained configuration, you can overwrite the various *FLAGS..)

-- 
Marc-André Lureau



reply via email to

[Prev in Thread] Current Thread [Next in Thread]