diff options
author | David Malcolm <dmalcolm@redhat.com> | 2020-01-29 20:24:42 -0500 |
---|---|---|
committer | David Malcolm <dmalcolm@redhat.com> | 2020-01-30 09:21:20 -0500 |
commit | d177c49cd31131c8cededb216da30877d8a3856d (patch) | |
tree | 44202900181a2c828182ebd265561e0453e1f48b /gcc | |
parent | 64464e5f369231d2998608138da760274f256581 (diff) | |
download | gcc-d177c49cd31131c8cededb216da30877d8a3856d.zip gcc-d177c49cd31131c8cededb216da30877d8a3856d.tar.gz gcc-d177c49cd31131c8cededb216da30877d8a3856d.tar.bz2 |
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.
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/analyzer/ChangeLog | 15 | ||||
-rw-r--r-- | gcc/analyzer/program-state.cc | 23 | ||||
-rw-r--r-- | gcc/analyzer/program-state.h | 4 | ||||
-rw-r--r-- | gcc/analyzer/region-model.cc | 16 | ||||
-rw-r--r-- | gcc/testsuite/ChangeLog | 5 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c | 25 |
6 files changed, 73 insertions, 15 deletions
diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 94a67ea..f1ac6e6 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,18 @@ +2020-01-30 David Malcolm <dmalcolm@redhat.com> + + 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 <jakub@redhat.com> * analyzer.h (PUSH_IGNORE_WFORMAT, POP_IGNORE_WFORMAT): Remove. diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index a9e300f..f41f105 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 adc71a4..0a4e35f 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 a5b3dff..c838454 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 ab61b48..a97bf32 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-30 David Malcolm <dmalcolm@redhat.com> + + PR analyzer/93450 + * gcc.dg/analyzer/torture/pr93450.c: New test. + 2020-01-30 Jakub Jelinek <jakub@redhat.com> 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 0000000..7f6cba0 --- /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; +} |