| Message ID | 20200218012913.11549-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | analyzer: fix ICE on pointer arithmetic with incomplete types [PR 93774] | expand |
Hi David in principle, any valid test case (especially for an ICE) should count as obvious and simple, so no approval should be needed. Having said that, I think I would prefer a copy of the original test case rather than an include statement. Although we usually do not change or remove test cases, sometimes it is done for one reason or anotjer, and in this case I would prefer that the derived test case does not change automatically. So, all four of your test cases are OK if you change those which use include to a plain test case, maybe with a comment pointing to the original one. In the future, you can just commit this kind of test case as simple and obvious; we would appreciate a note to fortran@ if you do so. Regards Thomas
On Tue, 2020-02-18 at 08:57 +0100, Thomas König wrote: > Hi David > > in principle, any valid test case (especially for an ICE) should > count as obvious and simple, so no approval should be needed. > > Having said that, I think I would prefer a copy of the original test > case rather than an include statement. Although we usually do not > change or remove test cases, sometimes it is done for one reason or > anotjer, and in this case I would prefer that the derived test case > does not change automatically. Thanks - I wasn't sure which approach was preferable here. > So, all four of your test cases are OK if you change those which use > include to a plain test case, maybe with a comment pointing to the > original one. I'll go ahead and rework those tests accordingly. > In the future, you can just commit this kind of test case as simple > and obvious; we would appreciate a note to fortran@ if you do so. Thanks - will do. Dave
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index deb201546f3..659255a8db4 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -6514,24 +6514,27 @@ region_model::convert_byte_offset_to_array_index (tree ptr_type, /* Arithmetic on void-pointers is a GNU C extension, treating the size of a void as 1. - https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html - - Returning early for this case avoids a diagnostic from within the - call to size_in_bytes. */ + https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html */ if (TREE_CODE (elem_type) == VOID_TYPE) return offset_sid; - /* 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_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); + /* First, use int_size_in_bytes, to reject the case where we have an + incomplete type, or a non-constant value. */ + HOST_WIDE_INT hwi_byte_size = int_size_in_bytes (elem_type); + if (hwi_byte_size > 0) + { + /* Now call size_in_bytes to get the answer in tree form. */ + tree byte_size = size_in_bytes (elem_type); + gcc_assert (byte_size); + /* Try to get a constant by dividing, ensuring that we're in a + signed representation first. */ + tree 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); + } } /* Otherwise, we don't know the array index; generate a new unknown value. diff --git a/gcc/testsuite/gfortran.dg/analyzer/deferred_character_25.f90 b/gcc/testsuite/gfortran.dg/analyzer/deferred_character_25.f90 new file mode 100644 index 00000000000..09303630d98 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/analyzer/deferred_character_25.f90 @@ -0,0 +1,3 @@ +! { dg-do compile } +! { dg-additional-options "-Wno-analyzer-too-complex" } +include "../deferred_character_25.f90"
PR analyzer/93774 reports an ICE in gfortran with -fanalyzer within region_model::convert_byte_offset_to_array_index on a pointer of incomplete type ("character(kind=1)[0:][1:0] * restrict"). This patch bulletproofs the routine against incomplete types, fixing the ICE. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. I think I have permission to self-approve the analyzer part. Is the Fortran testcase OK for master? gcc/analyzer/ChangeLog: PR analyzer/93774 * region-model.cc (region_model::convert_byte_offset_to_array_index): Use int_size_in_bytes before calling size_in_bytes, to gracefully fail on incomplete types. gcc/testsuite/ChangeLog: PR analyzer/93774 * gfortran.dg/analyzer/deferred_character_25.f90: New test. --- gcc/analyzer/region-model.cc | 33 ++++++++++--------- .../analyzer/deferred_character_25.f90 | 3 ++ 2 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/analyzer/deferred_character_25.f90