From 364925475f42774d15284306c2f75de1304e5fde Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 19 Jun 2020 17:29:40 -0400 Subject: [PATCH 242/293] FIXME: fix missing deref of NULL warning in pr94099.c --- gcc/analyzer/engine.cc | 29 ++++ gcc/analyzer/region-model2.cc | 163 ++++++++++--------- gcc/analyzer/region-model2.h | 2 + gcc/analyzer/sm-malloc.cc | 2 +- gcc/analyzer/sm.cc | 41 +++-- gcc/analyzer/sm.h | 10 +- gcc/testsuite/gcc.dg/analyzer/data-model-5.c | 4 + gcc/testsuite/gcc.dg/analyzer/pr94099.c | 3 +- 8 files changed, 163 insertions(+), 91 deletions(-) diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 6d8c5d7852b..1131a9254ca 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -455,6 +455,22 @@ public: m_sm_idx); } + tree is_zero_assignment (const gimple *stmt) FINAL OVERRIDE + { + const gassign *assign_stmt = dyn_cast (stmt); + if (!assign_stmt) + return NULL_TREE; + + enum tree_code op = gimple_assign_rhs_code (assign_stmt); + if (TREE_CODE_CLASS (op) != tcc_constant) + return NULL_TREE; + + if (!zerop (gimple_assign_rhs1 (assign_stmt))) + return NULL_TREE; + + return gimple_assign_lhs (assign_stmt); + } + log_user m_logger; exploded_graph &m_eg; const exploded_node *m_enode_for_diag; @@ -617,6 +633,19 @@ public: m_sm_idx); } + tree is_zero_assignment (const gimple *stmt) FINAL OVERRIDE + { + const gassign *assign_stmt = dyn_cast (stmt); + if (!assign_stmt) + return NULL_TREE; + if (const svalue2 *sval + = m_new_state->m_region_model2->get_gassign_result (assign_stmt, NULL)) + if (tree cst = sval->maybe_get_constant ()) + if (::zerop(cst)) + return gimple_assign_lhs (assign_stmt); + return NULL_TREE; + } + log_user m_logger; exploded_graph &m_eg; const exploded_node *m_enode_for_diag; diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index cfea545d5bb..c1f1fef5ade 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -4389,51 +4389,22 @@ region_model2::on_stmt (const gimple *stmt, } } -// FIXME: -/* Update this model for the ASSIGN stmt, using CTXT to report any - diagnostics. */ +/* If ASSIGN is a stmt that can be modelled via + set_value (lhs_reg, SVALUE, CTXT) + for some SVALUE, get the SVALUE. + Otherwise return NULL. */ -bool -region_model2::on_gassign (const gassign *assign, - region_model2_context *ctxt) +const svalue2 * +region_model2::get_gassign_result (const gassign *assign, + region_model2_context *ctxt) { tree lhs = gimple_assign_lhs (assign); tree rhs1 = gimple_assign_rhs1 (assign); - - const region2 *lhs_reg = get_lvalue (lhs, ctxt); - enum tree_code op = gimple_assign_rhs_code (assign); switch (op) { default: - { - if (1) - sorry_at (assign->location, "unhandled assignment op: %qs", - get_tree_code_name (op)); - gcc_unreachable (); - //set_to_new_unknown_value (lhs_reg, TREE_TYPE (lhs), ctxt); - } - break; - - case CONSTRUCTOR: - { - if (TREE_CLOBBER_P (rhs1)) - { - /* e.g. "x ={v} {CLOBBER};" */ - /* TODO: should we remove the value, or replace it with an - "unknown" value? */ - clobber_region (lhs_reg); - } - else - { - /* Any CONSTRUCTOR that survives to this point is presumably - just a zero-init of everything. */ - gcc_assert (CONSTRUCTOR_NELTS (rhs1) == 0); - gcc_assert (!CONSTRUCTOR_NO_CLEARING (rhs1)); - zero_fill_region (lhs_reg); - } - } - break; + return NULL; case POINTER_PLUS_EXPR: { @@ -4448,18 +4419,9 @@ region_model2::on_gassign (const gassign *assign, offset_sval = m_mgr->get_or_create_cast (size_type_node, offset_sval); const svalue2 *sval_binop - = m_mgr->get_or_create_binop (TREE_TYPE (lhs), op, - ptr_sval, offset_sval); - set_value (lhs_reg, sval_binop, ctxt); -#if 0 - region2 *element_reg - = get_or_create_pointer_plus_expr (TREE_TYPE (TREE_TYPE (ptr)), - ptr_sval, offset_sval, - ctxt); - const svalue2 *element_ptr_sval - = get_or_create_ptr_svalue2 (TREE_TYPE (ptr), element_reg); - set_value (lhs_reg, element_ptr_sval, ctxt); -#endif + = m_mgr->get_or_create_binop (TREE_TYPE (lhs), op, + ptr_sval, offset_sval); + return sval_binop; } break; @@ -4475,18 +4437,7 @@ region_model2::on_gassign (const gassign *assign, const svalue2 *sval_binop = m_mgr->get_or_create_binop (TREE_TYPE (lhs), op, rhs1_sval, rhs2_sval); - set_value (lhs_reg, sval_binop, ctxt); - } - break; - - case STRING_CST: - { - /* e.g. "struct s2 x = {{'A', 'B', 'C', 'D'}};". */ - /* Add a default binding, rather than a direct one, so that array - access will "inherit" the individual chars. */ - const svalue2 *rhs_sval = get_rvalue (rhs1, ctxt); - m_store.set_value (m_mgr->get_store2_manager(), lhs_reg, rhs_sval, - BK_default); + return sval_binop; } break; @@ -4506,11 +4457,7 @@ region_model2::on_gassign (const gassign *assign, case PARM_DECL:/* LHS = VAR; */ case REALPART_EXPR: case IMAGPART_EXPR: - { - const svalue2 *rhs_sval = get_rvalue (rhs1, ctxt); - set_value (lhs_reg, rhs_sval, ctxt); - } - break; + return get_rvalue (rhs1, ctxt); case ABS_EXPR: case ABSU_EXPR: @@ -4524,9 +4471,8 @@ region_model2::on_gassign (const gassign *assign, const svalue2 *rhs_sval = get_rvalue (rhs1, ctxt); const svalue2 *sval_unaryop = m_mgr->get_or_create_unaryop (TREE_TYPE (lhs), op, rhs_sval); - set_value (lhs_reg, sval_unaryop, ctxt); + return sval_unaryop; } - break; case EQ_EXPR: case GE_EXPR: @@ -4543,19 +4489,17 @@ region_model2::on_gassign (const gassign *assign, tristate t = eval_condition (rhs1_sval, op, rhs2_sval); if (t.is_known ()) - set_value (lhs_reg, - get_rvalue (t.is_true () - ? boolean_true_node - : boolean_false_node, - ctxt), - ctxt); + return get_rvalue (t.is_true () + ? boolean_true_node + : boolean_false_node, + ctxt); else { // TODO: symbolic value for binop const svalue2 *sval_binop = m_mgr->get_or_create_binop (TREE_TYPE (lhs), op, rhs1_sval, rhs2_sval); - set_value (lhs_reg, sval_binop, ctxt); + return sval_binop; } } break; @@ -4591,7 +4535,74 @@ region_model2::on_gassign (const gassign *assign, const svalue2 *sval_binop = m_mgr->get_or_create_binop (TREE_TYPE (lhs), op, rhs1_sval, rhs2_sval); - set_value (lhs_reg, sval_binop, ctxt); + return sval_binop; + } + } +} + +// FIXME: +/* Update this model for the ASSIGN stmt, using CTXT to report any + diagnostics. */ + +bool +region_model2::on_gassign (const gassign *assign, + region_model2_context *ctxt) +{ + tree lhs = gimple_assign_lhs (assign); + tree rhs1 = gimple_assign_rhs1 (assign); + + const region2 *lhs_reg = get_lvalue (lhs, ctxt); + + /* Most assignments are handled by: + set_value (lhs_reg, SVALUE, CTXT) + for some SVALUE. */ + if (const svalue2 *sval = get_gassign_result (assign, ctxt)) + { + set_value (lhs_reg, sval, ctxt); + return true; + } + + enum tree_code op = gimple_assign_rhs_code (assign); + switch (op) + { + default: + { + if (1) + sorry_at (assign->location, "unhandled assignment op: %qs", + get_tree_code_name (op)); + gcc_unreachable (); + //set_to_new_unknown_value (lhs_reg, TREE_TYPE (lhs), ctxt); + } + break; + + case CONSTRUCTOR: + { + if (TREE_CLOBBER_P (rhs1)) + { + /* e.g. "x ={v} {CLOBBER};" */ + /* TODO: should we remove the value, or replace it with an + "unknown" value? */ + clobber_region (lhs_reg); + } + else + { + /* Any CONSTRUCTOR that survives to this point is presumably + just a zero-init of everything. */ + gcc_assert (CONSTRUCTOR_NELTS (rhs1) == 0); + gcc_assert (!CONSTRUCTOR_NO_CLEARING (rhs1)); + zero_fill_region (lhs_reg); + } + } + break; + + case STRING_CST: + { + /* e.g. "struct s2 x = {{'A', 'B', 'C', 'D'}};". */ + /* Add a default binding, rather than a direct one, so that array + access will "inherit" the individual chars. */ + const svalue2 *rhs_sval = get_rvalue (rhs1, ctxt); + m_store.set_value (m_mgr->get_store2_manager(), lhs_reg, rhs_sval, + BK_default); } break; } diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h index 4a1f79cf007..1cf10b7c632 100644 --- a/gcc/analyzer/region-model2.h +++ b/gcc/analyzer/region-model2.h @@ -2142,6 +2142,8 @@ class region_model2 region_model2_context *ctxt); bool on_gassign (const gassign *stmt, region_model2_context *ctxt); + const svalue2 * get_gassign_result (const gassign *assign, + region_model2_context *ctxt); #if 0 bool on_gcall (const supernode *snode, const gcall *stmt); diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index 264810d82a6..5698aa26fe7 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -705,7 +705,7 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, } } - if (tree lhs = is_zero_assignment (stmt)) + if (tree lhs = sm_ctxt->is_zero_assignment (stmt)) if (any_pointer_p (lhs)) on_zero_assignment (sm_ctxt, node, stmt,lhs); diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc index affb5aa07db..baf6ff2c7b8 100644 --- a/gcc/analyzer/sm.cc +++ b/gcc/analyzer/sm.cc @@ -35,24 +35,47 @@ along with GCC; see the file COPYING3. If not see #if ENABLE_ANALYZER +namespace ana { + +#if 0 /* If STMT is an assignment from zero, return the LHS. */ tree -is_zero_assignment (const gimple *stmt) +is_zero_assignment (const gimple *stmt, sm_context *sm_ctxt) { const gassign *assign_stmt = dyn_cast (stmt); if (!assign_stmt) return NULL_TREE; - enum tree_code op = gimple_assign_rhs_code (assign_stmt); - if (TREE_CODE_CLASS (op) != tcc_constant) - return NULL_TREE; - - if (!zerop (gimple_assign_rhs1 (assign_stmt))) - return NULL_TREE; + switch (gimple_assign_rhs_code (assign_stmt)) + { + default: + return NULL_TREE; + + // FIXME: etc: + case COMPONENT_REF: + { + tree arg = gimple_assign_rhs1 (assign_stmt); + if (sm_ctxt->zerop (arg)) + return gimple_assign_lhs (assign_stmt); + } + break; + + case INTEGER_CST: +#if 0 + /* TODO: refactor region_model2::on_gassign so that we can get result, + for those that are expressed in "set_value" form. */ +#else + if (!zerop (gimple_assign_rhs1 (assign_stmt))) + return NULL_TREE; + + return gimple_assign_lhs (assign_stmt); +#endif + } - return gimple_assign_lhs (assign_stmt); + return NULL_TREE; } +#endif /* Return true if VAR has pointer or reference type. */ @@ -62,8 +85,6 @@ any_pointer_p (tree var) return POINTER_TYPE_P (TREE_TYPE (var)); } -namespace ana { - /* Add a state with name NAME to this state_machine. The string is required to outlive the state_machine. diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h index 5afb0c17a07..f94e4787c3e 100644 --- a/gcc/analyzer/sm.h +++ b/gcc/analyzer/sm.h @@ -23,15 +23,14 @@ along with GCC; see the file COPYING3. If not see /* Utility functions for use by state machines. */ -extern tree is_zero_assignment (const gimple *stmt); -extern bool any_pointer_p (tree var); - namespace ana { class state_machine; class sm_context; class pending_diagnostic; +extern bool any_pointer_p (tree var); + /* An abstract base class for a state machine describing an API. A mapping from state IDs to names, and various virtual functions for pattern-matching on statements. */ @@ -171,6 +170,11 @@ public: a signal handler. */ virtual void on_custom_transition (custom_transition *transition) = 0; + /* If STMT is an assignment known to assign zero to its LHS, return + the LHS. + Otherwise return NULL_TREE. */ + virtual tree is_zero_assignment (const gimple *stmt) = 0; + protected: sm_context (int sm_idx, const state_machine &sm) : m_sm_idx (sm_idx), m_sm (sm) {} diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-5.c b/gcc/testsuite/gcc.dg/analyzer/data-model-5.c index 5cf334768d8..22ecdbc0c74 100644 --- a/gcc/testsuite/gcc.dg/analyzer/data-model-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/data-model-5.c @@ -90,6 +90,10 @@ void unref (base_obj *obj) { if (--obj->ob_refcnt == 0) /* { dg-bogus "dereference of uninitialized pointer 'obj'" } */ obj->ob_type->tp_dealloc (obj); + /* { dg-warning "dereference of NULL 'obj'" "deref of NULL" { target *-*-* } .-2 } */ + /* FIXME: ideally we wouldn't issue this, as we've already issued a + warning about str_obj which is now in the "stop" state; the cast + confuses things. */ } void test_1 (const char *str) diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94099.c b/gcc/testsuite/gcc.dg/analyzer/pr94099.c index 0a34f561821..1d7a5d771d6 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr94099.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr94099.c @@ -21,7 +21,8 @@ pl (void) for (sc = 0; sc < 1; ++sc) { th.gk.hk = 0; - th.gk.bg[sc] = 0; /* { dg-warning "uninitialized" } */ + th.gk.bg[sc] = 0; /* { dg-warning "dereference of NULL '0'" } */ + // TODO: above message could be improved l3 (&th); } } -- 2.26.2