* [Bug analyzer/104224] Testcases for analyzer "uninit" from fedora-devel
2022-01-25 14:23 [Bug analyzer/104224] New: Testcases for analyzer "uninit" from fedora-devel dmalcolm at gcc dot gnu.org
@ 2022-01-25 14:28 ` dmalcolm at gcc dot gnu.org
2022-01-26 14:45 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2022-01-25 14:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104224
--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
gcc trunk with -fanalyzer: https://godbolt.org/z/T17TbqYdx
^ permalink raw reply [flat|nested] 7+ messages in thread* [Bug analyzer/104224] Testcases for analyzer "uninit" from fedora-devel
2022-01-25 14:23 [Bug analyzer/104224] New: Testcases for analyzer "uninit" from fedora-devel dmalcolm at gcc dot gnu.org
2022-01-25 14:28 ` [Bug analyzer/104224] " dmalcolm at gcc dot gnu.org
@ 2022-01-26 14:45 ` cvs-commit at gcc dot gnu.org
2022-01-27 23:04 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-01-26 14:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104224
--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
https://gcc.gnu.org/g:9ff3e2368d86c1bf7d1e8876a14e58c0d6617ffe
commit r12-6876-g9ff3e2368d86c1bf7d1e8876a14e58c0d6617ffe
Author: David Malcolm <dmalcolm@redhat.com>
Date: Tue Jan 25 14:10:46 2022 -0500
analyzer: fix missing uninit warning on args to stdio builtins [PR104224]
We were failing to check for uninitialized arguments to stdio builtins,
such as when passing local "go" to the call to "printf" in "main" in
the testcase.
gcc/analyzer/ChangeLog:
PR analyzer/104224
* region-model.cc (region_model::check_call_args): New.
(region_model::on_call_pre): Call it when ignoring stdio builtins.
* region-model.h (region_model::check_call_args): New decl
gcc/testsuite/ChangeLog:
PR analyzer/104224
* gcc.dg/analyzer/pr104224.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [Bug analyzer/104224] Testcases for analyzer "uninit" from fedora-devel
2022-01-25 14:23 [Bug analyzer/104224] New: Testcases for analyzer "uninit" from fedora-devel dmalcolm at gcc dot gnu.org
2022-01-25 14:28 ` [Bug analyzer/104224] " dmalcolm at gcc dot gnu.org
2022-01-26 14:45 ` cvs-commit at gcc dot gnu.org
@ 2022-01-27 23:04 ` cvs-commit at gcc dot gnu.org
2022-02-09 22:46 ` dmalcolm at gcc dot gnu.org
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-01-27 23:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104224
--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
https://gcc.gnu.org/g:00e7d024afb80e95fb36d74b1c059468d883a850
commit r12-6906-g00e7d024afb80e95fb36d74b1c059468d883a850
Author: David Malcolm <dmalcolm@redhat.com>
Date: Wed Jan 26 16:24:08 2022 -0500
analyzer: show region creation events for uninit warnings
When reviewing the output of -fanalyzer on PR analyzer/104224 I noticed
that despite very verbose paths, the diagnostic paths for
-Wanalyzer-use-of-uninitialized-value
don't show where the uninitialized memory is allocated.
This patch adapts and simplifies material from
"[PATCH 3/6] analyzer: implement infoleak detection"
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584377.html
in order to add region creation events for the pertinent region (whether
on the stack or heap).
For example, this patch extends:
malloc-1.c: In function 'test_40':
malloc-1.c:461:5: warning: use of uninitialized value '*p' [CWE-457]
[-Wanalyzer-use-of-uninitialized-value]
461 | i = *p;
| ~~^~~~
'test_40': event 1
|
| 461 | i = *p;
| | ~~^~~~
| | |
| | (1) use of uninitialized value '*p' here
|
to:
malloc-1.c: In function 'test_40':
malloc-1.c:461:5: warning: use of uninitialized value '*p' [CWE-457]
[-Wanalyzer-use-of-uninitialized-value]
461 | i = *p;
| ~~^~~~
'test_40': events 1-2
|
| 460 | int *p = (int*)malloc(sizeof(int*));
| | ^~~~~~~~~~~~~~~~~~~~
| | |
| | (1) region created on heap here
| 461 | i = *p;
| | ~~~~~~
| | |
| | (2) use of uninitialized value '*p' here
|
and this helps readability of the resulting warnings, especially in
more complicated cases.
gcc/analyzer/ChangeLog:
* checker-path.cc (event_kind_to_string): Handle
EK_REGION_CREATION.
(region_creation_event::region_creation_event): New.
(region_creation_event::get_desc): New.
(checker_path::add_region_creation_event): New.
* checker-path.h (enum event_kind): Add EK_REGION_CREATION.
(class region_creation_event): New subclass.
(checker_path::add_region_creation_event): New decl.
* diagnostic-manager.cc
(diagnostic_manager::emit_saved_diagnostic): Pass NULL for new
param to add_events_for_eedge when handling trailing eedge.
(diagnostic_manager::build_emission_path): Create an interesting_t
instance, allow the pending diagnostic to populate it, and pass it
to the calls to add_events_for_eedge.
(diagnostic_manager::add_events_for_eedge): Add "interest" param.
Use it to add region_creation_events for on-stack regions created
within at function entry, and when pertinent dynamically-sized
regions are created.
(diagnostic_manager::prune_for_sm_diagnostic): Add case for
EK_REGION_CREATION.
* diagnostic-manager.h (diagnostic_manager::add_events_for_eedge):
Add "interest" param.
* pending-diagnostic.cc: Include "selftest.h", "tristate.h",
"analyzer/call-string.h", "analyzer/program-point.h",
"analyzer/store.h", and "analyzer/region-model.h".
(interesting_t::add_region_creation): New.
(interesting_t::dump_to_pp): New.
* pending-diagnostic.h (struct interesting_t): New.
(pending_diagnostic::mark_interesting_stuff): New vfunc.
* region-model.cc
(poisoned_value_diagnostic::poisoned_value_diagnostic): Add
(poisoned_value_diagnostic::operator==): Compare m_pkind and
m_src_region fields.
(poisoned_value_diagnostic::mark_interesting_stuff): New.
(poisoned_value_diagnostic::m_src_region): New.
(region_model::check_for_poison): Call
get_region_for_poisoned_expr for uninit values and pass the resul
to the diagnostic.
(region_model::get_region_for_poisoned_expr): New.
(region_model::deref_rvalue): Pass NULL for
poisoned_value_diagnostic's src_region.
* region-model.h (region_model::get_region_for_poisoned_expr): New
decl.
* region.h (frame_region::get_fndecl): New.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/data-model-1.c: Add dg-message directives for
expected region creation events.
* gcc.dg/analyzer/malloc-1.c: Likewise.
* gcc.dg/analyzer/memset-CVE-2017-18549-1.c: Likewise.
* gcc.dg/analyzer/pr101547.c: Likewise.
* gcc.dg/analyzer/pr101875.c: Likewise.
* gcc.dg/analyzer/pr101962.c: Likewise.
* gcc.dg/analyzer/pr104224.c: Likewise.
* gcc.dg/analyzer/pr94047.c: Likewise.
* gcc.dg/analyzer/symbolic-1.c: Likewise.
* gcc.dg/analyzer/uninit-1.c: Likewise.
* gcc.dg/analyzer/uninit-4.c: Likewise.
* gcc.dg/analyzer/uninit-alloca.c: New test.
* gcc.dg/analyzer/uninit-pr94713.c: Add dg-message directive for
expected region creation event.
* gcc.dg/analyzer/uninit-pr94714.c: Likewise.
* gcc.dg/analyzer/zlib-3.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [Bug analyzer/104224] Testcases for analyzer "uninit" from fedora-devel
2022-01-25 14:23 [Bug analyzer/104224] New: Testcases for analyzer "uninit" from fedora-devel dmalcolm at gcc dot gnu.org
` (2 preceding siblings ...)
2022-01-27 23:04 ` cvs-commit at gcc dot gnu.org
@ 2022-02-09 22:46 ` dmalcolm at gcc dot gnu.org
2023-02-21 22:05 ` cvs-commit at gcc dot gnu.org
2023-02-21 22:29 ` dmalcolm at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2022-02-09 22:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104224
David Malcolm <dmalcolm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I've added further test coverage for "uninit" based on a discussion with Steve
Grubb, as r12-7157-g91b27d984ce174.
Closing this one out.
^ permalink raw reply [flat|nested] 7+ messages in thread* [Bug analyzer/104224] Testcases for analyzer "uninit" from fedora-devel
2022-01-25 14:23 [Bug analyzer/104224] New: Testcases for analyzer "uninit" from fedora-devel dmalcolm at gcc dot gnu.org
` (3 preceding siblings ...)
2022-02-09 22:46 ` dmalcolm at gcc dot gnu.org
@ 2023-02-21 22:05 ` cvs-commit at gcc dot gnu.org
2023-02-21 22:29 ` dmalcolm at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-02-21 22:05 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104224
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
https://gcc.gnu.org/g:8f6369157917a4371b5d66dfe82b84aded3b8268
commit r13-6267-g8f6369157917a4371b5d66dfe82b84aded3b8268
Author: David Malcolm <dmalcolm@redhat.com>
Date: Tue Feb 21 16:58:36 2023 -0500
analyzer: stop exploring the path after certain diagnostics [PR108830]
PR analyzer/108830 reports a situation in which there are lots of
followup -Wanalyzer-null-dereference warnings after the first access of
a NULL pointer, leading to very noisy output from -fanalyzer.
The analyzer's logic for stopping emitting multiple warnings from a
state machine doesn't quite work for NULL pointers: it attempts to
transition the malloc state machine's NULL pointer to the "stop" state,
which doesn't seem to make much sense in retrospect, and seems to get
confused over types.
Similarly, poisoned_value_diagnostic can be very noisy for uninit
variables, emitting a warning for every access to an uninitialized
variable. In theory, region_model::check_for_poison makes some attempts
to suppress followups, but only for the symbolic value itself; if the
user's code keeps accessing the same region, we would get a warning on
each one. For example, this showed up in Doom's s_sound.c where there
were 7 followup uninit warnings after the first uninit warning in
"S_ChangeMusic".
This patch adds an extra mechanism, giving pending diagnostics the
option of stopping the analysis of an execution path if they're saved
for emission on it, and turning this on for these warnings:
-Wanalyzer-null-dereference
-Wanalyzer-null-argument
-Wanalyzer-use-after-free
-Wanalyzer-use-of-pointer-in-stale-stack-frame
-Wanalyzer-use-of-uninitialized-value
Doing so should hopefully reduce the cascades of diagnostics that
-fanalyzer can sometimes emit.
I added a -fno-analyzer-suppress-followups for the cases where you
really want the followup warnings (e.g. in some DejaGnu tests, and
for microbenchmarks of UB detection, such as PR analyzer/104224).
Integration testing shows this patch reduces the number of probable
false positives reported by 94, and finds one more true positive:
Comparison: 9.34% -> 10.91%
GOOD: 66 -> 67 (+1)
BAD: 641 -> 547 (-94)
where the affected warnings/projects are:
-Wanalyzer-null-dereference: 0.00% GOOD: 0 BAD: 269 -> 239 (-30)
Unclassified: 257 -> 228 (-29)
apr-1.7.0: 12 -> 5 (-7)
doom: 1 -> 0 (-1)
haproxy-2.7.1: 47 -> 41 (-6)
ImageMagick-7.1.0-57: 13 -> 9 (-4)
qemu-7.2.0: 165 -> 154 (-11)
Known false: 7 -> 6 (-1)
xz-5.4.0: 4 -> 3 (-1)
-Wanalyzer-use-of-uninitialized-value: 0.00% GOOD: 0 BAD: 143 -> 80 (-63)
Known false: 47 -> 16 (-31)
doom: 42 -> 11 (-31)
Unclassified: 96 -> 64 (-32)
coreutils-9.1: 14 -> 10 (-4)
haproxy-2.7.1: 29 -> 23 (-6)
qemu-7.2.0: 48 -> 26 (-22)
-Wanalyzer-null-argument: 0.00% -> 2.33% GOOD: 0 -> 1 (+1) BAD: 43 -> 42
(-1)
Unclassified: 39 -> 38 (-1)
due to coreutils-9.1: 9 -> 8 (-1)
True positive: 0 -> 1 (+1)
(in haproxy-2.7.1)
gcc/analyzer/ChangeLog:
PR analyzer/108830
* analyzer.opt (fanalyzer-suppress-followups): New option.
* engine.cc (impl_region_model_context::warn): Terminate the path
if the diagnostic's terminate_path_p vfunc returns true and
-fanalyzer-suppress-followups is true (the default).
(impl_sm_context::warn): Likewise, for both overloads.
* pending-diagnostic.h (pending_diagnostic::terminate_path_p): New
vfunc.
* program-state.cc (program_state::on_edge): Terminate the path if
the ctxt requests it during updating the edge.
* region-model.cc (poisoned_value_diagnostic::terminate_path_p):
New vfunc.
* sm-malloc.cc (null_deref::terminate_path_p): New vfunc.
(null_arg::terminate_path_p): New vfunc.
gcc/ChangeLog:
PR analyzer/108830
* doc/invoke.texi: Document -fno-analyzer-suppress-followups.
gcc/testsuite/ChangeLog:
PR analyzer/108830
* gcc.dg/analyzer/attribute-nonnull.c: Update for
-Wanalyzer-use-of-uninitialized-value terminating analysis along
a path.
* gcc.dg/analyzer/call-summaries-2.c: Likewise.
* gcc.dg/analyzer/data-model-1.c: Likewise.
* gcc.dg/analyzer/data-model-5.c: Likewise.
* gcc.dg/analyzer/doom-s_sound-pr108867.c: New test.
* gcc.dg/analyzer/memset-CVE-2017-18549-1.c: Add
-fno-analyzer-suppress-followups.
* gcc.dg/analyzer/null-deref-pr108830.c: New test.
* gcc.dg/analyzer/pipe-1.c: Add -fno-analyzer-suppress-followups.
* gcc.dg/analyzer/pipe-void-return.c: Likewise.
* gcc.dg/analyzer/pipe2-1.c: Likewise.
* gcc.dg/analyzer/pr101547.c: Update for
-Wanalyzer-use-of-uninitialized-value terminating analysis along
a path.
* gcc.dg/analyzer/pr101875.c: Likewise.
* gcc.dg/analyzer/pr104224-split.c: New test, based on...
* gcc.dg/analyzer/pr104224.c: Add
-fno-analyzer-suppress-followups.
* gcc.dg/analyzer/realloc-2.c: Add
-fno-analyzer-suppress-followups.
* gcc.dg/analyzer/realloc-3.c: Likewise.
* gcc.dg/analyzer/realloc-5.c: Likewise.
* gcc.dg/analyzer/stdarg-1-ms_abi.c: Likewise.
* gcc.dg/analyzer/stdarg-1-sysv_abi.c: Likewise.
* gcc.dg/analyzer/stdarg-1.c: Likewise.
* gcc.dg/analyzer/symbolic-1.c: Likewise.
* gcc.dg/analyzer/symbolic-7.c: Update for
-Wanalyzer-use-of-uninitialized-value terminating analysis along a
path.
* gcc.dg/analyzer/uninit-4.c: Likewise.
* gcc.dg/analyzer/uninit-8.c: New test.
* gcc.dg/analyzer/uninit-pr94713.c: Update for
-Wanalyzer-use-of-uninitialized-value terminating analysis along a
path.
* gcc.dg/analyzer/zlib-6a.c: Add -fno-analyzer-suppress-followups.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [Bug analyzer/104224] Testcases for analyzer "uninit" from fedora-devel
2022-01-25 14:23 [Bug analyzer/104224] New: Testcases for analyzer "uninit" from fedora-devel dmalcolm at gcc dot gnu.org
` (4 preceding siblings ...)
2023-02-21 22:05 ` cvs-commit at gcc dot gnu.org
@ 2023-02-21 22:29 ` dmalcolm at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-02-21 22:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104224
--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Given the above patch, we now need -fno-analyzer-suppress-followups if you want
to see all the warnings in the testcase (rather than just stopping after the
first).
^ permalink raw reply [flat|nested] 7+ messages in thread