On 11/20/20 8:39 AM, Pavel Hrdina wrote:
> On Fri, Nov 20, 2020 at 06:57:10AM -0500, John Ferlan wrote:
>>
>>
>> On 11/16/20 10:36 AM, Pavel Hrdina wrote:
>>> Inspired by QEMU code.
>>>
>>> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
>>> ---
>>> meson.build | 14 --------------
>>> src/internal.h | 4 ++++
>>> 2 files changed, 4 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index cecaad199d..dbbc9632f1 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -142,20 +142,6 @@ if get_option('test_coverage')
>>> endif
>>>
>>>
>>> -# Detect when running under the clang static analyzer's scan-build driver
>>> -# or Coverity-prevent's cov-build. Define STATIC_ANALYSIS accordingly.
>>> -
>>> -rc = run_command(
>>> - 'sh', '-c',
>>> - 'test -n "${CCC_ANALYZER_HTML}"' +
>>> - ' -o -n "${CCC_ANALYZER_ANALYSIS+set}"' +
>>> - ' -o -n "$COVERITY_BUILD_COMMAND$COVERITY_LD_PRELOAD"',
>>> -)
>>> -if rc.returncode() == 0
>>> - conf.set('STATIC_ANALYSIS', 1)
>>> -endif
>>> -
>>> -
>>> # Add RPATH information when building for a non-standard prefix, or
>>> # when explicitly requested to do so
>>>
>>> diff --git a/src/internal.h b/src/internal.h
>>> index d167e56b48..5226667d3d 100644
>>> --- a/src/internal.h
>>> +++ b/src/internal.h
>>> @@ -29,6 +29,10 @@
>>> #include <stdlib.h>
>>> #include "glibcompat.h"
>>>
>>> +#if defined __clang_analyzer__ || defined __COVERITY__
>>
>> ^^ Bah humbug ... what defines __COVERITY__ then?
>>
>> In my Coverity environment, nothing... OK, sure I can add it, no problem
>> but something in QEMU's coverity build environment must do that as it's
>> not predefined as far as I can tell.
>
> Hi John,
>
> There is no need to add it anywhere. When building something using
> coverity it is indeed not defined so GCC will ignore any code guarded
> with __COVERITY__.
>
> The __COVERITY__ is defined by cov-emit binary which is executed by
> cov-build internally which creates the files that are later analyzed.
>
> This is directly from cov-emit --help:
>
> Description
>
> The cov-emit command parses a source file and outputs it into a directory
> (emit repository) that can later be analyzed with cov-analyze. The
> cov-emit command is typically called by cov-translate, which is in turn
> typically called by cov-build (cov-emit is a low-level command and is not
> normally called directly). The cov-emit command defines the __COVERITY__
> preprocessor symbol.
>
> Pavel
>
Well it didn't seem to get defined in my case; otherwise, I wouldn't
have noted it here. ;-) Of course more research ends up figuring this
out <read on if truly interested>.
Prior to this change, STATIC_ANALYSIS was defined in my <build-dir>/
meson-config.h file because COVERITY_LD_PRELOAD is defined. After this
change it is not defined there.
After this change, I got a build error because I have some extra
sa_assert's in my local sources to avoid various Coverity warnings. One
of those is in qemu_agent.c in the qemuAgentCommandFull where I had to add:
+ if (ret == 0)
+ sa_assert(*reply);
just prior to the return. This resulted in the build error:
../src/qemu/qemu_agent.c: In function ‘qemuAgentCommandFull’:
../src/qemu/qemu_agent.c:1137:26: error: suggest braces around empty
body in an ‘if’ statement [-Werror=empty-body]
1137 | sa_assert(*reply);
| ^
cc1: all warnings being treated as errors
[10/305] Compiling C object
src/qemu/l..._driver_qemu_impl.a.p/qemu_hotplug.c.o
ninja: build stopped: subcommand failed.
which implied to me that sa_assert wasn't defined as a result of a
coding error if sa_assert wasn't defined.
FWIW:
This was done to avoid callers complaining because *reply was set to
NULL before a goto cleanup; and before being set to msg.rxObject.
Coverity has a "hard time" deciding when one variable (in this case
@ret) controls two conditions (in this case @*reply would be filled in).
So in the caller it runs a pass where the return value would 0 and
*reply = NULL - it's an impossible condition when you read the code.
Hence I added the sa_assert, which interestingly enough in this case
tripped because of missing { ... }, but that ended up being showing me
that STATIC_ANALYSIS was not defined in my build env any more.
What got a bit more confusing to me on this though is that if I fix my
local patch to have the { }, then I don't get any Coverity warnings like
I was getting when the sa_assert wasn't there. So that perhaps implies
there's a pass running with *and* without STATIC_ANALYSIS being defined.
If I look at the Coverity created build-log.txt I see two passes being made:
...
[1182705] EXECUTING: /bin/sh -c "gcc ... qemu_agent.c"
...
[1182707] COMPILING: /opt/cov-analysis-linux64-2020.09/bin/cov-translate
gcc ... qemu_agent.c
...
So I believe I have my answer <sigh>...
John
