[PATCH] analyzer: fix handling of negative byte offsets (v2) (PR 93281)
Richard Biener
rguenther@suse.de
Fri Jan 17 08:06:00 GMT 2020
On Thu, 16 Jan 2020, David Malcolm wrote:
> 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?
OK.
> 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);
> }
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
More information about the Gcc-patches
mailing list