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

[Paolo Bonzini]

On 16/01/2018 14:50, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jan 16, 2018 at 1:06 PM, Peter Maydell <address@hidden> wrote:
>> On 16 January 2018 at 11:58, Marc-André Lureau
>> <address@hidden> wrote:
>>> Hi
>>>
>>> On Tue, Jan 16, 2018 at 12:25 PM, Peter Maydell
>>> <address@hidden> wrote:
>>>> On 15 January 2018 at 23:35, Paolo Bonzini <address@hidden> wrote:
>>>>> The following changes since commit 
>>>>> 997eba28a3ed5400a80f754bf3a1c8044b75b9ff:
>>>>>
>>>>>   Merge remote-tracking branch 
>>>>> 'remotes/pmaydell/tags/pull-target-arm-20180111' into staging (2018-01-11 
>>>>> 14:34:41 +0000)
>>>>>
>>>>> are available in the git repository at:
>>>>>
>>>>>
>>>>>   git://github.com/bonzini/qemu.git tags/for-upstream
>>>>>
>>>>> for you to fetch changes up to ff9adba50bf8a4c080b8aee9be2314ef179a7b5f:
>>>>>
>>>>>   ucontext: annotate coroutine stack for ASAN (2018-01-12 15:21:14 +0100)
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> * QemuMutex tracing improvements (Alex)
>>>>> * ram_addr_t optimization (David)
>>>>> * SCSI fixes (Fam, Stefan, me)
>>>>> * do {} while (0) fixes (Eric)
>>>>> * KVM fix for PMU (Jan)
>>>>> * memory leak fixes from ASAN (Marc-André)
>>>>> * migration fix for HPET, icount, loadvm (Maria, Pavel)
>>>>> * hflags fixes (me, Tao)
>>>>> * block/iscsi uninitialized variable (Peter L.)
>>>>> * full support for GMainContexts in character devices (Peter Xu)
>>>>> * more boot-serial-test (Thomas)
>>>>> * Memory leak fix (Zhecheng)
>>>>
>>>> Various build failures, I'm afraid:
>>>>
>>>
>>> damn, sorry..
>>>
>>>> x86-64/Linux/gcc:
>>>>
>>>> configure produces an error message:
>>>>
>>>> ERROR: ASAN build enabled, but ASAN header is too old.
>>>>        Without code annotation, the report may be inferior.
>>>>
>>>> even though this configure did not explicitly request ASAN.
>>>> Then configure seems to exit successfully anyway, since the
>>>> build proceeds.
>>>
>>> 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
> 
> Would that be enough to drop the "ERROR:" prefix ?
> 
>>>> For some reason the build creates config-target.h a lot:
>>
>>> One per target no? (the build should be more silent than before)
>>
>> Oh, I guess that makes sense. It's a bit odd that the message neither
>> gives the target part of the filename nor has those entering/leaving
>> directory messages...
>>
>>>>
>>>> Then it runs configure again, this time without the ERROR message,
>>>> and eventually fails with:
>>>>
>>>>   CC      hw/display/exynos4210_fimd.o
>>>> /home/petmay01/linaro/qemu-for-merges/hw/display/exynos4210_fimd.c: In
>>>> function ‘fimd_get_buffer_id’:
>>>> /home/petmay01/linaro/qemu-for-merges/hw/display/exynos4210_fimd.c:1105:5:
>>>> error: case label does not reduce to an integer constant
>>>>      case FIMD_WINCON_BUF2_STAT:
>>>
>>> never saw that error, hmm interesting. This is related to
>>> -fsanitize=address ? Is this on Debian stable?
> 
> Interesting, looks like a bug in gcc ubsan that doesn't happen with
> recent versions (related to
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80550 but probably a
> different bug). Adding a cast is enough:
> 
> -#define FIMD_WINCON_BUF2_STAT       ((0 << 21) | (1 << 31))
> +#define FIMD_WINCON_BUF2_STAT       (uint32_t)((0 << 21) | (1 << 31))
> 
> It looks like there are no other cases like this
> 
> 
> 
>>
>> Ubuntu xenial (16.04.3 LTS). No idea if it's related to the
>> sanitizer or not.
>>
>>>
>>>>      ^
>>>>
>>>> On sparc64 host configure fails:
>>
>>> Hmm, I guess the check -fsanitize=address doesn't return an error
>>> unless -Werror is given. Perhaps it needs:
>>>
>>> diff --git a/configure b/configure
>>> index f5550f3289..ba68c550c9 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -5190,7 +5190,7 @@ fi
>>>
>>>  have_asan=no
>>>  write_c_skeleton
>>> -if compile_prog "-fsanitize=address" ""; then
>>> +if compile_prog "-Werror -fsanitize=address" ""; then
>>>      have_asan=yes
>>>  fi
>>>
>>> @@ -5207,7 +5207,7 @@ int main(void) {
>>>    return 0;
>>>  }
>>>  EOF
>>> -if compile_prog "-fsanitize=address" "" ; then
>>> +if compile_prog "-Werror -fsanitize=address" "" ; then
>>>      have_asan_iface_fiber=yes
>>>  fi
>>>
>>
>> Looks plausible.
> 
> Actually, it probably needs also $CPU_CFLAGS
> 
> Paolo, would you drop "build-sys: add some sanitizers when
> --enable-debug if possible" & "ucontext: annotate coroutine stack for
> ASAN" from the series? I'll send a new version for those 2.

Yes, that was already my plan.  Thanks for confirming!

Paolo