[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