9:31 a.m.
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(a)redhat.com>
>> ---
>> 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