From 25ef215abb1aa701db7ab173b9f2ac653cecf634 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 28 Aug 2020 10:10:38 -0400 Subject: [PATCH] analyzer: eliminate sm_context::warn_for_state in favor of a new 'warn' vfunc This patch is yet more preliminary work towards generalizing sm-malloc.cc beyond just malloc/free. It eliminates sm_context::warn_for_state in terms of a new sm_context::warn vfunc, guarded by sm_context::get_state calls. gcc/analyzer/ChangeLog: * diagnostic-manager.cc (null_assignment_sm_context::warn_for_state): Replace with... (null_assignment_sm_context::warn): ...this. * engine.cc (impl_sm_context::warn_for_state): Replace with... (impl_sm_context::warn): ...this. * sm-file.cc (fileptr_state_machine::on_stmt): Replace warn_for_state and on_transition calls with a get_state test guarding warn and set_next_state calls. * sm-malloc.cc (malloc_state_machine::on_stmt): Likewise. * sm-pattern-test.cc (pattern_test_state_machine::on_condition): Replace warn_for_state call with warn call. * sm-sensitive.cc (sensitive_state_machine::warn_for_any_exposure): Replace warn_for_state call with a get_state test guarding a warn call. * sm-signal.cc (signal_state_machine::on_stmt): Likewise. * sm-taint.cc (taint_state_machine::on_stmt): Replace warn_for_state and on_transition calls with a get_state test guarding warn and set_next_state calls. * sm.h (sm_context::warn_for_state): Replace with... (sm_context::warn): ...this. --- gcc/analyzer/diagnostic-manager.cc | 5 +- gcc/analyzer/engine.cc | 34 +++++-------- gcc/analyzer/sm-file.cc | 9 ++-- gcc/analyzer/sm-malloc.cc | 77 +++++++++++++++++++----------- gcc/analyzer/sm-pattern-test.cc | 2 +- gcc/analyzer/sm-sensitive.cc | 5 +- gcc/analyzer/sm-signal.cc | 7 +-- gcc/analyzer/sm-taint.cc | 49 +++++++++++-------- gcc/analyzer/sm.h | 8 ++-- 9 files changed, 108 insertions(+), 88 deletions(-) diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 6fd15c21962..4a95d4c569e 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -808,9 +808,8 @@ struct null_assignment_sm_context : public sm_context *m_new_state)); } - void warn_for_state (const supernode *, const gimple *, - tree, state_machine::state_t, - pending_diagnostic *d) FINAL OVERRIDE + void warn (const supernode *, const gimple *, + tree, pending_diagnostic *d) FINAL OVERRIDE { delete d; } diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 07b1b15d195..49701b74fd4 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -254,35 +254,23 @@ public: to, origin_new_sval, m_eg.get_ext_state ()); } - void warn_for_state (const supernode *snode, const gimple *stmt, - tree var, state_machine::state_t state, - pending_diagnostic *d) FINAL OVERRIDE + void warn (const supernode *snode, const gimple *stmt, + tree var, pending_diagnostic *d) FINAL OVERRIDE { LOG_FUNC (get_logger ()); gcc_assert (d); // take ownership - impl_region_model_context old_ctxt (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL); - state_machine::state_t current; - if (var) - { - const svalue *var_old_sval - = m_old_state->m_region_model->get_rvalue (var, &old_ctxt); - current = m_old_smap->get_state (var_old_sval, m_eg.get_ext_state ()); - } - else - current = m_old_smap->get_global_state (); - if (state == current) - { - const svalue *var_old_sval - = m_old_state->m_region_model->get_rvalue (var, &old_ctxt); - m_eg.get_diagnostic_manager ().add_diagnostic - (&m_sm, m_enode_for_diag, snode, stmt, m_stmt_finder, - var, var_old_sval, state, d); - } - else - delete d; + const svalue *var_old_sval + = m_old_state->m_region_model->get_rvalue (var, &old_ctxt); + state_machine::state_t current + = (var + ? m_old_smap->get_state (var_old_sval, m_eg.get_ext_state ()) + : m_old_smap->get_global_state ()); + m_eg.get_diagnostic_manager ().add_diagnostic + (&m_sm, m_enode_for_diag, snode, stmt, m_stmt_finder, + var, var_old_sval, current, d); } /* Hook for picking more readable trees for SSA names of temporaries, diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc index 33b445195d5..58a0fd461fa 100644 --- a/gcc/analyzer/sm-file.cc +++ b/gcc/analyzer/sm-file.cc @@ -344,9 +344,12 @@ fileptr_state_machine::on_stmt (sm_context *sm_ctxt, sm_ctxt->on_transition (node, stmt , arg, m_nonnull, m_closed); - sm_ctxt->warn_for_state (node, stmt, arg, m_closed, - new double_fclose (*this, diag_arg)); - sm_ctxt->on_transition (node, stmt, arg, m_closed, m_stop); + if (sm_ctxt->get_state (stmt, arg) == m_closed) + { + sm_ctxt->warn (node, stmt, arg, + new double_fclose (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } return true; } diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index 19afff49519..2f7db924b94 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -716,14 +716,20 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, we don't want to complain about double-free of NULL. */ /* freed -> stop, with warning. */ - sm_ctxt->warn_for_state (node, stmt, arg, m_freed, - new double_free (*this, diag_arg)); - sm_ctxt->on_transition (node, stmt, arg, m_freed, m_stop); + if (sm_ctxt->get_state (stmt, arg) == m_freed) + { + sm_ctxt->warn (node, stmt, arg, + new double_free (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } /* non-heap -> stop, with warning. */ - sm_ctxt->warn_for_state (node, stmt, arg, m_non_heap, - new free_of_non_heap (*this, diag_arg)); - sm_ctxt->on_transition (node, stmt, arg, m_non_heap, m_stop); + if (sm_ctxt->get_state (stmt, arg) == m_non_heap) + { + sm_ctxt->warn (node, stmt, arg, + new free_of_non_heap (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } return true; } @@ -744,17 +750,23 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, || bitmap_bit_p (nonnull_args, i)) { tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); - sm_ctxt->warn_for_state - (node, stmt, arg, m_unchecked, - new possible_null_arg (*this, diag_arg, callee_fndecl, - i)); - sm_ctxt->on_transition (node, stmt, arg, m_unchecked, - m_nonnull); - - sm_ctxt->warn_for_state - (node, stmt, arg, m_null, - new null_arg (*this, diag_arg, callee_fndecl, i)); - sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop); + state_t state = sm_ctxt->get_state (stmt, arg); + /* Can't use a switch as the states are non-const. */ + if (state == m_unchecked) + { + sm_ctxt->warn (node, stmt, arg, + new possible_null_arg (*this, diag_arg, + callee_fndecl, + i)); + sm_ctxt->set_next_state (stmt, arg, m_nonnull); + } + else if (state == m_null) + { + sm_ctxt->warn (node, stmt, arg, + new null_arg (*this, diag_arg, + callee_fndecl, i)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } } } BITMAP_FREE (nonnull_args); @@ -800,17 +812,26 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, tree arg = TREE_OPERAND (op, 0); tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); - sm_ctxt->warn_for_state (node, stmt, arg, m_unchecked, - new possible_null_deref (*this, diag_arg)); - sm_ctxt->on_transition (node, stmt, arg, m_unchecked, m_nonnull); - - sm_ctxt->warn_for_state (node, stmt, arg, m_null, - new null_deref (*this, diag_arg)); - sm_ctxt->on_transition (node, stmt, arg, m_null, m_stop); - - sm_ctxt->warn_for_state (node, stmt, arg, m_freed, - new use_after_free (*this, diag_arg)); - sm_ctxt->on_transition (node, stmt, arg, m_freed, m_stop); + state_t state = sm_ctxt->get_state (stmt, arg); + /* Can't use a switch as the states are non-const. */ + if (state == m_unchecked) + { + sm_ctxt->warn (node, stmt, arg, + new possible_null_deref (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_nonnull); + } + else if (state == m_null) + { + sm_ctxt->warn (node, stmt, arg, + new null_deref (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } + else if (state == m_freed) + { + sm_ctxt->warn (node, stmt, arg, + new use_after_free (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } } } return false; diff --git a/gcc/analyzer/sm-pattern-test.cc b/gcc/analyzer/sm-pattern-test.cc index 6a59e8fff83..bb6d3b1e719 100644 --- a/gcc/analyzer/sm-pattern-test.cc +++ b/gcc/analyzer/sm-pattern-test.cc @@ -128,7 +128,7 @@ pattern_test_state_machine::on_condition (sm_context *sm_ctxt, return; pending_diagnostic *diag = new pattern_match (lhs, op, rhs); - sm_ctxt->warn_for_state (node, stmt, lhs, m_start, diag); + sm_ctxt->warn (node, stmt, lhs, diag); } bool diff --git a/gcc/analyzer/sm-sensitive.cc b/gcc/analyzer/sm-sensitive.cc index f10008307af..49f9eb387b1 100644 --- a/gcc/analyzer/sm-sensitive.cc +++ b/gcc/analyzer/sm-sensitive.cc @@ -174,8 +174,9 @@ sensitive_state_machine::warn_for_any_exposure (sm_context *sm_ctxt, tree arg) const { tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); - sm_ctxt->warn_for_state (node, stmt, arg, m_sensitive, - new exposure_through_output_file (*this, diag_arg)); + if (sm_ctxt->get_state (stmt, arg) == m_sensitive) + sm_ctxt->warn (node, stmt, arg, + new exposure_through_output_file (*this, diag_arg)); } /* Implementation of state_machine::on_stmt vfunc for diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc index 21c9d58f6de..bf6ea480423 100644 --- a/gcc/analyzer/sm-signal.cc +++ b/gcc/analyzer/sm-signal.cc @@ -346,9 +346,10 @@ signal_state_machine::on_stmt (sm_context *sm_ctxt, if (const gcall *call = dyn_cast (stmt)) if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call)) if (signal_unsafe_p (callee_fndecl)) - sm_ctxt->warn_for_state (node, stmt, NULL_TREE, m_in_signal_handler, - new signal_unsafe_call (*this, call, - callee_fndecl)); + if (sm_ctxt->get_global_state () == m_in_signal_handler) + sm_ctxt->warn (node, stmt, NULL_TREE, + new signal_unsafe_call (*this, call, + callee_fndecl)); } return false; diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc index 385909ce26c..49bbd6dfb13 100644 --- a/gcc/analyzer/sm-taint.cc +++ b/gcc/analyzer/sm-taint.cc @@ -233,27 +233,36 @@ taint_state_machine::on_stmt (sm_context *sm_ctxt, if (INTEGRAL_TYPE_P (TREE_TYPE (arg))) is_unsigned = TYPE_UNSIGNED (TREE_TYPE (arg)); - /* Complain about missing bounds. */ - sm_ctxt->warn_for_state - (node, stmt, arg, m_tainted, - new tainted_array_index (*this, diag_arg, - is_unsigned - ? BOUNDS_LOWER : BOUNDS_NONE)); - sm_ctxt->on_transition (node, stmt, arg, m_tainted, m_stop); - - /* Complain about missing upper bound. */ - sm_ctxt->warn_for_state (node, stmt, arg, m_has_lb, - new tainted_array_index (*this, diag_arg, - BOUNDS_LOWER)); - sm_ctxt->on_transition (node, stmt, arg, m_has_lb, m_stop); - - /* Complain about missing lower bound. */ - if (!is_unsigned) + state_t state = sm_ctxt->get_state (stmt, arg); + /* Can't use a switch as the states are non-const. */ + if (state == m_tainted) { - sm_ctxt->warn_for_state (node, stmt, arg, m_has_ub, - new tainted_array_index (*this, diag_arg, - BOUNDS_UPPER)); - sm_ctxt->on_transition (node, stmt, arg, m_has_ub, m_stop); + /* Complain about missing bounds. */ + pending_diagnostic *d + = new tainted_array_index (*this, diag_arg, + is_unsigned + ? BOUNDS_LOWER : BOUNDS_NONE); + sm_ctxt->warn (node, stmt, arg, d); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } + else if (state == m_has_lb) + { + /* Complain about missing upper bound. */ + sm_ctxt->warn (node, stmt, arg, + new tainted_array_index (*this, diag_arg, + BOUNDS_LOWER)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } + else if (state == m_has_ub) + { + /* Complain about missing lower bound. */ + if (!is_unsigned) + { + sm_ctxt->warn (node, stmt, arg, + new tainted_array_index (*this, diag_arg, + BOUNDS_UPPER)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } } } } diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h index c5071464994..740cbecd7a9 100644 --- a/gcc/analyzer/sm.h +++ b/gcc/analyzer/sm.h @@ -198,11 +198,9 @@ public: } /* Called by state_machine in response to pattern matches: - issue a diagnostic D if VAR is in state STATE, using NODE and STMT - for location information. */ - virtual void warn_for_state (const supernode *node, const gimple *stmt, - tree var, state_machine::state_t state, - pending_diagnostic *d) = 0; + issue a diagnostic D using NODE and STMT for location information. */ + virtual void warn (const supernode *node, const gimple *stmt, + tree var, pending_diagnostic *d) = 0; /* For use when generating trees when creating pending_diagnostics, so that rather than e.g. -- 2.30.2