On Thu, 2020-01-30 at 15:36 +0100, Jakub Jelinek wrote:
> On Thu, Jan 30, 2020 at 09:27:33AM -0500, David Malcolm wrote:
> > --- 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))
>
> Isn't the analyzer invoked on GIMPLE? In GIMPLE, pointer equality of
> types
> is often not guaranteed and the compiler generally doesn't care about
> that,
> what we care about is whether the two types are compatible
> (types_compatible_p). E.g. if originally both types were say long
> int,
> but on lp64 one of the arguments was cast from long long int to long
> int,
> you can end up with one operand long int and the other long long int;
> they have the same precision etc., so it is ok to compare them.
Thanks. In patch 1 of the attached I've replaced the pointer comparison
with a call to types_compatible_p, and added a call to
types_compatible_p to guard a place where constants were being compared.
> > + {
> > + 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);
>
> This seems to be a waste of compile time memory. fold_build2 is
> essentially
> tem = fold_binary_loc (loc, code, type, op0, op1);
> if (!tem)
> tem = build2_loc (loc, code, type, op0, op1 PASS_MEM_STAT);
> but as you only care if the result is boolean_true_node or
> boolean_false_node, the build2_loc is unnecessary. So, just use
> fold_binary instead? If it returns NULL_TREE, it won't compare equal
> to
> either and will fall through to the TS_UNKNOWN case.
Thanks. I did some grepping and found that I'd made this mistake in
six places in the analyzer. Sorry about that. Patch 2 should fix all
of them.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for master?
David Malcolm (2):
analyzer: further fixes for comparisons between uncomparable types (PR
93450)
analyzer: avoid use of fold_build2
gcc/analyzer/constraint-manager.cc | 19 +++++++++----------
gcc/analyzer/region-model.cc | 8 ++++----
2 files changed, 13 insertions(+), 14 deletions(-)
--
2.21.0
