Ericsson / codechecker

CodeChecker is an analyzer tooling, defect database and viewer extension for static and dynamic analyzer tools.
https://codechecker.readthedocs.io
Apache License 2.0
2.27k stars 381 forks source link

`__clang_analyzer__` not defined in headers? #3676

Open dfarley1 opened 2 years ago

dfarley1 commented 2 years ago

I'm working on adding CodeChecker support for our codebase and the docs mention that you should use assert() to guide clangsa around false positives. Our codebase doesn't currently use assert() so I went to add it as a macro guarded by __clang_analyzer__ so that it doesn't affect our normal builds:

(def.h)
#ifdef __clang_analyzer__
#include <assert.h>
#define ANALYZER_ASSSERT(_x) assert(_x)
#else
#define ANALYZER_ASSERT(_x) ;
#endif

I have some code in api.c that looks like

(api.c)
#include "def.h"
...

    uint64_t cycle_len = cycle_length_sec(...) * 1000UL;
    ...
    from = ROUNDDOWN(from, cycle_len);
}

Which produces this error:

[HIGH] /depot/.../api.c:2665:12: Division by zero [core.DivideZero]
    from = ROUNDDOWN(from, cycle_len);
           ^
  Report hash: 3f8e83794357ed3de3e4313f66e20b0a
  Macro expansions:
    1, api.c:2665:12: Macro 'ROUNDDOWN(from, cycle_len)' expanded to '(((from )/(cycle_len ))*(cycle_len ))'
  Steps:
    1, api.c:2836:9: Assuming field '...' is not equal to 0
    2, api.c:2840:9: Assuming field '...' is true
    3, api.c:2841:16: Calling 'this_function'
    4, api.c:2632:1: Entered call from 'caller_function'
    5, api.c:2648:5: 'cycle_len' initialized to 0
    6, api.c:2665:12: Division by zero

We know that cycle_length_sec() can technically, but never practically, return 0, so I added our assert right above the line:

(api.c)
#include "def.h"
...

    uint64_t cycle_len = cycle_length_sec(...) * 1000UL;
    ...
    ANALYZER_ASSERT(cycle_len != 0);
    from = ROUNDDOWN(from, cycle_len);

But the warning keeps appearing, and clangsa doesn't seem to acknowledge the macro in its list of Macro expansions like it does for ROUNDDOWN. However, if I move the ifdef block into api.c then the warning is suppressed as I'd expect:

(api.c)
#include "def.h"
...

    uint64_t cycle_len = cycle_length_sec(...) * 1000UL;
    ...
#ifdef __clang_analyzer__
    #include <assert.h>
    assert(cycle_len != 0);
#endif
    from = ROUNDDOWN(from, cycle_len);

So for some reason it seems like __clang_analyzer__ is defined in api.c but not in def.h, even though the latter is directly #included from the former??

I tried running the analyzer with debug logging turned on and I do see -D__clang_analyzer__ in the CTU portion of the analysis:

[DEBUG_ANALYZER][2022-05-31 15:31:44] {analyzer} [3639438] <139897325340480> - ctu_manager.py:128 generate_ast() - Generating AST using '/usr/lib/llvm/13/bin/clang-13 --target=x86_64-pc-linux-gnu -isystem /usr/include -c -x c -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -I(some paths) -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -Wno-format-truncation -Wno-stringop-truncation -fstack-protector-strong -iquote (some paths) -fPIC -fno-common /depot/.../api.c -std=gnu17 -emit-ast -D__clang_analyzer__ -w -o /depot/out/CodeChecker/codechecker/ctu-dir/x86_64/ast/depot/extrahop/bridge/base/api.c.ast'

But I do not see it in the analysis run itself:

[DEBUG_ANALYZER][2022-05-31 15:32:23] {analyzer} [3643024] <139897325340480> - analyzer_base.py:86 analyze() - /usr/lib/llvm/13/bin/clang-13 --analyze -Qunused-arguments -Xclang -analyzer-opt-analyze-headers -Xclang -analyzer-output=plist-multi-file -o /depot/out/CodeChecker/codechecker/api.c_clangsa_0a49ab7edc14c2d34deb130dd8dfe00a.plist -Xclang -analyzer-config -Xclang expand-macros=true -Xclang -analyzer-checker=alpha.security.cert.pos.34c,core.CallAndMessage,core.DivideZero,core.NonNullParamChecker,core.NullDereference,core.StackAddressEscape,core.UndefinedBinaryOperatorResult,core.VLASize,core.uninitialized.ArraySubscript,core.uninitialized.Assign,core.uninitialized.Branch,core.uninitialized.CapturedBlockVariable,core.uninitialized.UndefReturn,cplusplus.InnerPointer,cplusplus.Move,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,cplusplus.PlacementNew,cplusplus.PureVirtualCall,deadcode.DeadStores,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,optin.cplusplus.UninitializedObject,optin.cplusplus.VirtualCall,optin.portability.UnixAPI,security.FloatLoopCounter,security.insecureAPI.UncheckedReturn,security.insecureAPI.getpw,security.insecureAPI.gets,security.insecureAPI.mkstemp,security.insecureAPI.mktemp,security.insecureAPI.rand,security.insecureAPI.vfork,unix.API,unix.Malloc,unix.MallocSizeof,unix.MismatchedDeallocator,unix.Vfork,unix.cstring.BadSizeArg,unix.cstring.NullArg,valist.CopyToSelf,valist.Uninitialized,valist.Unterminated -Xclang -analyzer-config -Xclang aggressive-binary-operation-simplification=true -Xclang -analyzer-config -Xclang experimental-enable-naive-ctu-analysis=true -Xclang -analyzer-config -Xclang ctu-dir=/depot/out/CodeChecker/codechecker/ctu-dir/x86_64 -Xclang -analyzer-config -Xclang display-ctu-progress=true -x c --target=x86_64-pc-linux-gnu -std=gnu17 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -I(some paths) -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -Wno-format-truncation -Wno-stringop-truncation -fstack-protector-strong -iquote (some paths) -fPIC -fno-common -isystem /usr/include /depot/extrahop/bridge/base/api.c

Is this something I need to go bug LLVM about or is there something going on with how CodeChecker invokes it?

CodeChecker version v6.19.1

steakhal commented 2 years ago

It seems working on my side. However, I noted you had a typo: ANALYZER_ASSSERT -> ANALYZER_ASSERT It should fix your issue. Let me know.

dfarley1 commented 2 years ago

Ha, sorry the typo is an artifact of me copy/pasting/sanitizing for this issue, I've double checked that things are correctly spelled in the actual code.

steakhal commented 2 years ago

I could not reproduce your issue.

cat > def.h << EOF
#ifdef __clang_analyzer__
  #include <assert.h>
  #define ANALYZER_ASSERT(x) assert(x)
#else
  #define ANALYZER_ASSERT(x) do {} while (0)
#endif
EOF

cat > api.c << EOF
#include "def.h"

int foo(int x) {
  ANALYZER_ASSERT(x == 2);
  return x / (x - 2);
}
EOF

CodeChecker log -b "gcc -Wall -Wextra -c api.c -o /dev/null" -o db.json
CodeChecker analyze db.json -o reports --verbose debug

After this we should expect a div by zero error because the ANALYZER_ASSERT(x == 2) introduced the assumption that x must be equal to 2, thus x - 2 is zero causing the div by zero. Consequently, The analyzer correctly expanded the conditional #ifdef __clang_analyzer_ and selected the assert(cond) version of the macro.

I'll close this issue unless you can give a clean end-to-end reproducer.