summaryrefslogtreecommitdiff
path: root/gcc/analyzer/diagnostic-manager.cc
diff options
context:
space:
mode:
authorDavid Malcolm <dmalcolm@redhat.com>2022-02-16 09:06:46 -0500
committerDavid Malcolm <dmalcolm@redhat.com>2022-02-16 18:48:30 -0500
commita61aaee63848d422e8443e17bbec3257ee59d5d8 (patch)
tree2b0b4a63040799e86211bd329f4c93b2961a1555 /gcc/analyzer/diagnostic-manager.cc
parent24ca97325cab7bc454c785d55f37120fe7ea6f74 (diff)
analyzer: fixes to free of non-heap detection [PR104560]
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>
Diffstat (limited to 'gcc/analyzer/diagnostic-manager.cc')
-rw-r--r--gcc/analyzer/diagnostic-manager.cc105
1 files changed, 70 insertions, 35 deletions
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 80bca6a0738..680016e94bc 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1272,6 +1272,35 @@ diagnostic_manager::build_emission_path (const path_builder &pb,
interesting_t interest;
pb.get_pending_diagnostic ()->mark_interesting_stuff (&interest);
+
+ /* Add region creation events for any globals of interest, at the
+ beginning of the path. */
+ {
+ for (auto reg : interest.m_region_creation)
+ switch (reg->get_memory_space ())
+ {
+ default:
+ continue;
+ case MEMSPACE_CODE:
+ case MEMSPACE_GLOBALS:
+ case MEMSPACE_READONLY_DATA:
+ {
+ const region *base_reg = reg->get_base_region ();
+ if (tree decl = base_reg->maybe_get_decl ())
+ if (DECL_P (decl)
+ && DECL_SOURCE_LOCATION (decl) != UNKNOWN_LOCATION)
+ {
+ emission_path->add_region_creation_event
+ (reg,
+ DECL_SOURCE_LOCATION (decl),
+ NULL_TREE,
+ 0);
+ }
+ }
+ }
+ }
+
+ /* Walk EPATH, adding events as appropriate. */
for (unsigned i = 0; i < epath.m_edges.length (); i++)
{
const exploded_edge *eedge = epath.m_edges[i];
@@ -1569,6 +1598,11 @@ struct null_assignment_sm_context : public sm_context
return NULL_TREE;
}
+ const program_state *get_old_program_state () const FINAL OVERRIDE
+ {
+ return m_old_state;
+ }
+
const program_state *m_old_state;
const program_state *m_new_state;
const gimple *m_stmt;
@@ -1729,46 +1763,47 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb,
break;
}
- /* Look for changes in dynamic extents, which will identify
- the creation of heap-based regions and alloca regions. */
- if (interest)
- {
- const region_model *src_model = src_state.m_region_model;
- const region_model *dst_model = dst_state.m_region_model;
- if (src_model->get_dynamic_extents ()
- != dst_model->get_dynamic_extents ())
- {
- unsigned i;
- const region *reg;
- FOR_EACH_VEC_ELT (interest->m_region_creation, i, reg)
- {
- const region *base_reg = reg->get_base_region ();
- const svalue *old_extents
- = src_model->get_dynamic_extents (base_reg);
- const svalue *new_extents
- = dst_model->get_dynamic_extents (base_reg);
- if (old_extents == NULL && new_extents != NULL)
- switch (base_reg->get_kind ())
- {
- default:
- break;
- case RK_HEAP_ALLOCATED:
- case RK_ALLOCA:
- emission_path->add_region_creation_event
- (reg,
- src_point.get_location (),
- src_point.get_fndecl (),
- src_stack_depth);
- break;
- }
- }
- }
- }
}
}
break;
}
+ /* Look for changes in dynamic extents, which will identify
+ the creation of heap-based regions and alloca regions. */
+ if (interest)
+ {
+ const region_model *src_model = src_state.m_region_model;
+ const region_model *dst_model = dst_state.m_region_model;
+ if (src_model->get_dynamic_extents ()
+ != dst_model->get_dynamic_extents ())
+ {
+ unsigned i;
+ const region *reg;
+ FOR_EACH_VEC_ELT (interest->m_region_creation, i, reg)
+ {
+ const region *base_reg = reg->get_base_region ();
+ const svalue *old_extents
+ = src_model->get_dynamic_extents (base_reg);
+ const svalue *new_extents
+ = dst_model->get_dynamic_extents (base_reg);
+ if (old_extents == NULL && new_extents != NULL)
+ switch (base_reg->get_kind ())
+ {
+ default:
+ break;
+ case RK_HEAP_ALLOCATED:
+ case RK_ALLOCA:
+ emission_path->add_region_creation_event
+ (reg,
+ src_point.get_location (),
+ src_point.get_fndecl (),
+ src_stack_depth);
+ break;
+ }
+ }
+ }
+ }
+
if (pb.get_feasibility_problem ()
&& &pb.get_feasibility_problem ()->m_eedge == &eedge)
{