| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Avoid generating execution paths for warnings that are ultimately
rejected due to -Wno-analyzer-* flags.
This improves the test case from taking at least several minutes
(before I killed it) to taking under a second.
This doesn't fix the slowdown seen in PR analyzer/104955 with large
numbers of warnings when the warnings are still enabled.
gcc/analyzer/ChangeLog:
PR analyzer/104955
* diagnostic-manager.cc (get_emission_location): New.
(diagnostic_manager::diagnostic_manager): Initialize
m_num_disabled_diagnostics.
(diagnostic_manager::add_diagnostic): Reject diagnostics that
will eventually be rejected due to being disabled.
(diagnostic_manager::emit_saved_diagnostics): Log the number
of disabled diagnostics.
(diagnostic_manager::emit_saved_diagnostic): Split out logic for
determining emission location to get_emission_location.
* diagnostic-manager.h
(diagnostic_manager::m_num_disabled_diagnostics): New field.
* engine.cc (stale_jmp_buf::get_controlling_option): New.
(stale_jmp_buf::emit): Use it.
* pending-diagnostic.h
(pending_diagnostic::get_controlling_option): New vfunc.
* region-model.cc
(poisoned_value_diagnostic::get_controlling_option): New.
(poisoned_value_diagnostic::emit): Use it.
(shift_count_negative_diagnostic::get_controlling_option): New.
(shift_count_negative_diagnostic::emit): Use it.
(shift_count_overflow_diagnostic::get_controlling_option): New.
(shift_count_overflow_diagnostic::emit): Use it.
(dump_path_diagnostic::get_controlling_option): New.
(dump_path_diagnostic::emit): Use it.
(write_to_const_diagnostic::get_controlling_option): New.
(write_to_const_diagnostic::emit): Use it.
(write_to_string_literal_diagnostic::get_controlling_option): New.
(write_to_string_literal_diagnostic::emit): Use it.
* sm-file.cc (double_fclose::get_controlling_option): New.
(double_fclose::emit): Use it.
(file_leak::get_controlling_option): New.
(file_leak::emit): Use it.
* sm-malloc.cc (mismatching_deallocation::get_controlling_option):
New.
(mismatching_deallocation::emit): Use it.
(double_free::get_controlling_option): New.
(double_free::emit): Use it.
(possible_null_deref::get_controlling_option): New.
(possible_null_deref::emit): Use it.
(possible_null_arg::get_controlling_option): New.
(possible_null_arg::emit): Use it.
(null_deref::get_controlling_option): New.
(null_deref::emit): Use it.
(null_arg::get_controlling_option): New.
(null_arg::emit): Use it.
(use_after_free::get_controlling_option): New.
(use_after_free::emit): Use it.
(malloc_leak::get_controlling_option): New.
(malloc_leak::emit): Use it.
(free_of_non_heap::get_controlling_option): New.
(free_of_non_heap::emit): Use it.
* sm-pattern-test.cc (pattern_match::get_controlling_option): New.
(pattern_match::emit): Use it.
* sm-sensitive.cc
(exposure_through_output_file::get_controlling_option): New.
(exposure_through_output_file::emit): Use it.
* sm-signal.cc (signal_unsafe_call::get_controlling_option): New.
(signal_unsafe_call::emit): Use it.
* sm-taint.cc (tainted_array_index::get_controlling_option): New.
(tainted_array_index::emit): Use it.
(tainted_offset::get_controlling_option): New.
(tainted_offset::emit): Use it.
(tainted_size::get_controlling_option): New.
(tainted_size::emit): Use it.
(tainted_divisor::get_controlling_option): New.
(tainted_divisor::emit): Use it.
(tainted_allocation_size::get_controlling_option): New.
(tainted_allocation_size::emit): Use it.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/many-disabled-diagnostics.c: New test.
* gcc.dg/plugin/analyzer_gil_plugin.c
(gil_diagnostic::get_controlling_option): New.
(double_save_thread::emit): Use it.
(fncall_without_gil::emit): Likewise.
(pyobject_usage_without_gil::emit): Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
Testing cc1 on pr93032-mztools-unsigned-char.c
Benchmark #1: (without patch)
Time (mean ± σ): 338.8 ms ± 13.6 ms [User: 323.2 ms, System: 14.2 ms]
Range (min … max): 326.7 ms … 363.1 ms 10 runs
Benchmark #2: (with patch)
Time (mean ± σ): 332.3 ms ± 12.8 ms [User: 316.6 ms, System: 14.3 ms]
Range (min … max): 322.5 ms … 357.4 ms 10 runs
Summary
./cc1.new ran 1.02 ± 0.06 times faster than ./cc1.old
gcc/analyzer/ChangeLog:
* store.cc (store::store): Presize m_cluster_map.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/104863
* constraint-manager.cc (constraint_manager::add_constraint):
Refresh the EC IDs when adding constraints implied by offsets.
gcc/testsuite/ChangeLog:
PR analyzer/104863
* gcc.dg/analyzer/torture/pr104863.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
The previous patch extended
-Wanalyzer-write-to-const
-Wanalyzer-write-to-string-literal
to make use of __attribute__ ((access, ....), but the results could be
inscrutable.
This patch adds notes to such diagnostics to give the user a reason for
why the analyzer is complaining.
Example output:
test.c: In function 'main':
test.c:15:13: warning: write to string literal [-Wanalyzer-write-to-string-literal]
15 | if (getrandom((char *)test, sizeof(buf), GRND_RANDOM))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
'main': event 1
|
| 15 | if (getrandom((char *)test, sizeof(buf), GRND_RANDOM))
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (1) write to string literal here
|
test.c:3:5: note: parameter 1 of 'getrandom' marked with attribute 'access (write_only, 1, 2)'
3 | int getrandom (void *__buffer, size_t __length,
| ^~~~~~~~~
Unfortunately we don't have location information for the attributes
themselves, just the function declaration, and there doesn't seem to be
a good way of getting at the location of the individual parameters from
the middle end (the C and C++ FEs both have get_fndecl_argument_location,
but the implementations are different).
gcc/analyzer/ChangeLog:
PR analyzer/104793
* analyzer.h (class pending_note): New forward decl.
* diagnostic-manager.cc (saved_diagnostic::saved_diagnostic):
Initialize m_notes.
(saved_diagnostic::operator==): Compare m_notes.
(saved_diagnostic::add_note): New.
(saved_diagnostic::emit_any_notes): New.
(diagnostic_manager::add_note): New.
(diagnostic_manager::emit_saved_diagnostic): Call emit_any_notes
after emitting the warning.
* diagnostic-manager.h (saved_diagnostic::add_note): New decl.
(saved_diagnostic::emit_any_notes): New decl.
(saved_diagnostic::m_notes): New field.
(diagnostic_manager::add_note): New decl.
* engine.cc (impl_region_model_context::add_note): New.
* exploded-graph.h (impl_region_model_context::add_note): New
decl.
* pending-diagnostic.h (class pending_note): New.
(class pending_note_subclass): New template.
* region-model.cc (class reason_attr_access): New.
(check_external_function_for_access_attr): Add class
annotating_ctxt and use it when checking region.
(noop_region_model_context::add_note): New.
* region-model.h (region_model_context::add_note): New vfunc.
(noop_region_model_context::add_note): New decl.
(class region_model_context_decorator): New.
(class note_adding_context): New.
gcc/testsuite/ChangeLog:
PR analyzer/104793
* gcc.dg/analyzer/write-to-const-2.c: Add dg-message directives
for expected notes.
* gcc.dg/analyzer/write-to-function-1.c: Likewise.
* gcc.dg/analyzer/write-to-string-literal-2.c: Likewise.
* gcc.dg/analyzer/write-to-string-literal-3.c: Likewise.
* gcc.dg/analyzer/write-to-string-literal-4.c: Likewise.
* gcc.dg/analyzer/write-to-string-literal-5.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
This patch extends:
-Wanalyzer-write-to-const
-Wanalyzer-write-to-string-literal
so that they will check for __attribute__ ((access, ....) on calls to
externally-defined functions, and complain about read-only regions
pointed to by arguments marked with a "write_only" or "read_write"
attribute.
gcc/analyzer/ChangeLog:
PR analyzer/104793
* region-model.cc
(region_model::check_external_function_for_access_attr): New.
(region_model::handle_unrecognized_call): Call it.
* region-model.h
(region_model::check_external_function_for_access_attr): New decl.
(region_model::handle_unrecognized_call): New decl.
gcc/testsuite/ChangeLog:
PR analyzer/104793
* gcc.dg/analyzer/write-to-const-2.c: New test.
* gcc.dg/analyzer/write-to-function-1.c: New test.
* gcc.dg/analyzer/write-to-string-literal-2.c: New test.
* gcc.dg/analyzer/write-to-string-literal-3.c: New test.
* gcc.dg/analyzer/write-to-string-literal-4.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
* sm-taint.cc (taint_state_machine::check_for_tainted_size_arg):
Avoid generating duplicate saved_diagnostics by only handling the
rdwr_map entry for the ptrarg, not the duplicate entry for the
sizarg.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/taint-size-access-attr-1.c: Add
-fanalyzer-show-duplicate-count to options; verify that a
duplicate was not created for the tainted size.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/101983 reports what I thought were false positives
from -Wanalyzer-malloc-leak, but on closer inspection, the
analyzer is correctly reporting heap-allocated buffers that are
no longer reachable.
However, these "leaks" occur at the end of "main". The analyzer already
has some logic to avoid reporting leaks at the end of main, where the
leak is detected at the end of the EXIT basic block. However, in this case,
the leak is detected at the clobber in BB 2 here:
<bb 2> :
func (&res);
res ={v} {CLOBBER(eol)};
_4 = 0;
<bb 3> :
<L0>:
return _4;
where we have a chain BB 2 -> BB 3 -> EXIT BB.
This patch generalizes the "are we at the end of 'main'" detection to
handle such cases, silencing -Wanalyzer-malloc-leak on them.
There's a remaining issue where the analyzer unhelpfully describes one
of the leaking values as '<unknown>', rather than 'res.a', but I'm
leaving that for a followup (covered by PR analyzer/99771).
gcc/analyzer/ChangeLog:
PR analyzer/101983
* engine.cc (returning_from_function_p): New.
(impl_region_model_context::on_state_leak): Use it when rejecting
leaks at the return from "main".
gcc/testsuite/ChangeLog:
PR analyzer/101983
* gcc.dg/analyzer/pr101983-main.c: New test.
* gcc.dg/analyzer/pr101983-not-main.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Like in r10-7215-g700d4cb08c88aec37c13e21e63dd61fd698baabc 2 years ago,
I've run
grep -v 'long long\|optab optab\|template template\|double double' *.{[chS],cc} */*.{[chS],cc} *.def config/*/* 2>/dev/null | grep ' \([a-zA-Z]\+\) \1 '
and for the cases that looked clearly wrong changed them, mostly by removing
one of the duplicated words but in some cases with other changes.
2022-03-07 Jakub Jelinek <jakub@redhat.com>
gcc/
* tree-ssa-propagate.cc: Fix up duplicated word issue in a comment.
* config/riscv/riscv.cc: Likewise.
* config/darwin.h: Likewise.
* config/i386/i386.cc: Likewise.
* config/aarch64/thunderx3t110.md: Likewise.
* config/aarch64/fractional-cost.h: Likewise.
* config/vax/vax.cc: Likewise.
* config/rs6000/pcrel-opt.md: Likewise.
* config/rs6000/predicates.md: Likewise.
* ctfc.h: Likewise.
* tree-ssa-uninit.cc: Likewise.
* value-relation.h: Likewise.
* gimple-range-gori.cc: Likewise.
* ipa-polymorphic-call.cc: Likewise.
* pointer-query.cc: Likewise.
* ipa-sra.cc: Likewise.
* internal-fn.cc: Likewise.
* varasm.cc: Likewise.
* gimple-ssa-warn-access.cc: Likewise.
gcc/analyzer/
* store.cc: Fix up duplicated word issue in a comment.
* analyzer.cc: Likewise.
* engine.cc: Likewise.
* sm-taint.cc: Likewise.
gcc/c-family/
* c-attribs.cc: Fix up duplicated word issue in a comment.
gcc/cp/
* cvt.cc: Fix up duplicated word issue in a comment.
* pt.cc: Likewise.
* module.cc: Likewise.
* coroutines.cc: Likewise.
gcc/fortran/
* trans-expr.cc: Fix up duplicated word issue in a comment.
* gfortran.h: Likewise.
* scanner.cc: Likewise.
gcc/jit/
* libgccjit.h: Fix up duplicated word issue in a comment.
|
|
|
|
PR analyzer/103521 reports that commit r12-5585-g132902177138c09803d639e12b1daebf2b9edddc
("analyzer: further false leak fixes due to overzealous state merging [PR103217]")
led to failures of gcc.dg/analyzer/pr93032-mztools.c on some targets,
where rather than reporting FILE * leaks, the analyzer would hit
complexity limits and give up.
The cause is that pr93032-mztools.c has some 'unsigned char' values that
are copied to 'char'. On targets where 'char' defaults to being signed,
this leads to casts, whereas on targets where 'char' defaults to being
unsigned, no casts are needed.
When the casts occur, various symbolic values within the loop (the
locals 'crc', 'cpsize', and 'uncpsize') become sufficiently complex as
to hit the --param=analyzer-max-svalue-depth= limit, and are treated as
UNKNOWN, allowing the analysis of the loop to quickly terminate, with
much of this state as UNKNOWN (but retaining the FILE * information, and
thus correctly reporting the FILE * leaks).
Without the casts, the symbolic values for these variables don't quite
hit the complexity limit, and the analyzer attempts to track these
values in the loop, leading to the analyzer eventually hitting the
per-program-point limit on the number of states, and giving up on
these execution paths, thus failing to report the FILE * leaks.
This patch tweaks the default value of the param:
--param=analyzer-max-svalue-depth=.
from 13 down to 12. This allows the pr93032-mztools.c testcase to
succeeed with both -fsigned-char and -funsigned-char, and thus allows
this integration test to succeed on both styles of target without
requiring extra command-line flags. The patch duplicates the test so
it runs with both -fsigned-char and -funsigned-char.
My hope is that this will allow similar cases to terminate loop analysis
earlier. I tried reducing it further, but doing so caused some test
cases to regress.
The tradeoff here is between:
(a) precision of individual states in the analysis, versus
(b) maximizing code-path coverage in the analysis
I can imagine a more nuanced approach that splits the current
per-program-point hard limit into soft and hard limits: on hitting the
soft limit at a program point, go into a less precise mode for states
at that program point, in the hope that we can fully explore execution
paths beyond it without hitting the hard limit, but this seems like
GCC 13 material.
Another possible future fix might be for the analysis plan to make an
attempt to prioritize parts of the code in an enode budget, rather than
setting the same hard limit uniformly across all program points.
gcc/analyzer/ChangeLog:
PR analyzer/103521
* analyzer.opt (-param=analyzer-max-svalue-depth=): Reduce from 13
to 12.
gcc/testsuite/ChangeLog:
PR analyzer/103521
* gcc.dg/analyzer/pr93032-mztools.c: Move to...
* gcc.dg/analyzer/pr93032-mztools-signed-char.c: ...this, adding
-fsigned-char to args, and...
* gcc.dg/analyzer/pr93032-mztools-unsigned-char.c: ...copy to here,
adding -funsigned-char to args.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
When testing -fanalyzer on openblas-0.3, I noticed slightly over 2000
false positives from -Wanalyzer-malloc-leak on code like this:
if( LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'p' ) ) {
pt_t = (lapack_complex_float*)
LAPACKE_malloc( sizeof(lapack_complex_float) *
ldpt_t * MAX(1,n) );
[...snip...]
}
[...snip lots of code...]
if( LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'q' ) ) {
LAPACKE_free( pt_t );
}
where LAPACKE_lsame is a char-comparison function implemented in a
different TU.
The analyzer naively considers the execution path where:
LAPACKE_lsame( vect, 'b' ) || LAPACKE_lsame( vect, 'p' )
is true at the malloc guard, but then false at the free guard, which
is thus a memory leak.
This patch makes -fanalyer respect __attribute__((const)), so that the
analyzer treats such functions as returning the same value when given
the same inputs.
I've filed https://github.com/xianyi/OpenBLAS/issues/3543 suggesting that
LAPACKE_lsame be annotated with __attribute__((const)); with that, and
with this patch, the false positives seem to be fixed.
gcc/analyzer/ChangeLog:
PR analyzer/104434
* analyzer.h (class const_fn_result_svalue): New decl.
* region-model-impl-calls.cc (call_details::get_manager): New.
* region-model-manager.cc
(region_model_manager::get_or_create_const_fn_result_svalue): New.
(region_model_manager::log_stats): Log
m_const_fn_result_values_map.
* region-model.cc (const_fn_p): New.
(maybe_get_const_fn_result): New.
(region_model::on_call_pre): Handle fndecls with
__attribute__((const)) by calling the above rather than making
a conjured_svalue.
* region-model.h (visitor::visit_const_fn_result_svalue): New.
(region_model_manager::get_or_create_const_fn_result_svalue): New
decl.
(region_model_manager::const_fn_result_values_map_t): New typedef.
(region_model_manager::m_const_fn_result_values_map): New field.
(call_details::get_manager): New decl.
* svalue.cc (svalue::cmp_ptr): Handle SK_CONST_FN_RESULT.
(const_fn_result_svalue::dump_to_pp): New.
(const_fn_result_svalue::dump_input): New.
(const_fn_result_svalue::accept): New.
* svalue.h (enum svalue_kind): Add SK_CONST_FN_RESULT.
(svalue::dyn_cast_const_fn_result_svalue): New.
(class const_fn_result_svalue): New.
(is_a_helper <const const_fn_result_svalue *>::test): New.
(template <> struct default_hash_traits<const_fn_result_svalue::key_t>):
New.
gcc/testsuite/ChangeLog:
PR analyzer/104434
* gcc.dg/analyzer/attr-const-1.c: New test.
* gcc.dg/analyzer/attr-const-2.c: New test.
* gcc.dg/analyzer/attr-const-3.c: New test.
* gcc.dg/analyzer/pr104434-const.c: New test.
* gcc.dg/analyzer/pr104434-nonconst.c: New test.
* gcc.dg/analyzer/pr104434.h: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/104576 tracks that we issue a false positive from
-Wanalyzer-use-of-uninitialized-value for the reproducers of PR 63311
when optimization is disabled.
The root cause is that the analyzer was considering that a call to
__builtin_sinf could have side-effects.
This patch fixes things by generalizing the handling for "pure"
functions to also consider "const" functions.
gcc/analyzer/ChangeLog:
PR analyzer/104576
* region-model.cc: Include "calls.h".
(region_model::on_call_pre): Use flags_from_decl_or_type to
generalize check for DECL_PURE_P to also check for ECF_CONST.
gcc/testsuite/ChangeLog:
PR analyzer/104576
* gcc.dg/analyzer/torture/uninit-pr63311.c: New test.
* gcc.dg/analyzer/uninit-pr104576.c: New test.
* gfortran.dg/analyzer/uninit-pr63311.f90: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/104560 reports various false positives from
-Wanalyzer-free-of-non-heap seen with rdma-core, on what's
effectively:
free (&ptr->field)
where in this case "field" is the first element of its struct, and thus
&ptr->field == ptr, and could be on the heap.
The root cause is due to malloc_state_machine::on_stmt making
"LHS = &EXPR;"
transition LHS from start to non_heap when EXPR is not a MEM_REF;
this assumption doesn't hold for the above case.
This patch eliminates that state transition, instead relying on
malloc_state_machine::get_default_state to detect regions known to
not be on the heap.
Doing so fixes the false positive, but eliminates some events relating
to free-of-alloca identifying the alloca, so the patch also reworks
free_of_non_heap to capture which region has been freed, adding
region creation events to diagnostic paths, so that the alloca calls
can be identified, and using the memory space of the region for more
precise wording of the diagnostic.
The improvement to malloc_state_machine::get_default_state also
means we now detect attempts to free VLAs, functions and code labels.
In doing so I spotted that I wasn't adding region creation events for
regions for global variables, and for cases where an allocation is the
last stmt within its basic block, so the patch also fixes these issues.
gcc/analyzer/ChangeLog:
PR analyzer/104560
* diagnostic-manager.cc (diagnostic_manager::build_emission_path):
Add region creation events for globals of interest.
(null_assignment_sm_context::get_old_program_state): New.
(diagnostic_manager::add_events_for_eedge): Move check for
changing dynamic extents from PK_BEFORE_STMT case to after the
switch on the dst_point's kind so that we can emit them for the
final stmt in a basic block.
* engine.cc (impl_sm_context::get_old_program_state): New.
* sm-malloc.cc (malloc_state_machine::get_default_state): Rewrite
detection of m_non_heap to use get_memory_space.
(free_of_non_heap::free_of_non_heap): Add freed_reg param.
(free_of_non_heap::subclass_equal_p): Update for changes to
fields.
(free_of_non_heap::emit): Drop m_kind in favor of
get_memory_space.
(free_of_non_heap::describe_state_change): Remove logic for
detecting alloca.
(free_of_non_heap::mark_interesting_stuff): Add region-creation of
m_freed_reg.
(free_of_non_heap::get_memory_space): New.
(free_of_non_heap::kind): Drop enum.
(free_of_non_heap::m_freed_reg): New field.
(free_of_non_heap::m_kind): Drop field.
(malloc_state_machine::on_stmt): Drop transition to m_non_heap.
(malloc_state_machine::handle_free_of_non_heap): New function,
split out from on_deallocator_call and on_realloc_call, adding
detection of the freed region.
(malloc_state_machine::on_deallocator_call): Use it.
(malloc_state_machine::on_realloc_call): Likewise.
* sm.h (sm_context::get_old_program_state): New vfunc.
gcc/testsuite/ChangeLog:
PR analyzer/104560
* g++.dg/analyzer/placement-new.C: Update expected wording.
* g++.dg/analyzer/pr100244.C: Likewise.
* gcc.dg/analyzer/attr-malloc-1.c (test_7): Likewise.
* gcc.dg/analyzer/malloc-1.c (test_24): Likewise.
(test_25): Likewise.
(test_26): Likewise.
(test_50a, test_50b, test_50c): New.
* gcc.dg/analyzer/malloc-callbacks.c (test_5): Update expected
wording.
* gcc.dg/analyzer/malloc-paths-8.c: Likewise.
* gcc.dg/analyzer/pr104560-1.c: New test.
* gcc.dg/analyzer/pr104560-2.c: New test.
* gcc.dg/analyzer/realloc-1.c (test_7): Updated expected wording.
* gcc.dg/analyzer/vla-1.c (test_2): New. Prune output from
-Wfree-nonheap-object.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/104524
* region-model-manager.cc
(region_model_manager::maybe_fold_sub_svalue): Only call
get_or_create_cast if type is non-NULL.
gcc/testsuite/ChangeLog:
PR analyzer/104524
* gcc.dg/analyzer/pr104524.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
There is false positive from -Wanalyzer-use-of-uninitialized-value on
gcc.dg/analyzer/pr102692.c here:
‘fix_overlays_before’: events 1-3
|
| 75 | while (tail
| | ~~~~
| 76 | && (tem = make_lisp_ptr (tail, 5),
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (1) following ‘false’ branch (when ‘tail’ is NULL)...
| 77 | (end = marker_position (XOVERLAY (tem)->end)) >= pos))
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|......
| 82 | if (!tail || end < prev || !tail->next)
| | ~~~~~ ~~~~~~~~~~
| | | |
| | | (3) use of uninitialized value ‘end’ here
| | (2) ...to here
|
The issue is that inner || of the conditionals have been folded within the
frontend from a chain of control flow:
5 │ if (tail == 0B) goto <D.1986>; else goto <D.1988>;
6 │ <D.1988>:
7 │ if (end < prev) goto <D.1986>; else goto <D.1989>;
8 │ <D.1989>:
9 │ _1 = tail->next;
10 │ if (_1 == 0B) goto <D.1986>; else goto <D.1987>;
11 │ <D.1986>:
to an OR expr (and then to a bitwise-or by the gimplifier):
5 │ _1 = tail == 0B;
6 │ _2 = end < prev;
7 │ _3 = _1 | _2;
8 │ if (_3 != 0) goto <D.1986>; else goto <D.1988>;
9 │ <D.1988>:
10 │ _4 = tail->next;
11 │ if (_4 == 0B) goto <D.1986>; else goto <D.1987>;
This happens for sufficiently simple conditionals in fold_truth_andor.
In particular, the (end < prev) is short-circuited without optimization,
but is evaluated with optimization, leading to the false positive.
Given how early this folding occurs, it seems the simplest fix is to
try to detect places where this optimization appears to have happened,
and suppress uninit warnings within the statement that would have
been short-circuited.
gcc/analyzer/ChangeLog:
PR analyzer/102692
* exploded-graph.h (impl_region_model_context::get_stmt): New.
* region-model.cc: Include "gimple-ssa.h", "tree-phinodes.h",
"tree-ssa-operands.h", and "ssa-iterators.h".
(within_short_circuited_stmt_p): New.
(region_model::check_for_poison): Don't warn about uninit values
if within_short_circuited_stmt_p.
* region-model.h (region_model_context::get_stmt): New vfunc.
(noop_region_model_context::get_stmt): New.
gcc/testsuite/ChangeLog:
PR analyzer/102692
* gcc.dg/analyzer/pr102692-2.c: New test.
* gcc.dg/analyzer/pr102692.c: Remove xfail. Remove -O2 from
options and move to...
* gcc.dg/analyzer/torture/pr102692.c: ...here.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/104274 reports a false positive from
-Wanalyzer-use-of-uninitialized-value on hppa when passing
an empty struct as a function parameter.
pa_pass_by_reference returns true for empty structs, so the
call is turned into:
struct empty arg.0;
arg.0 = arg
called_function (arg.0);
by gimplify_parameters.
However, gimplify_modify_expr discards assignments statments
of empty types, so that we end up with:
struct empty arg.0;
called_function (arg.0);
which the analyzer considers to be a use of uninitialized "arg.0";
Given that gimplify_modify_expr will discard any assignments to
such types, it seems simplest for -Wanalyzer-use-of-uninitialized-value
to ignore values of empty types.
gcc/analyzer/ChangeLog:
PR analyzer/104274
* region-model.cc (region_model::check_for_poison): Ignore
uninitialized uses of empty types.
gcc/testsuite/ChangeLog:
PR analyzer/104274
* gcc.dg/analyzer/torture/empty-struct-1.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/98797
* region-model-manager.cc
(region_model_manager::maybe_fold_sub_svalue): Generalize getting
individual chars of a STRING_CST from element_region to any
subregion which is a concrete access of a single byte from its
parent region.
* region.cc (region::get_relative_concrete_byte_range): New.
* region.h (region::get_relative_concrete_byte_range): New decl.
gcc/testsuite/ChangeLog:
PR analyzer/98797
* gcc.dg/analyzer/casts-1.c: Mark xfails as fixed; add further
test coverage for casts of string literals.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/104452
* region-model.cc (selftest::test_bit_range_regions): New.
(selftest::analyzer_region_model_cc_tests): Call it.
* region.h (bit_range_region::key_t::hash): Fix hashing of m_bits
to avoid using uninitialized data.
gcc/testsuite/ChangeLog:
PR analyzer/104452
* gcc.dg/analyzer/pr104452.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
gcc/analyzer/ChangeLog:
PR analyzer/104417
* sm-taint.cc (tainted_allocation_size::tainted_allocation_size):
Remove overzealous assertion.
(tainted_allocation_size::emit): Likewise.
(region_model::check_dynamic_size_for_taint): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/104417
* gcc.dg/analyzer/pr104417.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
PR analyzer/103872 reports a failure of gcc.dg/analyzer/pr103526.c on
riscv64-unknown-elf-gcc. The issue is that I wrote the test on x86_64
where a memcpy in the test is optimized to a write to a read/write pair,
whereas due to alignment differences the analyzer can see it as a
memcpy call, revealing problems with the analyzer's implementation
of memcpy.
This patch reimplements region_model::impl_call_memcpy in terms of a
get_store_value followed by a set_value, fixing the issue.
gcc/analyzer/ChangeLog:
PR analyzer/103872
* region-model-impl-calls.cc (region_model::impl_call_memcpy):
Reimplement in terms of a get_store_value followed by a set_value.
gcc/testsuite/ChangeLog:
PR analyzer/103872
* gcc.dg/analyzer/memcpy-1.c: Add alternate versions of test cases
in which the calls to memcpy are hidden from the optimizer. Add
further test cases.
* gcc.dg/analyzer/taint-size-1.c: Add test coverage for memcpy
with tainted size.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
This patch fixes various issues with how -fanalyzer handles "realloc"
seen when debugging PR analyzer/104369.
Previously it wasn't correctly copying over the contents of the old
buffer for the success-with-move case, leading to false
-Wanalyzer-use-of-uninitialized-value diagnostics.
I also noticed that -fanalyzer failed to properly handle "realloc" for
cases where the ptr's region had unknown dynamic extents, and an ICE
for the case where a tainted value is used as a realloc size argument.
This patch fixes these issues, including the false uninit diagnostics
seen in PR analyzer/104369.
gcc/analyzer/ChangeLog:
PR analyzer/104369
* engine.cc (exploded_graph::process_node): Use the node for any
diagnostics, avoiding ICE if a bifurcation update adds a
saved_diagnostic, such as for a tainted realloc size.
* region-model-impl-calls.cc
(region_model::impl_call_realloc::success_no_move::update_model):
Require the old pointer to be non-NULL to be able successfully
grow in place. Use model->deref_rvalue rather than maybe_get_region
to support the old pointer being symbolic.
(region_model::impl_call_realloc::success_with_move::update_model):
Likewise. Add a constraint that the new pointer != the old pointer.
Use a sized_region when setting the value of the new region.
Handle the case where we don't know the dynamic size of the old
region by marking the new region as unknown.
* sm-taint.cc (tainted_allocation_size::tainted_allocation_size):
Update assertion to also allow for MEMSPACE_UNKNOWN.
(tainted_allocation_size::emit): Likewise.
(region_model::check_dynamic_size_for_taint): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/104369
* gcc.dg/analyzer/pr104369-1.c: New test.
* gcc.dg/analyzer/pr104369-2.c: New test.
* gcc.dg/analyzer/realloc-3.c: New test.
* gcc.dg/analyzer/realloc-4.c: New test.
* gcc.dg/analyzer/taint-realloc.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
It turned out that the analyzer wasn't treating calloc regions
as zero-filled, due to binding_cluster::fill_region getting an
unknown value for the byte_size_size_sval, and thus
get_or_create_repeated_svalue returning an unknown_svalue, which
was then used to fill the region.
Fixed thusly.
gcc/analyzer/ChangeLog:
* region-model-impl-calls.cc (region_model::impl_call_calloc): Use
a sized_region when calling zero_fill_region.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/calloc-1.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
When moving the -fanalyzer tests for -ftrivial-auto-var-init to the
"torture" subdirectory of gcc.dg/analyzer I noticed that -fanalyzer
wasn't always properly checking for initialization of return values.
The issue was that some "return" handling was using
region_model::copy_region to copy to the RESULT_DECL, and copy_region
wasn't checking for poisoned svalues.
This patch eliminates region_model::copy_region in favor of simply
doing a get_ravlue/set_value pair, fixing the issue.
gcc/analyzer/ChangeLog:
* region-model.cc (region_model::on_return): Replace usage of
copy_region with get_rvalue/set_value pair.
(region_model::pop_frame): Likewise.
(selftest::test_compound_assignment): Likewise.
* region-model.h (region_model::copy_region): Delete decl.
* region.cc (region_model::copy_region): Delete.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/torture/ubsan-1.c: Add missing return stmts.
* gcc.dg/analyzer/uninit-trivial-auto-var-init-pattern.c: Move
to...
* gcc.dg/analyzer/torture/uninit-trivial-auto-var-init-pattern.c:
...here.
* gcc.dg/analyzer/uninit-trivial-auto-var-init-uninitialized.c:
Move to...
* gcc.dg/analyzer/torture/uninit-trivial-auto-var-init-uninitialized.c:
...here.
* gcc.dg/analyzer/uninit-trivial-auto-var-init-zero.c: Move to...
* gcc.dg/analyzer/torture/uninit-trivial-auto-var-init-zero.c: ...here.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
gcc/analyzer/ChangeLog:
* region.cc (region::calc_offset): Consolidate effectively
identical cases.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
GCC 12 has gained -Wanalyzer-use-of-uninitialized-value, and I'm
seeing various false positives from it due to region_model::get_lvalue
not properly handling BIT_FIELD_REF, and falling back to
using an UNKNOWN_REGION for them.
This patch fixes these false positives by implementing a new
bit_range_region region subclass for handling BIT_FIELD_REF.
gcc/analyzer/ChangeLog:
* analyzer.h (class bit_range_region): New forward decl.
* region-model-manager.cc (region_model_manager::get_bit_range):
New.
(region_model_manager::log_stats): Handle m_bit_range_regions.
* region-model.cc (region_model::get_lvalue_1): Handle
BIT_FIELD_REF.
* region-model.h (region_model_manager::get_bit_range): New decl.
(region_model_manager::m_bit_range_regions): New field.
* region.cc (region::get_base_region): Handle RK_BIT_RANGE.
(region::base_region_p): Likewise.
(region::calc_offset): Likewise.
(bit_range_region::dump_to_pp): New.
(bit_range_region::get_byte_size): New.
(bit_range_region::get_bit_size): New.
(bit_range_region::get_byte_size_sval): New.
(bit_range_region::get_relative_concrete_offset): New.
* region.h (enum region_kind): Add RK_BIT_RANGE.
(region::dyn_cast_bit_range_region): New vfunc.
(class bit_range_region): New.
(is_a_helper <const bit_range_region *>::test): New.
(default_hash_traits<bit_range_region::key_t>): New.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/torture/uninit-bit-field-ref.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
[PR104270]
GCC 12 has gained two features for dealing with uninitialized variables:
(a) a new -Wanalyzer-use-of-uninitialized-value warning within -fanalyzer
for interprocedural path-sensitive detection of ununit uses, and
(b) a new -ftrivial-auto-var-init option for mitigating some uses of
uninit variables
It turns out that using (b) was thwarting (a), as it led to -fanalyzer
seeing calls to IFN_DEFERRED_INIT, which -fanalyzer wasn't
special-casing, thus treating it as initializing the variables in
question, and thus silencing -Wanalyzer-use-of-uninitialized-value on
them.
invoke.texi says:
"GCC still considers an automatic variable that doesn't have an explicit
initializer as uninitialized, @option{-Wuninitialized} will still report
warning messages on such automatic variables."
and thus -Wanalyzer-use-of-uninitialized-value ought to as well.
This patch adds special-case handling to -fanalyzer for
IFN_DEFERRED_INIT, so that -fanalyzer will warn on uninit uses of
variables that are mitigated by -ftrivial-auto-var-init.
gcc/analyzer/ChangeLog:
PR analyzer/104270
* region-model.cc (region_model::on_call_pre): Handle
IFN_DEFERRED_INIT.
gcc/testsuite/ChangeLog:
PR analyzer/104270
* gcc.dg/analyzer/uninit-trivial-auto-var-init-pattern.c: New
test.
* gcc.dg/analyzer/uninit-trivial-auto-var-init-uninitialized.c:
New test.
* gcc.dg/analyzer/uninit-trivial-auto-var-init-zero.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
When reviewing the output of -fanalyzer on PR analyzer/104224 I noticed
that despite very verbose paths, the diagnostic paths for
-Wanalyzer-use-of-uninitialized-value
don't show where the uninitialized memory is allocated.
This patch adapts and simplifies material from
"[PATCH 3/6] analyzer: implement infoleak detection"
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584377.html
in order to add region creation events for the pertinent region (whether
on the stack or heap).
For example, this patch extends:
malloc-1.c: In function 'test_40':
malloc-1.c:461:5: warning: use of uninitialized value '*p' [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
461 | i = *p;
| ~~^~~~
'test_40': event 1
|
| 461 | i = *p;
| | ~~^~~~
| | |
| | (1) use of uninitialized value '*p' here
|
to:
malloc-1.c: In function 'test_40':
malloc-1.c:461:5: warning: use of uninitialized value '*p' [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
461 | i = *p;
| ~~^~~~
'test_40': events 1-2
|
| 460 | int *p = (int*)malloc(sizeof(int*));
| | ^~~~~~~~~~~~~~~~~~~~
| | |
| | (1) region created on heap here
| 461 | i = *p;
| | ~~~~~~
| | |
| | (2) use of uninitialized value '*p' here
|
and this helps readability of the resulting warnings, especially in
more complicated cases.
gcc/analyzer/ChangeLog:
* checker-path.cc (event_kind_to_string): Handle
EK_REGION_CREATION.
(region_creation_event::region_creation_event): New.
(region_creation_event::get_desc): New.
(checker_path::add_region_creation_event): New.
* checker-path.h (enum event_kind): Add EK_REGION_CREATION.
(class region_creation_event): New subclass.
(checker_path::add_region_creation_event): New decl.
* diagnostic-manager.cc
(diagnostic_manager::emit_saved_diagnostic): Pass NULL for new
param to add_events_for_eedge when handling trailing eedge.
(diagnostic_manager::build_emission_path): Create an interesting_t
instance, allow the pending diagnostic to populate it, and pass it
to the calls to add_events_for_eedge.
(diagnostic_manager::add_events_for_eedge): Add "interest" param.
Use it to add region_creation_events for on-stack regions created
within at function entry, and when pertinent dynamically-sized
regions are created.
(diagnostic_manager::prune_for_sm_diagnostic): Add case for
EK_REGION_CREATION.
* diagnostic-manager.h (diagnostic_manager::add_events_for_eedge):
Add "interest" param.
* pending-diagnostic.cc: Include "selftest.h", "tristate.h",
"analyzer/call-string.h", "analyzer/program-point.h",
"analyzer/store.h", and "analyzer/region-model.h".
(interesting_t::add_region_creation): New.
(interesting_t::dump_to_pp): New.
* pending-diagnostic.h (struct interesting_t): New.
(pending_diagnostic::mark_interesting_stuff): New vfunc.
* region-model.cc
(poisoned_value_diagnostic::poisoned_value_diagnostic): Add
(poisoned_value_diagnostic::operator==): Compare m_pkind and
m_src_region fields.
(poisoned_value_diagnostic::mark_interesting_stuff): New.
(poisoned_value_diagnostic::m_src_region): New.
(region_model::check_for_poison): Call
get_region_for_poisoned_expr for uninit values and pass the resul
to the diagnostic.
(region_model::get_region_for_poisoned_expr): New.
(region_model::deref_rvalue): Pass NULL for
poisoned_value_diagnostic's src_region.
* region-model.h (region_model::get_region_for_poisoned_expr): New
decl.
* region.h (frame_region::get_fndecl): New.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/data-model-1.c: Add dg-message directives for
expected region creation events.
* gcc.dg/analyzer/malloc-1.c: Likewise.
* gcc.dg/analyzer/memset-CVE-2017-18549-1.c: Likewise.
* gcc.dg/analyzer/pr101547.c: Likewise.
* gcc.dg/analyzer/pr101875.c: Likewise.
* gcc.dg/analyzer/pr101962.c: Likewise.
* gcc.dg/analyzer/pr104224.c: Likewise.
* gcc.dg/analyzer/pr94047.c: Likewise.
* gcc.dg/analyzer/symbolic-1.c: Likewise.
* gcc.dg/analyzer/uninit-1.c: Likewise.
* gcc.dg/analyzer/uninit-4.c: Likewise.
* gcc.dg/analyzer/uninit-alloca.c: New test.
* gcc.dg/analyzer/uninit-pr94713.c: Add dg-message directive for
expected region creation event.
* gcc.dg/analyzer/uninit-pr94714.c: Likewise.
* gcc.dg/analyzer/zlib-3.c: Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
PR analyzer/104247
gcc/analyzer/ChangeLog:
* constraint-manager.cc (bounded_ranges_manager::log_stats):
Cast to long for format purpose.
* region-model-manager.cc (log_uniq_map): Likewise.
|
|
|
|
We were failing to check for uninitialized arguments to stdio builtins,
such as when passing local "go" to the call to "printf" in "main" in
the testcase.
gcc/analyzer/ChangeLog:
PR analyzer/104224
* region-model.cc (region_model::check_call_args): New.
(region_model::on_call_pre): Call it when ignoring stdio builtins.
* region-model.h (region_model::check_call_args): New decl
gcc/testsuite/ChangeLog:
PR analyzer/104224
* gcc.dg/analyzer/pr104224.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
Mikael Morin spotted that I got the sense wrong when discarding
redundant constraints in
r12-6782-gc4b8f3730a80025192fdb485ad2535c165340e41.
Fixed as follows, which also moves the rejection of contradictory
constraints in range::add_bound to earlier, so that this code can
be self-tested.
gcc/analyzer/ChangeLog:
PR analyzer/94362
* constraint-manager.cc (range::add_bound): Fix tests for
discarding redundant constraints. Perform test for rejecting
unsatisfiable constraints earlier so that they don't update
the object on failure.
(selftest::test_range): New.
(selftest::test_constant_comparisons): Add test coverage for
existing constraints becoming narrower until they are
unsatisfiable.
(selftest::run_constraint_manager_tests): Call test_range.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/104159 describes an ICE attempting to convert a vector_cst,
which occurs when symbolically executing within a recursive call on:
_4 = BIT_FIELD_REF <w_3(D), 32, 0>;
_1 = VIEW_CONVERT_EXPR<T>(_4);
where the BIT_FIELD_REF leads to a get_or_create_cast from
VEC<long, 8> to VEC<unsigned 4>
which get_code_for_cast erroneously picks NOP_EXPR for the cast, leading
to a bogus input to the VIEW_CONVERT_EXPR.
This patch fixes the issue by giving up on attempts to cast symbolic
values of vector types, treating the result of such casts as unknowable.
gcc/analyzer/ChangeLog:
PR analyzer/104159
* region-model-manager.cc
(region_model_manager::get_or_create_cast): Bail out if the types
are the same. Don't attempt to handle casts involving vector
types.
gcc/testsuite/ChangeLog:
PR analyzer/104159
* gcc.dg/analyzer/torture/pr104159.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
|
PR analyzer/94362 reports a false positive from
-Wanalyzer-null-dereference seen when analyzing OpenSSL.
The root cause is that the analyzer's path feasibility checker
erroneously considers this to be feasible:
(R + 1 > 0) && (R < 0)
for int R (the return value from sk_EVP_PKEY_ASN1_METHOD_num),
whereas it's not satisfiable for any int R.
This patch makes the constraint manager try harder to reject
such combinations of conditions, fixing the false positive;
perhaps in the longer term we ought to use an SMT solver.
gcc/analyzer/ChangeLog:
PR analyzer/94362
* constraint-manager.cc (bound::ensure_closed): Convert param to
enum bound_kind.
(range::constrained_to_single_element): Likewise.
(range::add_bound): New.
(constraint_manager::add_constraint): Handle SVAL + OFFSET
compared to a constant.
(constraint_manager::get_ec_bounds): Rewrite in terms of
range::add_bound.
(constraint_manager::eval_condition): Reject if range::add_bound
fails.
(selftest::test_constant_comparisons): Add test coverage for
various impossible combinations of integer comparisons.
* constraint-manager.h (enum bound_kind): New.
(struct bound): Likewise.
(bound::ensure_closed): Convert to param to enum bound_kind.
(struct range): Convert to...
(class range): ...this, making fields private.
(range::add_bound): New decls.
* region-model.cc (region_model::add_constraint): Fail if
constraint_manager::add_constraint fails.
gcc/testsuite/ChangeLog:
PR analyzer/94362
* gcc.dg/analyzer/pr94362-1.c: New test.
* gcc.dg/analyzer/pr94362-2.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
|
|
|
