Re: Unit Test Aborts when building with -ftrapv compiler option

[Daniel P . Berrangé]

On Fri, Feb 09, 2024 at 08:02:49AM +0100, Thomas Huth wrote:
> On 08/02/2024 20.23, Richard Schmitt wrote:
> > In an attempt to build qemu with hardened gcc compiler options, we
> > specified the -ftrapv switch rather than the -fwrapv switch.  The
> > switches define the behavior of integer overflows.  -ftrapv causes an
> > abort on integer overflow, -fwrapv causes overflows to simply wrap
> > without any error indication.  Wrapping overflows can result in
> > unexpected behavior and therefore, hardenened builds typically recommend
> > trapping overflows.
> > 
> > The abort occurs when running the “test-string-input-verifier” test and
> > begins at line 129 of the source:
> > 
> > v = visitor_input_test_init(data,
> > 
> >                              “-9223372036854775808, 9223372036854775807”);
> > 
> > check_ilist(v, expect3, ARRAY_SIZE(expect3);
> > 
> > Where expect3 is declared as:
> > 
> > int64_t expect3[] = { INT64_MIN, INT64_MAX };
> > 
> > The actual abort occurs in “string-input-visitor.c” line 209:
> > 
> > *obj = siv->rangeNext.i64++;
> > 
> > The test, as coded, will generate an overflow.  Using the -fwrapv
> > compiler option hides the overflow.
> > 
> > My question, is it the intent of the qemu community to rely on the
> > overflow wrap behavior or should this be considered an issue and added
> > as such in gitlab’s issue list?
> 
> As far as I understood, QEMU deliberately depends on this behavior - at
> least we require -fWrapv in meson.build:
> 
> # We use -fwrapv to tell the compiler that we require a C dialect where
> # left shift of signed integers is well defined and has the expected
> # 2s-complement style results. (Both clang and gcc agree that it
> # provides these semantics.)


Introduced by this:

commit 2d31515bc0880a1cea86ce638d2a109f4f4e6f7d
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Mon Sep 12 14:10:08 2016 +0100

    configure: Always compile with -fwrapv
    
    QEMU's code relies on left shifts of signed integers always
    being defined behaviour with the obvious 2s-complement
    semantics. The only way to tell the compiler (and any
    associated undefined-behaviour sanitizer) that we require a
    C dialect with these semantics is to use the -fwrapv option.
    This is a bit of a heavy hammer for the job as it also gives
    us guaranteed semantics on integer arithmetic overflow which
    in theory we don't require.
    
    In an ideal world this would allow us to drop the warning
    flag -Wno-shift-negative-value, but we must retain this to
    avoid spurious warnings on clang versions predating the
    fix to https://llvm.org/bugs/show_bug.cgi?id=25552.
    
    Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
    Reviewed-by: Markus Armbruster <armbru@redhat.com>
    Acked-by: Paolo Bonzini <pbonzini@redhat.com>
    Message-id: 1473685808-9629-1-git-send-email-peter.maydell@linaro.org


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|