From d177c49cd31131c8cededb216da30877d8a3856d Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 29 Jan 2020 20:24:42 -0500 Subject: [PATCH] analyzer: avoid comparisons between uncomparable types (PR 93450) PR analyzer/93450 reports an ICE trying to fold an EQ_EXPR comparison of (int)0 with (float)Inf. Most comparisons inside the analyzer come from gimple conditions, for which the necessary casts have already been added. This one is done inside constant_svalue::eval_condition as part of purging sm-state for an unknown function call, and fails to check the types being compared, leading to the ICE. sm_state_map::set_state calls region_model::eval_condition_without_cm in order to handle pointer equality (so that e.g. (void *)&r and (foo *)&r transition together), which leads to this code generating a bogus query to see if the two constants are equal. This patch fixes the ICE in two ways: - It avoids generating comparisons within constant_svalue::eval_condition unless the types are equal (thus for constants, but not for pointer values, which are handled by region_svalue). - It updates sm_state_map::set_state to bail immediately if the new state is the same as the old one, thus avoiding the above for the common case where an svalue_id has no sm-state (such as for the int and float constants in the reproducer), for which the above becomes a no-op. gcc/analyzer/ChangeLog: PR analyzer/93450 * program-state.cc (sm_state_map::set_state): For the overload taking an svalue_id, bail out if the set_state on the ec does nothing. Convert the latter's return type from void to bool, returning true if anything changed. (sm_state_map::impl_set_state): Convert the return type from void to bool, returning true if the state changed. * program-state.h (sm_state_map::set_state): Convert return type from void to bool. (sm_state_map::impl_set_state): Likewise. * region-model.cc (constant_svalue::eval_condition): Only call fold_build2 if the types are the same. gcc/testsuite/ChangeLog: PR analyzer/93450 * gcc.dg/analyzer/torture/pr93450.c: New test. --- gcc/analyzer/ChangeLog | 15 +++++++++++ gcc/analyzer/program-state.cc | 23 +++++++++++------ gcc/analyzer/program-state.h | 4 +-- gcc/analyzer/region-model.cc | 16 +++++++----- gcc/testsuite/ChangeLog | 5 ++++ .../gcc.dg/analyzer/torture/pr93450.c | 25 +++++++++++++++++++ 6 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 94a67ea40f7..f1ac6e6e63f 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,18 @@ +2020-01-30 David Malcolm + + PR analyzer/93450 + * program-state.cc (sm_state_map::set_state): For the overload + taking an svalue_id, bail out if the set_state on the ec does + nothing. Convert the latter's return type from void to bool, + returning true if anything changed. + (sm_state_map::impl_set_state): Convert the return type from void + to bool, returning true if the state changed. + * program-state.h (sm_state_map::set_state): Convert return type + from void to bool. + (sm_state_map::impl_set_state): Likewise. + * region-model.cc (constant_svalue::eval_condition): Only call + fold_build2 if the types are the same. + 2020-01-29 Jakub Jelinek * analyzer.h (PUSH_IGNORE_WFORMAT, POP_IGNORE_WFORMAT): Remove. diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index a9e300fba0f..f41f105ce6b 100644 --- a/gcc/analyzer/program-state.cc +++ b/gcc/analyzer/program-state.cc @@ -259,7 +259,8 @@ sm_state_map::set_state (region_model *model, if (model == NULL) return; equiv_class &ec = model->get_constraints ()->get_equiv_class (sid); - set_state (ec, state, origin); + if (!set_state (ec, state, origin)) + return; /* Also do it for all svalues that are equal via non-cm, so that e.g. (void *)&r and (foo *)&r transition together. */ @@ -276,34 +277,42 @@ sm_state_map::set_state (region_model *model, } /* Set the state of EC to STATE, recording that the state came from - ORIGIN. */ + ORIGIN. + Return true if any states of svalue_ids within EC changed. */ -void +bool sm_state_map::set_state (const equiv_class &ec, state_machine::state_t state, svalue_id origin) { int i; svalue_id *sid; + bool any_changed = false; FOR_EACH_VEC_ELT (ec.m_vars, i, sid) - impl_set_state (*sid, state, origin); + any_changed |= impl_set_state (*sid, state, origin); + return any_changed; } -/* Set state of PV to STATE, bypassing equivalence classes. */ +/* Set state of SID to STATE, bypassing equivalence classes. + Return true if the state changed. */ -void +bool sm_state_map::impl_set_state (svalue_id sid, state_machine::state_t state, svalue_id origin) { + if (get_state (sid) == state) + return false; + /* Special-case state 0 as the default value. */ if (state == 0) { if (m_map.get (sid)) m_map.remove (sid); - return; + return true; } gcc_assert (!sid.null_p ()); m_map.put (sid, entry_t (state, origin)); + return true; } /* Set the "global" state within this state map to STATE. */ diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h index adc71a4eda2..0a4e35f3d5d 100644 --- a/gcc/analyzer/program-state.h +++ b/gcc/analyzer/program-state.h @@ -161,10 +161,10 @@ public: svalue_id sid, state_machine::state_t state, svalue_id origin); - void set_state (const equiv_class &ec, + bool set_state (const equiv_class &ec, state_machine::state_t state, svalue_id origin); - void impl_set_state (svalue_id sid, + bool impl_set_state (svalue_id sid, state_machine::state_t state, svalue_id origin); diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index a5b3dffcc27..c838454a1eb 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -666,12 +666,16 @@ constant_svalue::eval_condition (constant_svalue *lhs, gcc_assert (CONSTANT_CLASS_P (lhs_const)); gcc_assert (CONSTANT_CLASS_P (rhs_const)); - tree comparison - = fold_build2 (op, boolean_type_node, lhs_const, rhs_const); - if (comparison == boolean_true_node) - return tristate (tristate::TS_TRUE); - if (comparison == boolean_false_node) - return tristate (tristate::TS_FALSE); + /* Check for comparable types. */ + if (TREE_TYPE (lhs_const) == TREE_TYPE (rhs_const)) + { + tree comparison + = fold_build2 (op, boolean_type_node, lhs_const, rhs_const); + if (comparison == boolean_true_node) + return tristate (tristate::TS_TRUE); + if (comparison == boolean_false_node) + return tristate (tristate::TS_FALSE); + } return tristate::TS_UNKNOWN; } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index ab61b48c7bd..a97bf325013 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-30 David Malcolm + + PR analyzer/93450 + * gcc.dg/analyzer/torture/pr93450.c: New test. + 2020-01-30 Jakub Jelinek PR target/93494 diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c b/gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c new file mode 100644 index 00000000000..7f6cba0db6d --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c @@ -0,0 +1,25 @@ +void +ed (int); + +double +bg (void) +{ + double kl = __builtin_huge_val (); + + ed (0); + + return kl; +} + +static double __attribute__((noinline)) +get_hugeval (void) +{ + return __builtin_huge_val (); +} + +int test_2 (int i) +{ + if (i < get_hugeval ()) + return 42; + return 0; +} -- 2.30.2