| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
PR analyzer/108733 reports various false positives in qemu from
-Wanalyzer-use-of-uninitialized-value with __attribute__((cleanup))
at -O1 and above.
Root cause is that the state-purging code was failing to treat:
_25 = MEM[(void * *)&val];
as a usage of "val", leading to it erroneously purging the
initialization of "val" along an execution path that didn't otherwise
use "val", apart from the __attribute__((cleanup)).
Fixed thusly.
Integration testing on the patch show this change in the number of
diagnostics:
-Wanalyzer-use-of-uninitialized-value
coreutils-9.1: 18 -> 16 (-2)
qemu-7.2.0: 87 -> 80 (-7)
where all that I investigated appear to have been false positives, hence
an improvement.
gcc/analyzer/ChangeLog:
PR analyzer/108733
* state-purge.cc (get_candidate_for_purging): Add ADDR_EXPR
and MEM_REF.
gcc/testsuite/ChangeLog:
PR analyzer/108733
* gcc.dg/analyzer/torture/uninit-pr108733.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/108704 reports many false positives seen from
-Wanalyzer-use-of-uninitialized-value on qemu's softfloat.c on code like
the following:
struct st s;
s = foo ();
s = bar (s); // bogusly reports that s is uninitialized here
where e.g. "struct st" is "floatx80" in the qemu examples.
The root cause is overzealous purging of on-stack structs in the code I
added in r12-7718-gfaacafd2306ad7, where at:
s = bar (s);
state_purge_per_decl::process_point_backwards "sees" the assignment to 's'
and stops processing, effectively treating 's' as unneeded before this
stmt, not noticing the use of 's' in the argument.
Fixed thusly.
The patch greatly reduces the number of
-Wanalyzer-use-of-uninitialized-value warnings from my integration tests:
ImageMagick-7.1.0-57: 10 -> 6 (-4)
qemu-7.2: 858 -> 87 (-771)
haproxy-2.7.1: 1 -> 0 (-1)
All of the above that I've examined appear to be false positives.
gcc/analyzer/ChangeLog:
PR analyzer/108704
* state-purge.cc (state_purge_per_decl::process_point_backwards):
Don't stop processing the decl if it's fully overwritten by
this stmt if it's also used by this stmt.
gcc/testsuite/ChangeLog:
PR analyzer/108704
* gcc.dg/analyzer/uninit-7.c: New test.
* gcc.dg/analyzer/uninit-pr108704.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
[PR108661]
My integration testing shows many false positives from
-Wanalyzer-use-of-uninitialized-value.
One cause turns out to be that as of r13-1404-g97baacba963c06
fd_state_machine::on_stmt recognizes calls to "read", and returns true,
so that region_model::on_call_post doesn't call handle_unrecognized_call
on them, and so the analyzer erroneously "thinks" that the buffer
pointed to by "read" is never touched by the "read" call.
This works for "fread" because sm-file.cc implements kf_fread, which
handles calls to "fread" by clobbering the buffer pointed to. In the
long term we should probably be smarter about this and bifurcate the
analysis to consider e.g. errors vs full reads vs partial reads, etc
(which I'm tracking in PR analyzer/108689).
In the meantime, this patch adds a kf_read for "read" analogous to the
one for "fread", fixing 6 false positives seen in git-2.39.0 and
2 in haproxy-2.7.1.
gcc/analyzer/ChangeLog:
PR analyzer/108661
* sm-fd.cc (class kf_read): New.
(register_known_fd_functions): Register "read".
* sm-file.cc (class kf_fread): Update comment.
gcc/testsuite/ChangeLog:
PR analyzer/108661
* gcc.dg/analyzer/fread-pr108661.c: New test.
* gcc.dg/analyzer/read-pr108661.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/108633
* sm-fd.cc (fd_state_machine::check_for_fd_attrs): Add missing
"continue".
(fd_state_machine::on_listen): Don't issue phase-mismatch or
type-mismatch warnings for the "invalid" state.
gcc/testsuite/ChangeLog:
PR analyzer/108633
* gcc.dg/analyzer/fd-pr108633.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/108616
* pending-diagnostic.cc (fixup_location_in_macro_p): Add "alloca"
to macros that we shouldn't unwind inside.
gcc/testsuite/ChangeLog:
PR analyzer/108616
* gcc.dg/analyzer/allocation-size-multiline-3.c: New test.
* gcc.dg/analyzer/test-alloca.h: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
Reject -Wanalyzer-infinite-recursion diagnostics in which control flow
has been affected by conjured_svalues between the initial call to a
function and the subsequent entry to that function. This prevents false
positives such as in qemu's recursive JSON parser where function calls are
changing state in the rest of the program (e.g. consuming tokens), despite
the modelled state being effectively identical at both nested entrypoints.
gcc/analyzer/ChangeLog:
PR analyzer/108524
* analyzer.h (class feasible_node): New forward decl.
* diagnostic-manager.cc (epath_finder::get_best_epath): Add "pd"
param.
(epath_finder::explore_feasible_paths): Likewise.
(epath_finder::process_worklist_item): Likewise. Use it to call
pending_diagnostic::check_valid_fpath_p on the final fpath to
give pending_diagnostic a way to add additional restrictions on
feasibility.
(saved_diagnostic::calc_best_epath): Pass pending_diagnostic to
epath_finder::get_best_epath.
* infinite-recursion.cc: Include "analyzer/feasible-graph.h".
(infinite_recursion_diagnostic::check_valid_fpath_p): New.
(infinite_recursion_diagnostic::fedge_uses_conjured_svalue_p): New.
(infinite_recursion_diagnostic::expr_uses_conjured_svalue_p): New.
* pending-diagnostic.h (pending_diagnostic::check_valid_fpath_p):
New vfunc.
gcc/testsuite/ChangeLog:
PR analyzer/108524
* gcc.dg/analyzer/infinite-recursion-pr108524-1.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108524-2.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108524-qobject-json-parser.c:
New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
My integration testing [1] of -fanalyzer in GCC 13 is showing a lot of
diagnostics from the new -Wanalyzer-deref-before-check warning on
real-world C projects, and most of these seem to be false positives.
This patch updates the warning to make it much less likely to fire:
- only intraprocedural cases are now reported
- reject cases in which there are control flow paths to the check
that didn't come through the dereference, by looking at BB dominator
information. This fixes a false positive seen in git-2.39.0's
pack-revindex.c: load_revindex_from_disk (PR analyzer/108455), in
which a shared "cleanup:" section checks "data" for NULL, and
depending on how much of the function is executed "data" might or
might not have already been dereferenced.
The counts of -Wanalyzer-deref-before-check diagnostics in [1]
before/after this patch show this improvement:
Known false positives: 6 -> 0 (-6)
Known true positives: 1 -> 1
Unclassified positives: 123 -> 63 (-60)
[1] https://github.com/davidmalcolm/gcc-analyzer-integration-tests
gcc/analyzer/ChangeLog:
PR analyzer/108455
* analyzer.h (class checker_event): New forward decl.
(class state_change_event): Indent.
(class warning_event): New forward decl.
* checker-event.cc (state_change_event::state_change_event): Add
"enode" param.
(warning_event::get_desc): Update for new param of
evdesc::final_event ctor.
* checker-event.h (state_change_event::state_change_event): Add
"enode" param.
(state_change_event::get_exploded_node): New accessor.
(state_change_event::m_enode): New field.
(warning_event::warning_event): New "enode" param.
(warning_event::get_exploded_node): New accessor.
(warning_event::m_enode): New field.
* diagnostic-manager.cc
(state_change_event_creator::on_global_state_change): Pass
src_node to state_change_event ctor.
(state_change_event_creator::on_state_change): Likewise.
(null_assignment_sm_context::set_next_state): Pass NULL for
new param of state_change_event ctor.
* infinite-recursion.cc
(infinite_recursion_diagnostic::add_final_event): Update for new
param of warning_event ctor.
* pending-diagnostic.cc (pending_diagnostic::add_final_event):
Pass enode to warning_event ctor.
* pending-diagnostic.h (evdesc::final_event): Add reference to
warning_event.
* sm-malloc.cc: Include "analyzer/checker-event.h" and
"analyzer/exploded-graph.h".
(deref_before_check::deref_before_check): Initialize new fields.
(deref_before_check::emit): Reject warnings in which we were
unable to determine the enodes of the dereference and the check.
Reject warnings interprocedural warnings. Reject warnings in which
the dereference doesn't dominate the check.
(deref_before_check::describe_state_change): Set m_deref_enode.
(deref_before_check::describe_final_event): Set m_check_enode.
(deref_before_check::m_deref_enode): New field.
(deref_before_check::m_check_enode): New field.
gcc/testsuite/ChangeLog:
PR analyzer/108455
* gcc.dg/analyzer/deref-before-check-1.c: Add test coverage
involving dominance.
* gcc.dg/analyzer/deref-before-check-pr108455-1.c: New test.
* gcc.dg/analyzer/deref-before-check-pr108455-git-pack-revindex.c:
New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
|
|
Assume that switch on an enum doesn't follow an implicit default
skipping all cases when all enum values are covered by cases.
Fixes various false positives from -Wanalyzer-use-of-uninitialized-value
such as this one seen in Doom:
p_maputl.c: In function 'P_BoxOnLineSide':
p_maputl.c:151:8: warning: use of uninitialized value 'p1' [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
151 | if (p1 == p2)
| ^
'P_BoxOnLineSide': events 1-5
|
| 115 | int p1;
| | ^~
| | |
| | (1) region created on stack here
| | (2) capacity: 4 bytes
|......
| 118 | switch (ld->slopetype)
| | ~~~~~~
| | |
| | (3) following 'default:' branch...
|......
| 151 | if (p1 == p2)
| | ~
| | |
| | (4) ...to here
| | (5) use of uninitialized value 'p1' here
|
where "ld->slopetype" is a "slopetype_t" enum, and for every value of
that enum the switch has a case that initializes "p1".
gcc/analyzer/ChangeLog:
PR analyzer/105273
* region-model.cc (has_nondefault_case_for_value_p): New.
(has_nondefault_cases_for_all_enum_values_p): New.
(region_model::apply_constraints_for_gswitch): Skip
implicitly-created "default" when switching on an enum
and all enum values have non-default cases.
(rejected_default_case::dump_to_pp): New.
* region-model.h (region_model_context::possibly_tainted_p): New
decl.
(class rejected_default_case): New.
* sm-taint.cc (region_model_context::possibly_tainted_p): New.
* supergraph.cc (switch_cfg_superedge::dump_label_to_pp): Dump
when implicitly_created_default_p.
(switch_cfg_superedge::implicitly_created_default_p): New.
* supergraph.h
(switch_cfg_superedge::implicitly_created_default_p): New decl.
gcc/testsuite/ChangeLog:
PR analyzer/105273
* gcc.dg/analyzer/switch-enum-1.c: New test.
* gcc.dg/analyzer/switch-enum-2.c: New test.
* gcc.dg/analyzer/switch-enum-pr105273-git-vreportf-2.c: New test.
* gcc.dg/analyzer/switch-enum-taint-1.c: New test.
* gcc.dg/analyzer/switch-wrong-enum.c: New test.
* gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c: New
test.
* gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c:
New test.
* gcc.dg/analyzer/torture/switch-enum-pr105273-git-vreportf-1.c:
New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/108252 reports a false positive from -Wanalyzer-malloc-leak on
code like this:
*ptr_ptr = strdup(EXPR);
where ptr_ptr is an UNKNOWN_VALUE.
When we handle:
*UNKNOWN = PTR;
store::set_value normally marks *PTR as having escaped, and this means
we don't report PTR as leaking when the last usage of PTR is lost.
However this only works for cases where PTR is a region_svalue.
In the example in the bug, it's a conjured_svalue, rather than a
region_svalue. A similar problem can arise for FDs, which aren't
pointers.
This patch fixes the bug by updating store::set_value to mark any
values stored via *UNKNOWN = VAL as not leaking.
Additionally, sm-malloc.cc's known_allocator_p hardcodes strdup and
strndup as allocators (and thus transitioning their result to
"unchecked"), but we don't implement known_functions for these, leading
to the LHS being a CONJURED_SVALUE, rather than a region_svalue to a
heap-allocated region. A similar issue happens with functions marked
with __attribute__((malloc)). As part of a "belt and braces" fix, the
patch also updates the handling of these functions, so that they use
heap-allocated regions.
gcc/analyzer/ChangeLog:
PR analyzer/108252
* kf.cc (class kf_strdup): New.
(class kf_strndup): New.
(register_known_functions): Register them.
* region-model.cc (region_model::on_call_pre): Use
&HEAP_ALLOCATED_REGION for the default result of an external
function with the "malloc" attribute, rather than CONJURED_SVALUE.
(region_model::get_or_create_region_for_heap_alloc): Allow
"size_in_bytes" to be NULL.
* store.cc (store::set_value): When handling *UNKNOWN = VAL,
mark VAL as "maybe bound".
gcc/testsuite/ChangeLog:
PR analyzer/108252
* gcc.dg/analyzer/attr-malloc-pr108252.c: New test.
* gcc.dg/analyzer/fd-leak-pr108252.c: New test.
* gcc.dg/analyzer/flex-with-call-summaries.c: Remove xfail from
warning false +ve directives.
* gcc.dg/analyzer/pr103217-2.c: Add -Wno-analyzer-too-complex.
* gcc.dg/analyzer/pr103217-3.c: Likewise.
* gcc.dg/analyzer/strdup-pr108252.c: New test.
* gcc.dg/analyzer/strndup-pr108252.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
2022 -> 2023
|
|
|
|
PR analyzer/106479 notes that we don't always show the region-creation
event for a memmove from an uninitialized stack region. This occurs
when using kf_memcpy_memmove. Fix by passing a src_region hint to
region_model::check_for_poison.
gcc/analyzer/ChangeLog:
PR analyzer/106479
* kf.cc (kf_memcpy_memmove::impl_call_pre): Pass in source region
to region_model::check_for_poison.
* region-model-asm.cc (region_model::on_asm_stmt): Pass NULL
region to region_model::check_for_poison.
* region-model.cc (region_model::check_for_poison): Add
"src_region" param, and pass it to poisoned_value_diagnostic.
(region_model::on_assignment): Pass NULL region to
region_model::check_for_poison.
(region_model::get_rvalue): Likewise.
* region-model.h (region_model::check_for_poison): Add
"src_region" param.
* sm-fd.cc (fd_state_machine::on_accept): Pass in source region
to region_model::check_for_poison.
* varargs.cc (kf_va_copy::impl_call_pre): Pass NULL region to
region_model::check_for_poison.
(kf_va_arg::impl_call_pre): Pass in source region to
region_model::check_for_poison.
gcc/testsuite/ChangeLog:
PR analyzer/106479
* gcc.dg/analyzer/pr104308.c (test_memmove_within_uninit): Remove
xfail on region creation event.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/108065
* region.cc (decl_region::get_svalue_for_initializer): Bail out to
avoid calling binding_key::make with an empty region.
* store.cc (binding_map::apply_ctor_val_to_range): Likewise.
(binding_map::apply_ctor_pair_to_child_region): Likewise.
(binding_cluster::bind): Likewise.
(binding_cluster::purge_region): Likewise.
(binding_cluster::maybe_get_compound_binding): Likewise.
(binding_cluster::maybe_get_simple_value): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/108065
* gfortran.dg/analyzer/pr108065.f90: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/ChangeLog:
* Makefile.in (ANALYZER_OBJS): Update for renaming of
analyzer/region-model-impl-calls.cc to analyzer/kf.cc.
gcc/analyzer/ChangeLog:
* analyzer.h (class known_function): Expand comment.
* region-model-impl-calls.cc: Rename to...
* kf.cc: ...this.
* known-function-manager.h (class known_function_manager): Add
leading comment.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
[PR108003]
gcc/analyzer/ChangeLog:
PR analyzer/108003
* call-summary.cc
(call_summary_replay::convert_region_from_summary_1): Convert
heap_regs_in_use from auto_sbitmap to auto_bitmap.
* region-model-manager.cc
(region_model_manager::get_or_create_region_for_heap_alloc):
Convert from sbitmap to bitmap.
* region-model-manager.h: Likewise.
* region-model.cc
(region_model::get_or_create_region_for_heap_alloc): Convert from
auto_sbitmap to auto_bitmap.
(region_model::get_referenced_base_regions): Likewise.
* region-model.h: Include "bitmap.h" rather than "sbitmap.h".
(region_model::get_referenced_base_regions): Convert from
auto_sbitmap to auto_bitmap.
gcc/testsuite/ChangeLog:
PR analyzer/108003
* g++.dg/analyzer/pr108003.C: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
* region-model-impl-calls.cc (class kf_memcpy): Rename to...
(class kf_memcpy_memmove): ...this.
(kf_memcpy::impl_call_pre): Rename to...
(kf_memcpy_memmove::impl_call_pre): ...this, and check the src for
poison.
(register_known_functions): Update for above renaming, and
register BUILT_IN_MEMMOVE and BUILT_IN_MEMMOVE_CHK.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/memcpy-1.c (test_8a, test_8b): New tests.
* gcc.dg/analyzer/memmove-1.c: New test, based on memcpy-1.c
* gcc.dg/analyzer/out-of-bounds-1.c (test7): Update expected
result for uninit srcBuf.
* gcc.dg/analyzer/out-of-bounds-5.c (test8, test9): Add
dg-warnings for memcpy from uninit src vla.
* gcc.dg/analyzer/pr104308.c (test_memmove_within_uninit):
Expect creation point note to be missing on riscv*-*-*.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/107882 reports an ICE, due to trying to get a compound svalue
for this binding:
cluster for: a:
key: {bytes 0-3}
value: {UNKNOWN()}
key: {empty}
value: {UNKNOWN()}
key: {bytes 4-7}
value: {UNKNOWN()}
where there's an binding to the unknown value of zero bits in size
"somewhere" within "a" (perhaps between bits 3 and 4?)
This makes no sense, so this patch adds an assertion that we never
attempt to create a binding key for an empty region, and adds early
rejection of attempts to get or set the values of such regions, fixing
the ICE.
gcc/analyzer/ChangeLog:
PR analyzer/107882
* region-model.cc (region_model::get_store_value): Return an
unknown value for empty regions.
(region_model::set_value): Bail on empty regions.
* region.cc (region::empty_p): New.
* region.h (region::empty_p): New decl.
* state-purge.cc (same_binding_p): Bail if either region is empty.
* store.cc (binding_key::make): Assert that a concrete binding's
bit_size must be > 0.
(binding_cluster::mark_region_as_unknown): Bail on empty regions.
(binding_cluster::get_binding): Likewise.
(binding_cluster::remove_overlapping_bindings): Likewise.
(binding_cluster::on_unknown_fncall): Don't conjure values for
empty regions.
(store::fill_region): Bail on empty regions.
* store.h (class concrete_binding): Update comment to reflect that
the range of bits must be non-empty.
(concrete_binding::concrete_binding): Assert that bit range is
non-empty.
gcc/testsuite/ChangeLog:
PR analyzer/107882
* gcc.dg/analyzer/memcpy-pr107882.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
PR analyzer/106325 reports false postives from
-Wanalyzer-null-dereference on code like this:
__attribute__((nonnull))
void foo_a (Foo *p)
{
foo_b (p);
switch (p->type)
{
/* ... */
}
}
where foo_b (p) has a:
g_return_if_fail (p);
that expands to:
if (!p)
{
return;
}
The analyzer "sees" the comparison against NULL in foo_b, and splits the
analysis into the NULL and not-NULL cases; later, back in foo_a, at
switch (p->type)
it complains that p is NULL.
Previously we were only using __attribute__((nonnull)) as something to
complain about when it was violated; we weren't using it as a source of
knowledge.
This patch fixes things by making the analyzer respect
__attribute__((nonnull)) at the top-level of the analysis: any such
params are now assumed to be non-NULL, so that the analyzer assumes the
g_return_if_fail inside foo_b doesn't fail when called from foo_a
Doing so fixes the false positives.
gcc/analyzer/ChangeLog:
PR analyzer/106325
* region-model-manager.cc
(region_model_manager::get_or_create_null_ptr): New.
* region-model-manager.h
(region_model_manager::get_or_create_null_ptr): New decl.
* region-model.cc (region_model::on_top_level_param): Add
"nonnull" param and make use of it.
(region_model::push_frame): When handling a top-level entrypoint
to the analysis, determine which params __attribute__((nonnull))
applies to, and pass to on_top_level_param.
* region-model.h (region_model::on_top_level_param): Add "nonnull"
param.
gcc/testsuite/ChangeLog:
PR analyzer/106325
* gcc.dg/analyzer/attr-nonnull-pr106325.c: New test.
* gcc.dg/analyzer/attribute-nonnull.c (test_6): New.
(test_7): New.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/ChangeLog:
* Makefile.in (ANALYZER_OBJS): Add analyzer/call-details.o,
analyzer/kf-analyzer.o, and kf-lang-cp.o.
gcc/analyzer/ChangeLog:
* analyzer.h (register_known_analyzer_functions): New decl.
(register_known_functions_lang_cp): New decl.
* call-details.cc: New file, split out from
region-model-impl-calls.cc.
* call-details.h: New file, split out from region-model.h.
* call-info.cc: Include "analyzer/call-details.h".
* call-summary.h: Likewise.
* kf-analyzer.cc: New file, split out from
region-model-impl-calls.cc.
* kf-lang-cp.cc: Likewise.
* known-function-manager.cc: Include "analyzer/call-details.h".
* region-model-impl-calls.cc: Move definitions of call_details's
member functions to call-details.cc. Move class kf_analyzer_* to
kf-analyzer.cc. Move kf_operator_new and kf_operator_delete to
kf-lang-cp.cc. Refresh #includes accordingly.
(register_known_functions): Replace registration of __analyzer_*
functions with a call to register_known_analyzer_functions.
Replace registration of C++ support functions with a call to
register_known_functions_lang_cp.
* region-model.h (class call_details): Move to new call-details.h.
* sm-fd.cc: Include "analyzer/call-details.h".
* sm-file.cc: Likewise.
* sm-malloc.cc: Likewise.
* varargs.cc: Likewise.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/analyzer_kernel_plugin.c: Include
"analyzer/call-details.h".
* gcc.dg/plugin/analyzer_known_fns_plugin.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
* analyzer.h (struct event_loc_info): New forward decl.
* bounds-checking.cc: Use event_loc_info throughout to bundle the
loc, fndecl, depth triples.
* call-info.cc: Likewise.
* checker-event.cc: Likewise.
* checker-event.h (struct event_loc_info): New decl. Use it
throughout to bundle the loc, fndecl, depth triples.
* checker-path.cc: Likewise.
* checker-path.h: Likewise.
* diagnostic-manager.cc: Likewise.
* engine.cc: Likewise.
* infinite-recursion.cc: Likewise.
* pending-diagnostic.cc: Likewise.
* pending-diagnostic.h: Likewise.
* region-model.cc: Likewise.
* sm-signal.cc: Likewise.
* varargs.cc: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
In r13-2573-gc81b60b8c6ff3d I split up the analyzer's region-creation
events to describe the memory space and capacity of the region as two
separate events to avoid combinatorial explosion of message wordings.
However I didn't take into account r13-1405-ge6c3bb379f515b which
added a pending_diagnostic::describe_region_creation_event vfunc which
could change the wording of region creation events.
Hence for:
#include <stdlib.h>
#include <stdint.h>
void test ()
{
int32_t *ptr = malloc (1);
free (ptr);
}
trunk currently emits:
Compiler Explorer (x86_64 trunk): https://godbolt.org/z/e3Td7c9s5:
<source>: In function 'test':
<source>:6:18: warning: allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
6 | int32_t *ptr = malloc (1);
| ^~~~~~~~~~
'test': events 1-3
|
| 6 | int32_t *ptr = malloc (1);
| | ^~~~~~~~~~
| | |
| | (1) allocated 1 bytes here
| | (2) allocated 1 bytes here
| | (3) assigned to 'int32_t *' {aka 'int *'} here; 'sizeof (int32_t {aka int})' is '4'
|
where events (1) and (2) are different region_creation_events that have
had their wording overridden (also, with a "1 bytes" issue).
This patch reorganizes region creation events so that each
pending_diagnostic instead creates the events that is appropriate for it,
and the events have responsibility for their own wording.
With this patch, the above emits:
<source>: In function 'test':
<source>:6:18: warning: allocated buffer size is not a multiple of the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
6 | int32_t *ptr = malloc (1);
| ^~~~~~~~~~
'test': events 1-2
|
| 6 | int32_t *ptr = malloc (1);
| | ^~~~~~~~~~
| | |
| | (1) allocated 1 byte here
| | (2) assigned to 'int32_t *' {aka 'int *'} here; 'sizeof (int32_t {aka int})' is '4'
|
fixing the duplicate event, and fixing the singular/plural issue.
gcc/analyzer/ChangeLog:
PR analyzer/107851
* analyzer.cc (make_label_text_n): Convert param "n" from int to
unsigned HOST_WIDE_INT.
* analyzer.h (make_label_text_n): Likewise for decl.
* bounds-checking.cc: Include "analyzer/checker-event.h" and
"analyzer/checker-path.h".
(out_of_bounds::add_region_creation_events): New.
(concrete_past_the_end::describe_region_creation_event): Replace
with...
(concrete_past_the_end::add_region_creation_events): ...this.
(symbolic_past_the_end::describe_region_creation_event): Delete.
* checker-event.cc (region_creation_event::region_creation_event):
Update for dropping all member data.
(region_creation_event::get_desc): Delete, splitting out into
region_creation_event_memory_space::get_desc,
region_creation_event_capacity::get_desc, and
region_creation_event_debug::get_desc.
(region_creation_event_memory_space::get_desc): New.
(region_creation_event_capacity::get_desc): New.
(region_creation_event_allocation_size::get_desc): New.
(region_creation_event_debug::get_desc): New.
* checker-event.h: Include "analyzer/program-state.h".
(enum rce_kind): Delete.
(class region_creation_event): Drop all member data.
(region_creation_event::region_creation_event): Make protected.
(region_creation_event::get_desc): Delete.
(class region_creation_event_memory_space): New.
(class region_creation_event_capacity): New.
(class region_creation_event_allocation_size): New.
(class region_creation_event_debug): New.
* checker-path.cc (checker_path::add_region_creation_events): Add
"pd" param. Call pending_diangnostic::add_region_creation_events.
Update for conversion of RCE_DEBUG to region_creation_event_debug.
* checker-path.h (checker_path::add_region_creation_events): Add
"pd" param.
* diagnostic-manager.cc (diagnostic_manager::build_emission_path):
Pass pending_diagnostic to
emission_path::add_region_creation_events.
(diagnostic_manager::build_emission_path): Pass path_builder to
add_event_on_final_node.
(diagnostic_manager::add_event_on_final_node): Add "pb" param.
Pass pending_diagnostic to
emission_path::add_region_creation_events.
(diagnostic_manager::add_events_for_eedge): Pass
pending_diagnostic to emission_path::add_region_creation_events.
* diagnostic-manager.h
(diagnostic_manager::add_event_on_final_node): Add "pb" param.
* pending-diagnostic.cc
(pending_diagnostic::add_region_creation_events): New.
* pending-diagnostic.h (struct region_creation): Delete.
(pending_diagnostic::describe_region_creation_event): Delete.
(pending_diagnostic::add_region_creation_events): New vfunc.
* region-model.cc: Include "analyzer/checker-event.h" and
"analyzer/checker-path.h".
(dubious_allocation_size::dubious_allocation_size): Initialize
m_has_allocation_event.
(dubious_allocation_size::describe_region_creation_event): Delete.
(dubious_allocation_size::describe_final_event): Update for
replacement of m_allocation_event with m_has_allocation_event.
(dubious_allocation_size::add_region_creation_events): New.
(dubious_allocation_size::m_allocation_event): Replace with...
(dubious_allocation_size::m_has_allocation_event): ...this.
gcc/testsuite/ChangeLog:
PR analyzer/107851
* gcc.dg/analyzer/allocation-size-4.c: Update expected wording.
* gcc.dg/analyzer/allocation-size-multiline-1.c: New test.
* gcc.dg/analyzer/allocation-size-multiline-2.c: New test.
* gcc.dg/analyzer/out-of-bounds-multiline-1.c: Update expected
wording.
* gcc.dg/analyzer/out-of-bounds-multiline-2.c: New test.
* gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Update expected
wording.
* gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/107948
* region-model-manager.cc
(region_model_manager::maybe_fold_binop): Fold (0 - VAL) to -VAL.
* region-model.cc (region_model::eval_condition): Handle e.g.
"-X <= 0" as equivalent to X >= 0".
gcc/testsuite/ChangeLog:
PR analyzer/107948
* gcc.dg/analyzer/feasibility-pr107948.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
PR analyzer/106626
* bounds-checking.cc
(symbolic_past_the_end::describe_final_event): Delete, moving to
symbolic_buffer_overflow::describe_final_event and
symbolic_buffer_over_read::describe_final_event, eliminating
composition of text strings via "byte_str" and "m_dir_str".
(symbolic_past_the_end::m_dir_str): Delete field.
(symbolic_buffer_overflow::symbolic_buffer_overflow): Drop
m_dir_str.
(symbolic_buffer_overflow::describe_final_event): New, as noted
above.
(symbolic_buffer_over_read::symbolic_buffer_overflow): Drop
m_dir_str.
(symbolic_buffer_over_read::describe_final_event): New, as noted
above.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Convert out-of-bounds class hierarchy from:
pending_diagnostic
out_of_bounds
past_the_end
buffer_overflow (*)
buffer_over_read (*)
buffer_underwrite (*)
buffer_under_read (*)
symbolic_past_the_end
symbolic_buffer_overflow (*)
symbolic_buffer_over_read (*)
to:
pending_diagnostic
out_of_bounds
concrete_out_of_bounds
concrete_past_the_end
concrete_buffer_overflow (*)
concrete_buffer_over_read (*)
concrete_buffer_underwrite (*)
concrete_buffer_under_read (*)
symbolic_past_the_end
symbolic_buffer_overflow (*)
symbolic_buffer_over_read (*)
where the concrete classes (i.e. the instantiable ones) are marked
with a (*).
Doing so undercovered a bug where, for CWE-131-examples.c, we were
emitting an extra:
warning: heap-based buffer over-read [CWE-122] [-Wanalyzer-out-of-bounds]
at the:
WidgetList[numWidgets] = NULL;
The issue was that within set_next_state we get the rvalue for the LHS,
which looks like a read to the bounds-checker. The patch fixes this by
passing NULL as the region_model_context * for such accesses.
gcc/analyzer/ChangeLog:
* bounds-checking.cc (class out_of_bounds): Split out from...
(class concrete_out_of_bounds): New abstract subclass.
(class past_the_end): Rename to...
(class concrete_past_the_end): ...this, and make a subclass of
concrete_out_of_bounds.
(class buffer_overflow): Rename to...
(class concrete_buffer_overflow): ...this, and make a subclass of
concrete_past_the_end.
(class buffer_over_read): Rename to...
(class concrete_buffer_over_read): ...this, and make a subclass of
concrete_past_the_end.
(class buffer_underwrite): Rename to...
(class concrete_buffer_underwrite): ...this, and make a subclass
of concrete_out_of_bounds.
(class buffer_under_read): Rename to...
(class concrete_buffer_under_read): ...this, and make a subclass
of concrete_out_of_bounds.
(class symbolic_past_the_end): Convert to a subclass of
out_of_bounds.
(symbolic_buffer_overflow::get_kind): New.
(symbolic_buffer_over_read::get_kind): New.
(region_model::check_region_bounds): Update for renamings.
* engine.cc (impl_sm_context::set_next_state): Eliminate
"new_ctxt", passing NULL to get_rvalue instead.
(impl_sm_context::warn): Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
This patch tweaks the wording of -Wanalyzer-out-of-bounds:
* use the spellings/terminology of CWE:
* replace "underread" with "under-read", as per:
https://cwe.mitre.org/data/definitions/127.html
* replace "overread" with "over-read" as per:
https://cwe.mitre.org/data/definitions/126.html
* replace "underflow" with "underwrite" as per:
https://cwe.mitre.org/data/definitions/124.html
* wherever known, specify the memory region of the bad access,
so that it says e.g. "heap-based buffer over-read"
or "stack-based buffer over-read"
gcc/analyzer/ChangeLog:
PR analyzer/106626
* bounds-checking.cc (out_of_bounds::get_memory_space): New.
(buffer_overflow::emit): Use it.
(class buffer_overread): Rename to...
(class buffer_over_read): ...this.
(buffer_over_read::emit): Specify which memory space the read is
from, where known. Change "overread" to "over-read".
(class buffer_underflow): Rename to...
(class buffer_underwrite): ...this.
(buffer_underwrite::emit): Specify which memory space the write is
to, where known. Change "underflow" to "underwrite".
(class buffer_underread): Rename to...
(class buffer_under_read): Rename to...
(buffer_under_read::emit): Specify which memory space the read is
from, where known. Change "underread" to "under-read".
(symbolic_past_the_end::get_memory_space): New.
(symbolic_buffer_overflow::emit): Use it.
(class symbolic_buffer_overread): Rename to...
(class symbolic_buffer_over_read): ...this.
(symbolic_buffer_over_read::emit): Specify which memory space the
read is from, where known. Change "overread" to "over-read".
(region_model::check_symbolic_bounds): Update for class renaming.
(region_model::check_region_bounds): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/106626
* gcc.dg/analyzer/call-summaries-2.c: Update expected results.
* gcc.dg/analyzer/out-of-bounds-1.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-2.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-3.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-4.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-5.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-container_of.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Likewise. Rename
functions from "int_arr_" to "char_arr_".
* gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Update expected
results.
* gcc.dg/analyzer/out-of-bounds-read-struct-arr.c: New test.
* gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Update expected
results. Rename functions from "int_arr_" to "char_arr_".
* gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Update expected
results.
* gcc.dg/analyzer/out-of-bounds-write-struct-arr.c: New test.
* gcc.dg/analyzer/pr101962.c: Update expected results.
* gcc.dg/analyzer/realloc-5.c: Update expected results.
* gcc.dg/analyzer/zlib-3.c: Update expected results.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Consider -fanalyzer on:
#include <stdint.h>
int32_t arr[10];
void int_arr_write_element_after_end_off_by_one(int32_t x)
{
arr[10] = x;
}
Trunk x86_64: https://godbolt.org/z/17zn3qYY4
Currently we emit:
<source>: In function 'int_arr_write_element_after_end_off_by_one':
<source>:7:11: warning: buffer overflow [CWE-787] [-Wanalyzer-out-of-bounds]
7 | arr[10] = x;
| ~~~~~~~~^~~
event 1
|
| 3 | int32_t arr[10];
| | ^~~
| | |
| | (1) capacity is 40 bytes
|
+--> 'int_arr_write_element_after_end_off_by_one': events 2-3
|
| 5 | void int_arr_write_element_after_end_off_by_one(int32_t x)
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) entry to 'int_arr_write_element_after_end_off_by_one'
| 6 | {
| 7 | arr[10] = x;
| | ~~~~~~~~~~~
| | |
| | (3) out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40
|
<source>:7:11: note: write of 4 bytes to beyond the end of 'arr'
7 | arr[10] = x;
| ~~~~~~~~^~~
This is worded in terms of bytes, due to the way -Wanalyzer-out-of-bounds
is implemented, but this isn't what the user wrote.
This patch tries to get closer to the user's code by adding a note about
array bounds when we're referring to an array. In the above example it
adds this trailing note:
note: valid subscripts for 'arr' are '[0]' to '[9]'
gcc/analyzer/ChangeLog:
PR analyzer/106626
* bounds-checking.cc (out_of_bounds::maybe_describe_array_bounds):
New.
(buffer_overflow::emit): Call maybe_describe_array_bounds.
(buffer_overread::emit): Likewise.
(buffer_underflow::emit): Likewise.
(buffer_underread::emit): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/106626
* gcc.dg/analyzer/call-summaries-2.c: Add dg-message for expected
note about valid indexes.
* gcc.dg/analyzer/out-of-bounds-1.c: Likewise, fixing up existing
dg-message directives.
* gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Consider -fanalyzer on:
#include <stdint.h>
int32_t arr[10];
void int_arr_write_element_after_end_far(int32_t x)
{
arr[100] = x;
}
Trunk x86_64: https://godbolt.org/z/7GqEcYGq6
Currently we emit:
<source>: In function 'int_arr_write_element_after_end_far':
<source>:7:12: warning: buffer overflow [CWE-787] [-Wanalyzer-out-of-bounds]
7 | arr[100] = x;
| ~~~~~~~~~^~~
event 1
|
| 3 | int32_t arr[10];
| | ^~~
| | |
| | (1) capacity is 40 bytes
|
+--> 'int_arr_write_element_after_end_far': events 2-3
|
| 5 | void int_arr_write_element_after_end_far(int32_t x)
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) entry to 'int_arr_write_element_after_end_far'
| 6 | {
| 7 | arr[100] = x;
| | ~~~~~~~~~~~~
| | |
| | (3) out-of-bounds write from byte 400 till byte 403 but 'arr' ends at byte 40
|
<source>:7:12: note: write is 4 bytes past the end of 'arr'
7 | arr[100] = x;
| ~~~~~~~~~^~~
The wording of the final note:
"write is 4 bytes past the end of 'arr'"
reads to me as if the "4 bytes past" is describing where the access
occurs, which seems wrong, as the write is far beyond the end of the
array. Looking at the implementation, it's actually describing the
number of bytes within the access that are beyond the bounds of the
buffer.
This patch updates the wording so that the final note reads
"write of 4 bytes to beyond the end of 'arr'"
which more clearly expresses that it's the size of the access
being described.
The patch also uses inform_n to avoid emitting "1 bytes".
gcc/analyzer/ChangeLog:
PR analyzer/106626
* bounds-checking.cc (buffer_overflow::emit): Use inform_n.
Update wording to clarify that we're talking about the size of
the bad access, rather than its position.
(buffer_overread::emit): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/106626
* gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Update for
changes to expected wording.
* gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise.
* gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/ChangeLog:
* Makefile.in (ANALYZER_OBJS): Add analyzer/bounds-checking.o.
gcc/analyzer/ChangeLog:
* bounds-checking.cc: New file, taken from region-model.cc.
* region-model.cc (class out_of_bounds): Move to
bounds-checking.cc.
(class past_the_end): Likewise.
(class buffer_overflow): Likewise.
(class buffer_overread): Likewise.
(class buffer_underflow): Likewise.
(class buffer_underread): Likewise.
(class symbolic_past_the_end): Likewise.
(class symbolic_buffer_overflow): Likewise.
(class symbolic_buffer_overread): Likewise.
(region_model::check_symbolic_bounds): Likewise.
(maybe_get_integer_cst_tree): Likewise.
(region_model::check_region_bounds): Likewise.
* region-model.h: Add comment.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
PR analyzer/107928
* sm-fd.cc (fd_state_machine::on_bind): Handle m_constant_fd in
the "success" outcome.
(fd_state_machine::on_connect): Likewise.
* sm-fd.dot: Add "constant_fd" state and its transitions.
gcc/testsuite/ChangeLog:
PR analyzer/107928
* gcc.dg/analyzer/fd-bind-pr107928.c: New test.
* gcc.dg/analyzer/fd-connect-pr107928.c: New test.
* gcc.dg/analyzer/fd-stream-socket-active-open.c
(test_active_open_from_connect_constant): New, adapted from
test_active_open_from_connect.
* gcc.dg/analyzer/fd-stream-socket-passive-open.c
(test_passive_open_from_bind_constant): New, adapted from
test_passive_open_from_bind.
(test_passive_open_from_listen_constant): New, adapted from
test_passive_open_from_listen.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
* region-model-impl-calls.cc (class kf_fgets): Move to sm-file.cc.
(kf_fgets::impl_call_pre): Likewise.
(class kf_fread): Likewise.
(kf_fread::impl_call_pre): Likewise.
(class kf_getchar): Likewise.
(class kf_stdio_output_fn): Likewise.
(register_known_functions): Move registration of
BUILT_IN_FPRINTF, BUILT_IN_FPRINTF_UNLOCKED, BUILT_IN_FPUTC,
BUILT_IN_FPUTC_UNLOCKED, BUILT_IN_FPUTS, BUILT_IN_FPUTS_UNLOCKED,
BUILT_IN_FWRITE, BUILT_IN_FWRITE_UNLOCKED, BUILT_IN_PRINTF,
BUILT_IN_PRINTF_UNLOCKED, BUILT_IN_PUTC, BUILT_IN_PUTCHAR,
BUILT_IN_PUTCHAR_UNLOCKED, BUILT_IN_PUTC_UNLOCKED, BUILT_IN_PUTS,
BUILT_IN_PUTS_UNLOCKED, BUILT_IN_VFPRINTF, BUILT_IN_VPRINTF,
"getchar", "fgets", "fgets_unlocked", and "fread" to
register_known_file_functions.
* sm-file.cc (class kf_stdio_output_fn): Move here from
region-model-impl-calls.cc.
(class kf_fgets): Likewise.
(class kf_fread): Likewise.
(class kf_getchar): Likewise.
(register_known_file_functions): Move registration of
BUILT_IN_FPRINTF, BUILT_IN_FPRINTF_UNLOCKED, BUILT_IN_FPUTC,
BUILT_IN_FPUTC_UNLOCKED, BUILT_IN_FPUTS, BUILT_IN_FPUTS_UNLOCKED,
BUILT_IN_FWRITE, BUILT_IN_FWRITE_UNLOCKED, BUILT_IN_PRINTF,
BUILT_IN_PRINTF_UNLOCKED, BUILT_IN_PUTC, BUILT_IN_PUTCHAR,
BUILT_IN_PUTCHAR_UNLOCKED, BUILT_IN_PUTC_UNLOCKED, BUILT_IN_PUTS,
BUILT_IN_PUTS_UNLOCKED, BUILT_IN_VFPRINTF, BUILT_IN_VPRINTF,
"fgets", "fgets_unlocked", "fread", and "getchar" to here from
register_known_functions.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
PR analyzer/103546 tracks various false positives seen on
flex-generated lexers.
Whilst investigating them, I noticed an ICE with
-fanalyzer-call-summaries due to attempting to store sm-state
for an UNKNOWN svalue, which this patch fixes.
This patch also provides known_function implementations of all of the
external functions called by the lexer, reducing the number of false
positives.
The patch doesn't eliminate all false positives, but adds integration
tests to try to establish a baseline from which the remaining false
positives can be fixed.
gcc/analyzer/ChangeLog:
PR analyzer/103546
* analyzer.h (register_known_file_functions): New decl.
* program-state.cc (sm_state_map::replay_call_summary): Rejct
attempts to store sm-state for caller_sval that can't have
associated state.
* region-model-impl-calls.cc (register_known_functions): Call
register_known_file_functions.
* sm-fd.cc (class kf_isatty): New.
(register_known_fd_functions): Register it.
* sm-file.cc (class kf_ferror): New.
(class kf_fileno): New.
(class kf_getc): New.
(register_known_file_functions): New.
gcc/ChangeLog:
PR analyzer/103546
* doc/invoke.texi (Static Analyzer Options): Add isatty, ferror,
fileno, and getc to the list of functions known to the analyzer.
gcc/testsuite/ChangeLog:
PR analyzer/103546
* gcc.dg/analyzer/ferror-1.c: New test.
* gcc.dg/analyzer/fileno-1.c: New test.
* gcc.dg/analyzer/flex-with-call-summaries.c: New test.
* gcc.dg/analyzer/flex-without-call-summaries.c: New test.
* gcc.dg/analyzer/getc-1.c: New test.
* gcc.dg/analyzer/isatty-1.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
PR analyzer/105784
* region-model-manager.cc
(region_model_manager::maybe_fold_binop): For POINTER_PLUS_EXPR,
PLUS_EXPR and MINUS_EXPR, eliminate requirement that the final
type matches that of arg0 in favor of a cast.
gcc/testsuite/ChangeLog:
PR analyzer/105784
* gcc.dg/analyzer/torture/fold-ptr-arith-pr105784.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
Fixes the following warnings:
gcc/analyzer/varargs.cc:655:8: warning: 'matches_call_types_p' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
gcc/analyzer/varargs.cc:707:50: warning: unused parameter 'cd' [-Wunused-parameter]
gcc/analyzer/varargs.cc:707:8: warning: 'matches_call_types_p' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
gcc/analyzer/ChangeLog:
* varargs.cc: Fix Clang warnings.
|
|
PR analyzer/106473 reports a false positive from -Wanalyzer-malloc-leak
on:
void foo(char **args[], int *argc) {
*argc = 1;
(*args)[0] = __builtin_malloc(42);
}
The issue is that at the write to *argc we don't know if argc could
point within *args, and so we conservatiely set *args to be unknown.
At the write "(*args)[0] = __builtin_malloc(42)" we have the result of
the allocation written through an unknown pointer, so we mark the
heap_allocated_region as having escaped.
Unfortunately, within store::canonicalize we overzealously purge the
heap allocated region, losing the information that it has escaped, and
thus errnoeously report a leak.
The first part of the fix is to update store::canonicalize so that it
doesn't purge heap_allocated_regions that are marked as escaping.
Doing so fixes the leak false positive, but leads to various state
explosions relating to anywhere we have a malloc/free pair in a loop,
where the analysis of the iteration appears to only have been reaching
a fixed state due to a bug in the state merger code that was erroneously
merging state about the region allocated in one iteration with that
of another. On touching that, the analyzer fails to reach a fixed state
on any loops containing a malloc/free pair, since each analysis of a
malloc was creating a new heap_allocated_region instance.
Hence the second part of the fix is to revamp how heap_allocated_regions
are managed within the analyzer. Rather than create a new one at each
analysis of a malloc call, instead we reuse them across the analysis,
only creating a new one if the current path's state is referencing all
of the existing ones. Hence the heap_allocated_region instances get
used in a fixed order along every analysis path, so e.g. at:
if (flag)
p = malloc (4096);
else
p = malloc (1024);
both paths now use the same heap_allocated_region for their malloc
calls - but we still end up with two enodes after the CFG merger, by
rejecting merger of states with non-equal dynamic extents.
gcc/analyzer/ChangeLog:
PR analyzer/106473
* call-summary.cc
(call_summary_replay::convert_region_from_summary_1): Update for
change to creation of heap-allocated regions.
* program-state.cc (test_program_state_1): Likewise.
(test_program_state_merging): Likewise.
* region-model-impl-calls.cc (kf_calloc::impl_call_pre): Likewise.
(kf_malloc::impl_call_pre): Likewise.
(kf_operator_new::impl_call_pre): Likewise.
(kf_realloc::impl_call_postsuccess_with_move::update_model): Likewise.
* region-model-manager.cc
(region_model_manager::create_region_for_heap_alloc): Convert
to...
(region_model_manager::get_or_create_region_for_heap_alloc):
...this, reusing an existing region if it's unreferenced in the
client state.
* region-model-manager.h (region_model_manager::get_num_regions): New.
(region_model_manager::create_region_for_heap_alloc): Convert to...
(region_model_manager::get_or_create_region_for_heap_alloc): ...this.
* region-model.cc (region_to_value_map::can_merge_with_p): Reject
merger when the values are different.
(region_model::create_region_for_heap_alloc): Convert to...
(region_model::get_or_create_region_for_heap_alloc): ...this.
(region_model::get_referenced_base_regions): New.
(selftest::test_state_merging): Update for change to creation of
heap-allocated regions.
(selftest::test_malloc_constraints): Likewise.
(selftest::test_malloc): Likewise.
* region-model.h: Include "sbitmap.h".
(region_model::create_region_for_heap_alloc): Convert to...
(region_model::get_or_create_region_for_heap_alloc): ...this.
(region_model::get_referenced_base_regions): New decl.
* store.cc (store::canonicalize): Don't purge a heap-allocated region
that's been marked as escaping.
gcc/testsuite/ChangeLog:
PR analyzer/106473
* gcc.dg/analyzer/aliasing-pr106473.c: New test.
* gcc.dg/analyzer/allocation-size-2.c: Add
-fanalyzer-fine-grained".
* gcc.dg/analyzer/allocation-size-3.c: Likewise.
* gcc.dg/analyzer/explode-1.c: Mark leak with XFAIL.
* gcc.dg/analyzer/explode-3.c: New test.
* gcc.dg/analyzer/malloc-reuse.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
