On Thu, 2020-01-16 at 09:14 +0100, Jakub Jelinek wrote:
> On Wed, Jan 15, 2020 at 06:56:48PM -0500, David Malcolm wrote:
> > gcc/analyzer/ChangeLog:
> > PR analyzer/93281
> > * region-model.cc
> > (region_model::convert_byte_offset_to_array_index): Convert
> > offset_cst to ssizetype before dividing by byte_size.
> > ---
> > gcc/analyzer/region-model.cc | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-
> > model.cc
> > index 5c39be4fd7f..b62ddf82a40 100644
> > --- a/gcc/analyzer/region-model.cc
> > +++ b/gcc/analyzer/region-model.cc
> > @@ -6414,9 +6414,12 @@
> > region_model::convert_byte_offset_to_array_index (tree ptr_type,
> > /* This might not be a constant. */
> > tree byte_size = size_in_bytes (elem_type);
> >
> > + /* Ensure we're in a signed representation before doing the
> > division. */
> > + tree signed_offset_cst = fold_convert (ssizetype,
> > offset_cst);
> > +
> > tree index
> > = fold_build2 (TRUNC_DIV_EXPR, integer_type_node,
> > - offset_cst, byte_size);
> > + signed_offset_cst, byte_size);
>
> I'd say you want to fold_convert byte_size to ssizetype too.
> Another thing is the integer_type_node, that looks wrong when you are
> dividing ssizetype by ssizetype. The fold-const.c folders are
> sensitive to
> using incorrect types and type mismatches.
> And, I think fold_build2 is wasteful, you only care if you can
> simplify it
> to a constant (just constant or INTEGER_CST only?).
> So, either use fold_binary (TRUNC_DIV_EXPR, ssizetype, offset_cst,
> fold_convert (ssizetype, byte_size))
> or, if you have checked that both arguments are INTEGER_CSTs, perhaps
> int_const_binop or so.
>
> > if (CONSTANT_CLASS_P (index))
>
> And this would need to be if (index && TREE_CODE (index) ==
> INTEGER_CST)
> (or if you can handle other constants (index && CONSTANT_CLASS_P
> (index)).
>
> > return get_or_create_constant_svalue (index);
>
> Jakub
Thanks.
Here's an updated version of the patch which fold_converts both
inputs to the division, and uses fold_binary rather than fold_build2.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
verified with -m32 and -m64.
OK for master?
gcc/analyzer/ChangeLog:
PR analyzer/93281
* region-model.cc
(region_model::convert_byte_offset_to_array_index): Convert to
ssizetype before dividing by byte_size. Use fold_binary rather
than fold_build2 to avoid needlessly constructing a tree for the
non-const case.
---
gcc/analyzer/region-model.cc | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 5c39be4fd7f..f67572e2d45 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -6414,11 +6414,13 @@ region_model::convert_byte_offset_to_array_index (tree
ptr_type,
/* This might not be a constant. */
tree byte_size = size_in_bytes (elem_type);
+ /* Try to get a constant by dividing, ensuring that we're in a
+ signed representation first. */
tree index
- = fold_build2 (TRUNC_DIV_EXPR, integer_type_node,
- offset_cst, byte_size);
-
- if (CONSTANT_CLASS_P (index))
+ = fold_binary (TRUNC_DIV_EXPR, ssizetype,
+ fold_convert (ssizetype, offset_cst),
+ fold_convert (ssizetype, byte_size));
+ if (index && TREE_CODE (index) == INTEGER_CST)
return get_or_create_constant_svalue (index);
}
--
2.21.0
