| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
gcc/analyzer/ChangeLog:
* access-diagram.cc: Add #define INCLUDE_VECTOR.
* bounds-checking.cc: Likewise.
gcc/ChangeLog:
* diagnostic-format-sarif.cc: Add #define INCLUDE_VECTOR.
* diagnostic.cc: Likewise.
* text-art/box-drawing.cc: Likewise.
* text-art/canvas.cc: Likewise.
* text-art/ruler.cc: Likewise.
* text-art/selftests.cc: Likewise.
* text-art/selftests.h (text_art::canvas): New forward decl.
* text-art/style.cc: Add #define INCLUDE_VECTOR.
* text-art/styled-string.cc: Likewise.
* text-art/table.cc: Likewise.
* text-art/table.h: Remove #include <vector>.
* text-art/theme.cc: Add #define INCLUDE_VECTOR.
* text-art/types.h: Check that INCLUDE_VECTOR is defined.
Remove #include of <vector> and <string>.
* text-art/widget.cc: Add #define INCLUDE_VECTOR.
* text-art/widget.h: Remove #include <vector>.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic_plugin_test_text_art.c: Add
#define INCLUDE_VECTOR.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
This patch extends -Wanalyzer-out-of-bounds so that, where possible, it
will emit a text art diagram visualizing the spatial relationship between
(a) the memory region that the analyzer predicts would be accessed, versus
(b) the range of memory that is valid to access - whether they overlap,
are touching, are close or far apart; which one is before or after in
memory, the relative sizes involved, the direction of the access (read vs
write), and, in some cases, the values of data involved. This diagram
can be suppressed using -fdiagnostics-text-art-charset=none.
For example, given:
int32_t arr[10];
int32_t int_arr_read_element_before_start_far(void)
{
return arr[-100];
}
it emits:
demo-1.c: In function ‘int_arr_read_element_before_start_far’:
demo-1.c:7:13: warning: buffer under-read [CWE-127] [-Wanalyzer-out-of-bounds]
7 | return arr[-100];
| ~~~^~~~~~
‘int_arr_read_element_before_start_far’: event 1
|
| 7 | return arr[-100];
| | ~~~^~~~~~
| | |
| | (1) out-of-bounds read from byte -400 till byte -397 but ‘arr’ starts at byte 0
|
demo-1.c:7:13: note: valid subscripts for ‘arr’ are ‘[0]’ to ‘[9]’
┌───────────────────────────┐
│read of ‘int32_t’ (4 bytes)│
└───────────────────────────┘
^
│
│
┌───────────────────────────┐ ┌────────┬────────┬─────────┐
│ │ │ [0] │ ... │ [9] │
│ before valid range │ ├────────┴────────┴─────────┤
│ │ │‘arr’ (type: ‘int32_t[10]’)│
└───────────────────────────┘ └───────────────────────────┘
├─────────────┬─────────────┤├─────┬──────┤├─────────────┬─────────────┤
│ │ │
╭────────────┴───────────╮ ╭────┴────╮ ╭───────┴──────╮
│⚠️ under-read of 4 bytes│ │396 bytes│ │size: 40 bytes│
╰────────────────────────╯ ╰─────────╯ ╰──────────────╯
and given:
#include <string.h>
void
test_non_ascii ()
{
char buf[5];
strcpy (buf, "文字化け");
}
it emits:
demo-2.c: In function ‘test_non_ascii’:
demo-2.c:7:3: warning: stack-based buffer overflow [CWE-121] [-Wanalyzer-out-of-bounds]
7 | strcpy (buf, "文字化け");
| ^~~~~~~~~~~~~~~~~~~~~~~~
‘test_non_ascii’: events 1-2
|
| 6 | char buf[5];
| | ^~~
| | |
| | (1) capacity: 5 bytes
| 7 | strcpy (buf, "文字化け");
| | ~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) out-of-bounds write from byte 5 till byte 12 but ‘buf’ ends at byte 5
|
demo-2.c:7:3: note: write of 8 bytes to beyond the end of ‘buf’
7 | strcpy (buf, "文字化け");
| ^~~~~~~~~~~~~~~~~~~~~~~~
demo-2.c:7:3: note: valid subscripts for ‘buf’ are ‘[0]’ to ‘[4]’
┌─────┬─────┬─────┬────┬────┐┌────┬────┬────┬────┬────┬────┬────┬──────┐
│ [0] │ [1] │ [2] │[3] │[4] ││[5] │[6] │[7] │[8] │[9] │[10]│[11]│ [12] │
├─────┼─────┼─────┼────┼────┤├────┼────┼────┼────┼────┼────┼────┼──────┤
│0xe6 │0x96 │0x87 │0xe5│0xad││0x97│0xe5│0x8c│0x96│0xe3│0x81│0x91│ 0x00 │
├─────┴─────┴─────┼────┴────┴┴────┼────┴────┴────┼────┴────┴────┼──────┤
│ U+6587 │ U+5b57 │ U+5316 │ U+3051 │U+0000│
├─────────────────┼───────────────┼──────────────┼──────────────┼──────┤
│ 文 │ 字 │ 化 │ け │ NUL │
├─────────────────┴───────────────┴──────────────┴──────────────┴──────┤
│ string literal (type: ‘char[13]’) │
└──────────────────────────────────────────────────────────────────────┘
│ │ │ │ │ │ │ │ │ │ │ │ │
│ │ │ │ │ │ │ │ │ │ │ │ │
v v v v v v v v v v v v v
┌─────┬────────────────┬────┐┌─────────────────────────────────────────┐
│ [0] │ ... │[4] ││ │
├─────┴────────────────┴────┤│ after valid range │
│ ‘buf’ (type: ‘char[5]’) ││ │
└───────────────────────────┘└─────────────────────────────────────────┘
├─────────────┬─────────────┤├────────────────────┬────────────────────┤
│ │
╭────────┴────────╮ ╭───────────┴──────────╮
│capacity: 5 bytes│ │⚠️ overflow of 8 bytes│
╰─────────────────╯ ╰──────────────────────╯
showing that the overflow occurs partway through the UTF-8 encoding of
the U+5b57 code point.
There are lots more examples in the test suite.
It doesn't show up in this email, but the above diagrams are colorized
to constrast the valid and invalid access ranges.
gcc/ChangeLog:
PR analyzer/106626
* Makefile.in (ANALYZER_OBJS): Add analyzer/access-diagram.o.
* doc/invoke.texi (Wanalyzer-out-of-bounds): Add description of
text art.
(fanalyzer-debug-text-art): New.
gcc/analyzer/ChangeLog:
PR analyzer/106626
* access-diagram.cc: New file.
* access-diagram.h: New file.
* analyzer.h (class region_offset): Add default ctor.
(region_offset::make_byte_offset): New decl.
(region_offset::concrete_p): New.
(region_offset::get_concrete_byte_offset): New.
(region_offset::calc_symbolic_bit_offset): New decl.
(region_offset::calc_symbolic_byte_offset): New decl.
(region_offset::dump_to_pp): New decl.
(region_offset::dump): New decl.
(operator<, operator<=, operator>, operator>=): New decls for
region_offset.
* analyzer.opt
(-param=analyzer-text-art-string-ellipsis-threshold=): New.
(-param=analyzer-text-art-string-ellipsis-head-len=): New.
(-param=analyzer-text-art-string-ellipsis-tail-len=): New.
(-param=analyzer-text-art-ideal-canvas-width=): New.
(fanalyzer-debug-text-art): New.
* bounds-checking.cc: Include "intl.h", "diagnostic-diagram.h",
and "analyzer/access-diagram.h".
(class out_of_bounds::oob_region_creation_event_capacity): New.
(out_of_bounds::out_of_bounds): Add "model" and "sval_hint"
params.
(out_of_bounds::mark_interesting_stuff): Use the base region.
(out_of_bounds::add_region_creation_events): Use
oob_region_creation_event_capacity.
(out_of_bounds::get_dir): New pure vfunc.
(out_of_bounds::maybe_show_notes): New.
(out_of_bounds::maybe_show_diagram): New.
(out_of_bounds::make_access_diagram): New.
(out_of_bounds::m_model): New field.
(out_of_bounds::m_sval_hint): New field.
(out_of_bounds::m_region_creation_event_id): New field.
(concrete_out_of_bounds::concrete_out_of_bounds): Update for new
fields.
(concrete_past_the_end::concrete_past_the_end): Likewise.
(concrete_past_the_end::add_region_creation_events): Use
oob_region_creation_event_capacity.
(concrete_buffer_overflow::concrete_buffer_overflow): Update for
new fields.
(concrete_buffer_overflow::emit): Replace call to
maybe_describe_array_bounds with maybe_show_notes.
(concrete_buffer_overflow::get_dir): New.
(concrete_buffer_over_read::concrete_buffer_over_read): Update for
new fields.
(concrete_buffer_over_read::emit): Replace call to
maybe_describe_array_bounds with maybe_show_notes.
(concrete_buffer_overflow::get_dir): New.
(concrete_buffer_underwrite::concrete_buffer_underwrite): Update
for new fields.
(concrete_buffer_underwrite::emit): Replace call to
maybe_describe_array_bounds with maybe_show_notes.
(concrete_buffer_underwrite::get_dir): New.
(concrete_buffer_under_read::concrete_buffer_under_read): Update
for new fields.
(concrete_buffer_under_read::emit): Replace call to
maybe_describe_array_bounds with maybe_show_notes.
(concrete_buffer_under_read::get_dir): New.
(symbolic_past_the_end::symbolic_past_the_end): Update for new
fields.
(symbolic_buffer_overflow::symbolic_buffer_overflow): Likewise.
(symbolic_buffer_overflow::emit): Call maybe_show_notes.
(symbolic_buffer_overflow::get_dir): New.
(symbolic_buffer_over_read::symbolic_buffer_over_read): Update for
new fields.
(symbolic_buffer_over_read::emit): Call maybe_show_notes.
(symbolic_buffer_over_read::get_dir): New.
(region_model::check_symbolic_bounds): Add "sval_hint" param. Pass
it and sized_offset_reg to diagnostics.
(region_model::check_region_bounds): Add "sval_hint" param, passing
it to diagnostics.
* diagnostic-manager.cc
(diagnostic_manager::emit_saved_diagnostic): Pass logger to
pending_diagnostic::emit.
* engine.cc: Add logger param to pending_diagnostic::emit
implementations.
* infinite-recursion.cc: Likewise.
* kf-analyzer.cc: Likewise.
* kf.cc: Likewise. Add nullptr for new param of
check_region_for_write.
* pending-diagnostic.h: Likewise in decl.
* region-model-manager.cc
(region_model_manager::get_or_create_int_cst): Convert param from
poly_int64 to const poly_wide_int_ref &.
(region_model_manager::maybe_fold_binop): Support type being NULL
when checking for floating-point types.
Check for (X + Y) - X => Y. Be less strict about types when folding
associative ops. Check for (X + Y) * CST => (X * CST) + (Y * CST).
* region-model-manager.h
(region_model_manager::get_or_create_int_cst): Convert param from
poly_int64 to const poly_wide_int_ref &.
* region-model.cc: Add logger param to pending_diagnostic::emit
implementations.
(region_model::check_external_function_for_access_attr): Update
for new param of check_region_for_write.
(region_model::deref_rvalue): Use nullptr rather than NULL.
(region_model::get_capacity): Handle RK_STRING.
(region_model::check_region_access): Add "sval_hint" param; pass it to
check_region_bounds.
(region_model::check_region_for_write): Add "sval_hint" param;
pass it to check_region_access.
(region_model::check_region_for_read): Add NULL for new param to
check_region_access.
(region_model::set_value): Pass rhs_sval to
check_region_for_write.
(region_model::get_representative_path_var_1): Handle SK_CONSTANT
in the check for infinite recursion.
* region-model.h (region_model::check_region_for_write): Add
"sval_hint" param.
(region_model::check_region_access): Likewise.
(region_model::check_symbolic_bounds): Likewise.
(region_model::check_region_bounds): Likewise.
* region.cc (region_offset::make_byte_offset): New.
(region_offset::calc_symbolic_bit_offset): New.
(region_offset::calc_symbolic_byte_offset): New.
(region_offset::dump_to_pp): New.
(region_offset::dump): New.
(struct linear_op): New.
(operator<, operator<=, operator>, operator>=): New, for
region_offset.
(region::get_next_offset): New.
(region::get_relative_symbolic_offset): Use ptrdiff_type_node.
(field_region::get_relative_symbolic_offset): Likewise.
(element_region::get_relative_symbolic_offset): Likewise.
(bit_range_region::get_relative_symbolic_offset): Likewise.
* region.h (region::get_next_offset): New decl.
* sm-fd.cc: Add logger param to pending_diagnostic::emit
implementations.
* sm-file.cc: Likewise.
* sm-malloc.cc: Likewise.
* sm-pattern-test.cc: Likewise.
* sm-sensitive.cc: Likewise.
* sm-signal.cc: Likewise.
* sm-taint.cc: Likewise.
* store.cc (bit_range::contains_p): Allow "out" to be null.
* store.h (byte_range::get_start_bit_offset): New.
(byte_range::get_next_bit_offset): New.
* varargs.cc: Add logger param to pending_diagnostic::emit
implementations.
gcc/testsuite/ChangeLog:
PR analyzer/106626
* gcc.dg/analyzer/data-model-1.c (test_16): Update for
out-of-bounds working.
* gcc.dg/analyzer/out-of-bounds-diagram-1-ascii.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-1-debug.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-1-emoji.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-1-json.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-1-sarif.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-1-unicode.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-10.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-11.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-12.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-13.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-14.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-15.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-2.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-3.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-4.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-5-ascii.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-5-unicode.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-6.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-7.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-8.c: New test.
* gcc.dg/analyzer/out-of-bounds-diagram-9.c: New test.
* gcc.dg/analyzer/pattern-test-2.c: Update expected results.
* gcc.dg/analyzer/pr101962.c: Update expected results.
* gcc.dg/plugin/analyzer_gil_plugin.c: Add logger param to
pending_diagnostic::emit implementations.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
Currently, the analyzer tries to prove that the allocation size is a
multiple of the pointee's type size. This patch reverses the behavior
to try to prove that the expression is not a multiple of the pointee's
type size. With this change, each unhandled case should be gracefully
considered as correct. This fixes the bug reported in PR 109577 by
Paul Eggert.
Regression-tested on Linux x86-64 with -m32 and -m64.
2023-06-09 Tim Lange <mail@tim-lange.me>
PR analyzer/109577
gcc/analyzer/ChangeLog:
* constraint-manager.cc (class sval_finder): Visitor to find
childs in svalue trees.
(constraint_manager::sval_constrained_p): Add new function to
check whether a sval might be part of an constraint.
* constraint-manager.h: Add sval_constrained_p function.
* region-model.cc (class size_visitor): Reverse behavior to not
emit a warning on not explicitly considered cases.
(region_model::check_region_size):
Adapt to size_visitor changes.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/allocation-size-2.c: Change expected output
and add new test case.
* gcc.dg/analyzer/pr109577.c: New test.
|
|
|
|
PR analyzer/110112 notes that -fanalyzer is extremely slow on a source
file with large read-only static arrays, repeatedly building the
same compound_svalue representing the full initializer, and repeatedly
building svalues representing parts of the the full initialiazer.
This patch adds caches for both of these; together they reduce the time
taken by -fanalyzer -O2 on the testcase in the bug for an optimized
build:
91.2s : no caches (status quo)
32.4s : cache in decl_region::get_svalue_for_constructor
3.7s : cache in region::get_initial_value_at_main
3.1s : both caches (this patch)
gcc/analyzer/ChangeLog:
PR analyzer/110112
* region-model.cc (region_model::get_initial_value_for_global):
Move code to region::calc_initial_value_at_main.
* region.cc (region::get_initial_value_at_main): New function.
(region::calc_initial_value_at_main): New function, based on code
in region_model::get_initial_value_for_global.
(region::region): Initialize m_cached_init_sval_at_main.
(decl_region::get_svalue_for_constructor): Add a cache, splitting
out body to...
(decl_region::calc_svalue_for_constructor): ...this new function.
* region.h (region::get_initial_value_at_main): New decl.
(region::calc_initial_value_at_main): New decl.
(region::m_cached_init_sval_at_main): New field.
(decl_region::decl_region): Initialize m_ctor_svalue.
(decl_region::calc_svalue_for_constructor): New decl.
(decl_region::m_ctor_svalue): New field.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
This patch enhances -Wanalyzer-out-of-bounds that is no longer paired
with a -Wanalyzer-use-of-uninitialized-value on out-of-bounds-read.
This also fixes PR analyzer/109437.
Before there could always be at most one OOB-read warning per frame because
-Wanalyzer-use-of-uninitialized-value always terminates the analysis
path.
PR 109439
gcc/analyzer/ChangeLog:
* bounds-checking.cc (region_model::check_symbolic_bounds): Returns whether the BASE_REG
region access was OOB.
(region_model::check_region_bounds): Likewise.
* region-model.cc (region_model::get_store_value): Creates an
unknown svalue on OOB-read access to REG.
(region_model::check_region_access): Returns whether an unknown svalue needs be created.
(region_model::check_region_for_read): Passes check_region_access return value.
* region-model.h: Update prior function definitions.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/out-of-bounds-2.c: Cleaned test for uninitialized-value warning
* gcc.dg/analyzer/out-of-bounds-5.c: Likewise.
* gcc.dg/analyzer/pr101962.c: Likewise.
* gcc.dg/analyzer/realloc-5.c: Likewise.
* gcc.dg/analyzer/pr109439.c: New test.
|
|
|
|
This patch implements many of the __atomic_* builtins from
sync-builtins.def as known_function subclasses within the analyzer.
gcc/analyzer/ChangeLog:
PR analyzer/109015
* kf.cc (class kf_atomic_exchange): New.
(class kf_atomic_exchange_n): New.
(class kf_atomic_fetch_op): New.
(class kf_atomic_op_fetch): New.
(class kf_atomic_load): New.
(class kf_atomic_load_n): New.
(class kf_atomic_store_n): New.
(register_atomic_builtins): New function.
(register_known_functions): Call register_atomic_builtins.
gcc/testsuite/ChangeLog:
PR analyzer/109015
* gcc.dg/analyzer/atomic-builtins-1.c: New test.
* gcc.dg/analyzer/atomic-builtins-haproxy-proxy.c: New test.
* gcc.dg/analyzer/atomic-builtins-qemu-sockets.c: New test.
* gcc.dg/analyzer/atomic-types-1.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
* store.cc (store::eval_alias_1): Regions in different memory
spaces can't alias.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
* region-model-manager.cc (get_code_for_cast): Use _P defines from
tree.h.
(region_model_manager::get_or_create_cast): Ditto.
(region_model_manager::get_region_for_global): Ditto.
* region-model.cc (region_model::get_lvalue_1): Ditto.
* region.cc (decl_region::maybe_get_constant_value): Ditto.
|
|
|
|
[PR109239]
The patch has this effect on my integration tests of -fanalyzer:
Comparison:
GOOD: 129 (17.70% -> 17.92%)
BAD: 600 -> 591 (-9)
which is purely due to improvements to -Wanalyzer-deref-before-check
on the Linux kernel:
-Wanalyzer-deref-before-check:
GOOD: 1 (4.55% -> 7.69%)
BAD: 21 -> 12 (-9)
Known false positives: 16 -> 10 (-6)
linux-5.10.162: 7 -> 1 (-6)
Suspected false positives: 3 -> 0 (-3)
linux-5.10.162: 3 -> 0 (-3)
gcc/analyzer/ChangeLog:
PR analyzer/109239
* program-point.cc: Include "analyzer/inlining-iterator.h".
(program_point::effectively_intraprocedural_p): New function.
* program-point.h (program_point::effectively_intraprocedural_p):
New decl.
* sm-malloc.cc (deref_before_check::emit): Use it when rejecting
interprocedural cases, so that we reject interprocedural cases
that have become intraprocedural due to inlining.
gcc/testsuite/ChangeLog:
PR analyzer/109239
* gcc.dg/analyzer/deref-before-check-pr109239-linux-bus.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/109094 reports an ICE in the analyzer seen on qemu's
target/i386/tcg/translate.c
The issue turned out to be that when handling a longjmp, the code
to pop the frames was generating an svalue for the result_decl of any
popped frame that had a non-void return type (and discarding it) leading
to "uninit" poisoned_svalue_diagnostic instances being saved since the
result_decl is only set by the greturn stmt. Later, when checking the
feasibility of the path to these diagnostics, m_check_expr was evaluated
in the context of the frame of the longjmp, leading to an attempt to
evaluate the result_decl of each intervening frames whilst in the
context of the topmost frame, leading to an assertion failure in
frame_region::get_region_for_local here:
919 case RESULT_DECL:
920 gcc_assert (DECL_CONTEXT (expr) == m_fun->decl);
921 break;
This patch updates the analyzer's longjmp implementation so that it
doesn't attempt to generate svalues for the result_decls when popping
frames, fixing the assertion failure (and presumably fixing "uninit"
false positives in a release build).
gcc/analyzer/ChangeLog:
PR analyzer/109094
* region-model.cc (region_model::on_longjmp): Pass false for
new "eval_return_svalue" param of pop_frame.
(region_model::pop_frame): Add new "eval_return_svalue" param and
use it to suppress the call to get_rvalue on the result when
needed by on_longjmp.
* region-model.h (region_model::pop_frame): Add new
"eval_return_svalue" param.
gcc/testsuite/ChangeLog:
PR analyzer/109094
* gcc.dg/analyzer/setjmp-pr109094.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
If a bound region gets overwritten with UNKNOWN due to being
possibly-aliased during a write, that could have been the only
region keeping its value live, in which case we could falsely report
a leak. This is hidden somewhat by the "uncertainty" mechanism for
cases where the write happens in the same stmt as the last reference
to the value goes away, but not in the general case, which occurs
in PR analyzer/109059, which falsely complains about a leak whilst
haproxy updates a doubly-linked list.
The whole "uncertainty_t" class seems broken to me now; I think we need
to track (in the store) what values could have escaped to the external
part of the program. We do this to some extent for pointers by tracking
the region as escaped, though we're failing to do this for this case:
even though there could still be other pointers to the region,
eventually they go away; we want to capture the fact that the external
part of the state is still keeping it live. Also, this doesn't work for
non-pointer svalues, such as for detecting file-descriptor leaks.
As both a workaround and a step towards eventually removing
"class uncertainty_t" this patch updates the "mark_region_as_unknown"
code called by possibly-aliased set_value so that when old values are
removed, any base region pointed to them is marked as escaped, fixing
the leak false positive.
The patch has this effect on my integration tests of -fanalyzer:
Comparison:
GOOD: 129 (19.20% -> 20.22%)
BAD: 543 -> 509 (-34)
where there's a big improvement in -Wanalyzer-malloc-leak:
-Wanalyzer-malloc-leak:
GOOD: 61 (45.19% -> 54.95%)
BAD: 74 -> 50 (-24)
Known false positives: 25 -> 2 (-23)
haproxy-2.7.1: 24 -> 1 (-23)
Suspected false positives: 49 -> 48 (-1)
coreutils-9.1: 32 -> 31 (-1)
and some churn in the other warnings:
-Wanalyzer-use-of-uninitialized-value:
GOOD: 0
BAD: 81 -> 80 (-1)
-Wanalyzer-file-leak:
GOOD: 0
BAD: 10 -> 11 (+1)
-Wanalyzer-out-of-bounds:
GOOD: 0
BAD: 24 -> 22 (-2)
gcc/analyzer/ChangeLog:
PR analyzer/109059
* region-model.cc (region_model::mark_region_as_unknown): Gather a
set of maybe-live svalues and call on_maybe_live_values with it.
* store.cc (binding_map::remove_overlapping_bindings): Add new
"maybe_live_values" param; add any removed svalues to it.
(binding_cluster::clobber_region): Add NULL as new param of
remove_overlapping_bindings.
(binding_cluster::mark_region_as_unknown): Add "maybe_live_values"
param and pass it to remove_overlapping_bindings.
(binding_cluster::maybe_get_compound_binding): Add NULL for new
param of binding_map::remove_overlapping_bindings.
(binding_cluster::remove_overlapping_bindings): Add
"maybe_live_values" param and pass to
binding_map::remove_overlapping_bindings.
(store::set_value): Capture a set of maybe-live svalues, and call
on_maybe_live_values with it.
(store::on_maybe_live_values): New.
(store::mark_region_as_unknown): Add "maybe_live_values" param
and pass it to binding_cluster::mark_region_as_unknown.
(store::remove_overlapping_bindings): Pass NULL for new param of
binding_cluster::remove_overlapping_bindings.
* store.h (binding_map::remove_overlapping_bindings): Add
"maybe_live_values" param.
(binding_cluster::mark_region_as_unknown): Likewise.
(binding_cluster::remove_overlapping_bindings): Likewise.
(store::mark_region_as_unknown): Likewise.
(store::on_maybe_live_values): New decl.
gcc/testsuite/ChangeLog:
PR analyzer/109059
* gcc.dg/analyzer/flex-with-call-summaries.c: Remove xfail.
* gcc.dg/analyzer/leak-pr109059-1.c: New test.
* gcc.dg/analyzer/leak-pr109059-2.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Integration testing showed various false positives from
-Wanalyzer-deref-before-check where the expression that's dereferenced
is different from the one that's checked, but the diagnostic is emitted
because they both evaluate to the same symbolic value.
This patch rejects such warnings, unless we have tree expressions for
both and that both tree expressions are "spelled the same way" i.e.
would be printed to the same user-facing string.
gcc/analyzer/ChangeLog:
PR analyzer/108475
PR analyzer/109060
* sm-malloc.cc (deref_before_check::deref_before_check):
Initialize new field m_deref_expr. Assert that arg is non-NULL.
(deref_before_check::emit): Reject cases where the spelling of the
thing that was dereferenced differs from that of what is checked,
or if the dereference expression was not found. Remove code to
handle NULL m_arg.
(deref_before_check::describe_state_change): Remove code to handle
NULL m_arg.
(deref_before_check::describe_final_event): Likewise.
(deref_before_check::sufficiently_similar_p): New.
(deref_before_check::m_deref_expr): New field.
(malloc_state_machine::maybe_complain_about_deref_before_check):
Don't warn if the diag_ptr is NULL.
gcc/testsuite/ChangeLog:
PR analyzer/108475
PR analyzer/109060
* gcc.dg/analyzer/deref-before-check-pr108475-1.c: New test.
* gcc.dg/analyzer/deref-before-check-pr108475-haproxy-tcpcheck.c:
New test.
* gcc.dg/analyzer/deref-before-check-pr109060-haproxy-cfgparse.c:
New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
Previously, the analyzer lacked a known_function implementation of
sprintf, and thus would handle calls to sprintf with the "anything could
happen" fallback.
Whilst working on PR analyzer/107565 I noticed that this was preventing
a lot of genuine memory leaks from being reported for Doom; fixing
thusly.
Integration testing of the effect of the patch shows a big increase in
true positives due to the case mentioned in Doom, and one new false
positive (in pcre2), which I'm tracking as PR analyzer/109014.
Comparison:
GOOD: 67 -> 123 (+56); 10.91% -> 18.33%
BAD: 547 -> 548 (+1)
where the affected warnings/projects are:
-Wanalyzer-malloc-leak:
GOOD: 0 -> 56 (+56); 0.00% -> 41.48%
BAD: 79
True positives: 0 -> 56 (+56)
(all in Doom)
-Wanalyzer-use-of-uninitialized-value:
GOOD: 0; 0.00%
BAD: 80 -> 81 (+1)
False positives:
pcre2-10.42: 0 -> 1 (+1)
gcc/analyzer/ChangeLog:
* kf.cc (class kf_sprintf): New.
(register_known_functions): Register it.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/doom-d_main-IdentifyVersion.c: New test.
* gcc.dg/analyzer/sprintf-1.c: New test.
* gcc.dg/analyzer/sprintf-concat.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/108968
* region-model.cc (region_model::get_rvalue_1): Handle VAR_DECLs
with a DECL_HARD_REGISTER by returning UNKNOWN.
gcc/testsuite/ChangeLog:
PR analyzer/108968
* gcc.dg/analyzer/uninit-pr108968-register.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Without this definition, the errno definition for newlib
isn't recognized as such, and these tests fail for newlib
targets:
FAIL: gcc.dg/analyzer/call-summaries-errno.c (test for warnings, line 16)
FAIL: gcc.dg/analyzer/call-summaries-errno.c (test for excess errors)
FAIL: gcc.dg/analyzer/errno-1.c (test for warnings, line 20)
FAIL: gcc.dg/analyzer/errno-1.c (test for excess errors)
FAIL: gcc.dg/analyzer/flex-without-call-summaries.c (test for excess errors)
FAIL: gcc.dg/analyzer/isatty-1.c (test for warnings, line 31)
FAIL: gcc.dg/analyzer/isatty-1.c (test for warnings, line 35)
FAIL: gcc.dg/analyzer/isatty-1.c (test for warnings, line 46)
FAIL: gcc.dg/analyzer/isatty-1.c (test for warnings, line 56)
FAIL: gcc.dg/analyzer/isatty-1.c (test for excess errors)
gcc/analyzer:
* kf.cc (register_known_functions): Add __errno function for newlib.
|
|
|
|
Previously, if the analyzer saw a call to a non-pure and non-const
built-in function that it didn't have explicit knowledge of the behavior
of, it would fall back to assuming that the builtin could have arbitrary
behavior, similar to a function defined outside of the current TU.
However, this only worked for BUILTIN_NORMAL functions that matched
gimple_builtin_call_types_compatible_p; for BUILT_IN_FRONTEND and
BUILT_IN_MD, and for mismatched types the analyzer would erroneously
assume that the builtin had no side-effects, leading e.g. to
PR analyzer/107565, where the analyzer falsely reported that x
was still uninitialized after this target-specific builtin:
_1 = __builtin_ia32_rdrand64_step (&x);
This patch generalizes the handling to cover all classes of builtin,
fixing the above false positive.
Unfortunately this patch regresses gcc.dg/analyzer/pr99716-1.c due to
the:
fprintf (fp, "hello");
being optimized to:
__builtin_fwrite ("hello", 1, (ssizetype)5, fp_6);
and the latter has gimple_builtin_call_types_compatible_p return false,
whereas the original call had it return true. I'm assuming that this is
an optimization bug, and have filed it as PR middle-end/108988. The
effect on the analyzer is that it fails to recognize the call to
__builtin_fwrite and instead assumes arbitraty side-effects (including
that it could call fclose on fp, hence the report about the leak goes
away).
I tried various more involved fixes with new heuristics for handling
built-ins that aren't explicitly covered by the analyzer, but those
fixes tended to introduce many more regressions, so I'm going with this
simpler fix.
gcc/analyzer/ChangeLog:
PR analyzer/107565
* region-model.cc (region_model::on_call_pre): Flatten logic by
returning early. Consolidate logic for detecting const and pure
functions. When considering whether an unhandled built-in
function has side-effects, consider all kinds of builtin, rather
than just BUILT_IN_NORMAL, and don't require
gimple_builtin_call_types_compatible_p.
gcc/testsuite/ChangeLog:
PR analyzer/107565
* gcc.dg/analyzer/builtins-pr107565.c: New test.
* gcc.dg/analyzer/pr99716-1.c (test_2): Mark the leak as xfailing.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
PR analyzer/108935
* infinite-recursion.cc (contains_unknown_p): New.
(sufficiently_different_region_binding_p): New function, splitting
out inner loop from...
(sufficiently_different_p): ...here. Extend detection of unknown
svalues to also include svalues that contain unknown. Treat
changes in frames below the entry to the recursion as being
sufficiently different to reject being an infinite recursion.
gcc/testsuite/ChangeLog:
PR analyzer/108935
* gcc.dg/analyzer/infinite-recursion-pr108935-1.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108935-1a.c: New test.
* gcc.dg/analyzer/infinite-recursion-pr108935-2.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
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>
|
|
|
|
PR analyzer/108806 reports false +ves seen from -fanalyzer on code like this
in qemu-7.2.0's hw/intc/omap_intc.c:
[...snip...]
struct omap_intr_handler_bank_s* bank = NULL;
if ((offset & 0xf80) == 0x80) {
[...set "bank" to non-NULL...]
}
switch (offset) {
[...snip various cases that don't deref "bank"...]
case 0x80:
return bank->inputs;
case 0x84:
return bank->mask;
[...etc...]
}
where the analyzer falsely complains about execution paths in which
"(offset & 0xf80) == 0x80" was false (leaving "bank" as NULL), but then
in which "switch (offset)" goes to a case for which
"(offset & 0xf80) == 0x80" is true and dereferences NULL "bank", i.e.
paths in which "(offset & 0xf80) == 0x80" is both true *and* false.
This patch adds enough logic to constraint_manager for -fanalyzer to
reject such execution paths as impossible, fixing the false +ves.
Integration testing shows this eliminates 20 probable false positives:
Comparison: 9.08% -> 9.34% GOOD: 66 BAD: 661 -> 641 (-20)
where the affected warnings/projects are:
-Wanalyzer-null-dereference: 0.00% GOOD: 0 BAD: 279 -> 269 (-10)
qemu-7.2.0: 175 -> 165 (-10)
-Wanalyzer-use-of-uninitialized-value: 0.00% GOOD: 0 BAD: 153 -> 143 (-10)
coreutils-9.1: 18 -> 14 (-4)
qemu-7.2.0: 54 -> 48 (-6)
gcc/analyzer/ChangeLog:
PR analyzer/108806
* constraint-manager.cc (bounded_range::dump_to_pp): Use
bounded_range::singleton_p.
(constraint_manager::add_bounded_ranges): Handle singleton ranges
by adding an EQ_EXPR constraint.
(constraint_manager::impossible_derived_conditions_p): New.
(constraint_manager::eval_condition): Reject EQ_EXPR when it would
imply impossible derived conditions.
(selftest::test_bits): New.
(selftest::run_constraint_manager_tests): Run it.
* constraint-manager.h (bounded_range::singleton_p): New.
(constraint_manager::impossible_derived_conditions_p): New decl.
* region-model.cc (region_model::get_rvalue_1): Handle
BIT_AND_EXPR, BIT_IOR_EXPR, and BIT_XOR_EXPR.
gcc/testsuite/ChangeLog:
PR analyzer/108806
* gcc.dg/analyzer/null-deref-pr108806-qemu.c: New test.
* gcc.dg/analyzer/pr103217.c: Add -Wno-analyzer-too-complex.
* gcc.dg/analyzer/switch.c (test_bitmask_1): New.
(test_bitmask_2): New.
* gcc.dg/analyzer/uninit-pr108806-qemu.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
This patch updates poisoned_value_diagnostic so that, where possible,
it checks to see if the value is still poisoned along the execution
path seen during feasibility analysis, rather than just that seen
in the exploded graph.
Integration testing shows this reduction in the number of
false positives:
-Wanalyzer-use-of-uninitialized-value: 191 -> 153 (-38)
where the changes happen in:
coreutils-9.1: 34 -> 20 (-14)
qemu-7.2.0: 78 -> 54 (-24)
gcc/analyzer/ChangeLog:
PR analyzer/108664
PR analyzer/108666
PR analyzer/108725
* diagnostic-manager.cc (epath_finder::get_best_epath): Add
"target_stmt" param.
(epath_finder::explore_feasible_paths): Likewise.
(epath_finder::process_worklist_item): Likewise.
(saved_diagnostic::calc_best_epath): Pass m_stmt to
epath_finder::get_best_epath.
* engine.cc (feasibility_state::maybe_update_for_edge): Move
per-stmt logic to...
(feasibility_state::update_for_stmt): ...this new function.
* exploded-graph.h (feasibility_state::update_for_stmt): New decl.
* feasible-graph.cc (feasible_node::get_state_at_stmt): New.
* feasible-graph.h: Include "analyzer/exploded-graph.h".
(feasible_node::get_state_at_stmt): New decl.
* infinite-recursion.cc
(infinite_recursion_diagnostic::check_valid_fpath_p): Update for
vfunc signature change.
* pending-diagnostic.h (pending_diagnostic::check_valid_fpath_p):
Convert first param to a reference. Add stmt param.
* region-model.cc: Include "analyzer/feasible-graph.h".
(poisoned_value_diagnostic::poisoned_value_diagnostic): Add
"check_expr" param.
(poisoned_value_diagnostic::check_valid_fpath_p): New.
(poisoned_value_diagnostic::m_check_expr): New field.
(region_model::check_for_poison): Attempt to supply a check_expr
to the diagnostic
(region_model::deref_rvalue): Add NULL for new check_expr param
of poisoned_value_diagnostic.
(region_model::get_or_create_region_for_heap_alloc): Don't reuse
regions that are marked as TOUCHED.
gcc/testsuite/ChangeLog:
PR analyzer/108664
PR analyzer/108666
PR analyzer/108725
* gcc.dg/analyzer/coreutils-cksum-pr108664.c: New test.
* gcc.dg/analyzer/coreutils-sum-pr108666.c: New test.
* gcc.dg/analyzer/torture/uninit-pr108725.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
Integration testing shows this patch fixes all 9 known false positives
from -Wanalyzer-deref-before-check within ImageMagick-7.1.0-57, and
eliminates 34 further as-yet unassessed such diagnostics, without
eliminating the 1 known true positive.
This improves the rate of true positives for the warning from
1.56% to 4.76% of the total:
-Wanalyzer-deref-before-check: 1.56% -> 4.76% (GOOD: 1 BAD: 63->20)
TRUE: 1
FALSE: 15 -> 6 (-9)
ImageMagick-7.1.0-57: 9 -> 0 (-9)
TODO: 48 -> 14 (-34)
ImageMagick-7.1.0-57: 21 -> 1 (-20)
qemu-7.2.0: 25 -> 11 (-14)
gcc/analyzer/ChangeLog:
PR analyzer/108745
* sm-malloc.cc (deref_before_check::emit): Reject the warning if
the check occurs within a macro defintion.
gcc/testsuite/ChangeLog:
PR analyzer/108745
* gcc.dg/analyzer/deref-before-check-macro-pr108745.c: New test.
* gcc.dg/analyzer/deref-before-check-macro.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
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>
|
