-
Notifications
You must be signed in to change notification settings - Fork 14.6k
Description
After upgrade from LLVM-21 snapshot fcb4bda to 7dc7c15 (21.0.0_pre20250510 to 21.0.0_pre20250523), we are seeing numerous false positives and nonsensical output from clang-tidy
, for example:
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:5322:9: note: Uninitialized value stored to field 'used'
5322 | XMEMCPY(r->dp, a->dp, a->used * (word32)SP_WORD_SIZEOF);
| ^
./wolfssl/wolfcrypt/types.h:801:31: note: expanded from macro 'XMEMCPY'
801 | #define XMEMCPY(d,s,l) memcpy((d),(s),(l))
| ^~~~~~~~~~~~~~~~~~~
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:5325:13: note: Assigned value is uninitialized
5325 | r->used = a->used;
| ^ ~~~~~~~
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:8541:47: warning: The right operand of '>' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
8541 | else if ((err == MP_OKAY) && (a->used - i > r->size)) {
| ^
(XMEMCPY
is a macro that reduces to memcpy
in this build.)
The note that an uninited value was stored to used
by the memcpy
makes no sense -- the dp
slot is an inline array at the end of the struct (r
and a
are both sp_int
structs).
To be perfectly clear, the code at issue functions correctly, is clean on numerous other static and dynamic analyzers, and produces no warnings or notes on 21.0.0_pre20250510, all else equal.
In all, we saw these new false positives on 21.0.0_pre20250523:
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:5325:13: warning: Assigned value is uninitialized [clang-analyzer-core.uninitialized.Assign]
5325 | r->used = a->used;
| ^
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:8541:47: warning: The right operand of '>' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
8541 | else if ((err == MP_OKAY) && (a->used - i > r->size)) {
| ^
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:14137:15: warning: 3rd function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
14137 | err = sp_exptmod_ex(b, e, (int)e->used, m, r);
| ^
/home/wolfbot/tmp/wolfssl_test_workdir.17563/wolfssl/wolfcrypt/src/sp_int.c:17339:54: warning: The right operand of '>' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
17339 | if ((err == MP_OKAY) && (r != m) && (a->used * 2 > r->size)) {
| ^
The code under test is at https://github.com/wolfssl/wolfssl at commit 6c7edeba38, and the configuration under test in the above is
./configure --enable-all --enable-testcert --enable-acert --enable-dtls13 --enable-dtls-mtu --enable-dtls-frag-ch --enable-dtlscid --enable-quic --with-sys-crypto-policy --enable-sp-math-all CFLAGS='-Wunreachable-code-aggressive -Wthread-safety -Wloop-analysis -Wenum-compare-conditional -fcolor-diagnostics -fcomplete-member-pointers -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wno-language-extension-token -DTEST_ALWAYS_RUN_TO_END -g -fdebug-types-section -Wunreachable-code-break -Wunreachable-code-return -Wimplicit-fallthrough -DWOLFSSL_SP_INT_NEGATIVE -DKEEP_OUR_CERT -DKEEP_PEER_CERT -DWOLFSSL_ALT_NAMES -DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK -pedantic -Wdeclaration-after-statement -DTEST_LIBWOLFSSL_SOURCES_INCLUSION_SEQUENCE -DSP_ALLOC -DWOLFSSL_CLANG_TIDY -DNO_WOLFSSL_MEMORY'
(With a locally developed helper script, clang-tidy-builder.sh
, passed in as CC
.)
We have a complicated clang-tidy
configuration, but for core
checkers it is only enabling and disabling whole checkers, not frobbing their internal settings.
Activity
llvmbot commentedon May 26, 2025
@llvm/issue-subscribers-clang-tidy
Author: Daniel Pouzzner (douzzer)
The note that an uninited value was stored to
used
by thememcpy
makes no sense -- thedp
slot is an inline array at the end of the struct (r
anda
are bothsp_int
structs).To be perfectly clear, the code at issue functions correctly, is clean on numerous other static and dynamic analyzers, and produces no warnings or notes on 21.0.0_pre20250510, all else equal.
In all, we saw these new false positives on 21.0.0_pre20250523:
The code under test is at https://github.com/wolfssl/wolfssl at commit 6c7edeba38, and the configuration under test in the above is
(With a locally developed helper script,
clang-tidy-builder.sh
, passed in asCC
.)We have a complicated
clang-tidy
configuration, but forcore
checkers it is only enabling and disabling whole checkers, not frobbing their internal settings.douzzer commentedon May 26, 2025
Note that 21.0.0-pre20250517 (aaaae99) throws the same false positives. So the regression is narrowed to the range fcb4bda...aaaae99.
llvmbot commentedon May 26, 2025
@llvm/issue-subscribers-clang-static-analyzer
Author: Daniel Pouzzner (douzzer)
The note that an uninited value was stored to
used
by thememcpy
makes no sense -- thedp
slot is an inline array at the end of the struct (r
anda
are bothsp_int
structs).To be perfectly clear, the code at issue functions correctly, is clean on numerous other static and dynamic analyzers, and produces no warnings or notes on 21.0.0_pre20250510, all else equal.
In all, we saw these new false positives on 21.0.0_pre20250523:
The code under test is at https://github.com/wolfssl/wolfssl at commit 6c7edeba38, and the configuration under test in the above is
(With a locally developed helper script,
clang-tidy-builder.sh
, passed in asCC
.)We have a complicated
clang-tidy
configuration, but forcore
checkers it is only enabling and disabling whole checkers, not frobbing their internal settings.steakhal commentedon May 27, 2025
Thank you for your report. The commit range narrows it down to these 4 commits:
81d48e0 [clang][analyzer] Fix a nullptr dereference when -ftime-trace is used (Reland) (#139980)
4122958 [analyzer] Fix crashing __builtin_bit_cast (#139188)
6078f5e Reland [Clang][analyzer] replace Stmt* with ConstCFGElement in SymbolConjured (#137355)
9600a12 [analyzer] Workaround for slowdown spikes (unintended scope increase) (#136720)
One of these is the one to blame.
Could you give the preprocessed output of the translation unit that raises your issue?
[-][clang-tidy] New false positives from clang-analyzer-core[/-][+][analyzer] New false positives from clang-analyzer-core[/+]douzzer commentedon May 27, 2025
@steakhal sure thing, see attached.
sp_int.preprocessed.c.gz
The one-liner that generated it:
preprocess-sp_int.sh.gz
And a one-liner that compiles it:
compile-sp_int.sh.gz
That latter, oddly, only worked if I added
-Wno-gnu-line-marker -Wno-parentheses-equality
to the arg list, but whatever.steakhal commentedon May 28, 2025
Thanks for the artifacts. I managed to reproduce the 4 warnings you shared.
After reverting 9600a12 I only have a single warning:
Is this expected, or that's also a problem?
/cc @NagyDonat - the author of #136720
douzzer commentedon May 28, 2025
@steakhal not expected, not accurate, and not present with 21.0.0_pre20250510.
steakhal commentedon May 28, 2025
Well, than that must be also another regression. I used 29db305 as HEAD for my testing (2025.05.27 14:39:52).
douzzer commentedon May 28, 2025
Ah, so you don't have a quick+easy way to see if reverting 9600a12 relative to aaaae99 gives a clean run. That would be nice to know, but the other false positive needs to be fixed anyway, so may as well chase it down too.
steakhal commentedon May 28, 2025
I could check out the exact version you tested. And I'm sure it would demonstrate the behvior you say it has.
I used this recent HEAD because I had a hot ccache for it.
douzzer commentedon May 28, 2025
@steakhal correction to the above -- if I run
clang-tidy
from 21.0.0_pre20250510 directly on the preprocessed file, I see that warning you see. It's only if I run it on the unpreprocessed file that it's clean. There is aNOLINTBEGIN ... NOLINTEND
span around line 12010, which is snipped out by the preprocessing step (whoopsadoodle).The existing suppression was also to work around a false positive, but it's old and you shouldn't be distracted by it.
So it looks like the reversion you tested is all that's needed to fix the regression.