| Message ID | 20200117023645.4994-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | analyzer: fix handling of negative byte offsets (v2) (PR 93281) | expand |
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); > } > >
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); }