public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/1][PR fortran/29053&29054] Fix regressions in GDB intrinsic handling
@ 2022-04-22 10:52 Nils-Christian Kempke
  2022-04-22 10:52 ` [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix testsuite regressions for unix/-m32 board Nils-Christian Kempke
  0 siblings, 1 reply; 4+ messages in thread
From: Nils-Christian Kempke @ 2022-04-22 10:52 UTC (permalink / raw)
  To: gdb-patches

The patch submitted here

  https://sourceware.org/pipermail/gdb-patches/2022-April/187649.html

introduced regressions in the testsuite when running the unix/-m32
board, as described here

  https://sourceware.org/pipermail/gdb-patches/2022-April/187807.html

and in the GDB bugzilla entries

  https://sourceware.org/bugzilla/show_bug.cgi?id=29053

and

  https://sourceware.org/bugzilla/show_bug.cgi?id=29054

Thanks to Tom for spotting these - I am not sure how they slipped
through..

The reason for the failures however was, that the affected tests tried
to allocate Fortran arrays of sizes/dimensions bigger than 4 byte signed
integers, even when on 32 bit machines.

This lead to either compiler errors, as described in 29054, where an
array of size bigger than the max signed 4 byte integer was being
allocated which got caught by gfortran.  Or, as described in 29053, this
resulted in unexpected behavior of intrinsic functions called on arrays,
where the size itself was small enough to fit into a 4 byte signed
integer but instead the dimensions were outside of these limits.

For fixing this we saw ourselves presented with two options - either
remove the overflow test for 4 byte integer overflow or keep them and
fix them.

We for now decided to keep them and fixed the tests instead.  The given
patch does this in two ways.  First, a compile time detection for 32 bit
systems was necessary, in order to not allocate arrays with critical
dimensions/sizes when on 32 bit machines.  This was done using the
intrinsic module ISO_C_BINDING and from there C_NULL_PTR (Fortran 2003)
and C_SIZEOF (Fortran 2008).  We here check the size of a C pointer at
runtime to determine the system we are on.

Second, the lbound-ubound.exp file itself was adapted.  When running on a
32 bit target some of the test were disabled.

The tests compile and run with gcc/gfortran and icx/ifx.  They compile
but trigger some unrelated FAILS with icc/ifort where we are not sure
when these can be resolved.

The changes were tested for gcc/gfortran on unix/-32, unix,
native-remote, and native-extended-remote (the last two more out of
habit).  As icx/ifx does not support 32 bit currently only the other
three boards were run here.

Thanks,

Nils

Nils-Christian Kempke (1):
  gdb/testsuite: fix testsuite regressions for unix/-m32 board

 gdb/testsuite/gdb.fortran/lbound-ubound.F90 | 39 ++++++++++++---
 gdb/testsuite/gdb.fortran/lbound-ubound.exp | 13 +++--
 gdb/testsuite/gdb.fortran/size.f90          | 55 ++++++++++++++++-----
 3 files changed, 86 insertions(+), 21 deletions(-)

-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix testsuite regressions for unix/-m32 board
  2022-04-22 10:52 [PATCH 0/1][PR fortran/29053&29054] Fix regressions in GDB intrinsic handling Nils-Christian Kempke
@ 2022-04-22 10:52 ` Nils-Christian Kempke
  2022-05-05  6:26   ` Tom de Vries
  0 siblings, 1 reply; 4+ messages in thread
From: Nils-Christian Kempke @ 2022-04-22 10:52 UTC (permalink / raw)
  To: gdb-patches

This commit fixes the regressions noted in

  https://sourceware.org/bugzilla/show_bug.cgi?id=29053

and

  https://sourceware.org/bugzilla/show_bug.cgi?id=29054

introduced by 891e4190ba705373eec7b374209478215fff5401.

Reason for the failures was, that on a 32 bit machine the maximum
array length as well as the maximum allocatable memory for arrays
(in bytes) both seem to be limited by the maximum value of a 4
byte (signed) Fortran integer.  This lead to compiler errors/unexpected
behavior when compiling/running the test with the -m32 board.  This
behavior is compiler dependent and can differ for different compiler
implementations, but generally, it seemed like a good idea to simply
avoid such situations.

The affected tests check for GDB's overflow behavior when using KIND
parameters with GDB implemented Fortran intrinsic functions.  If these
KIND parameters are too small to fit the actual intrinsic function's
result, an overflow is expected.  This was done for 1, 2, and 4
byte overflows.  The last one caused problems, as it tried to allocate
arrays of length/byte-size bigger than the 4 byte signed integers which
would then be used with the LBOUND/UBOUND/SIZE intrinsics.

The tests were adapted to only execute the 4 byte overflow tests when
running on targets with 64 bit.  For this, the compiled tests evaluate the
byte size of a C_NULL_PTR via C_SIZEOF, both defined in the ISO_C_BINDING
module.  The ISO_C_BINDING constant C_NULL_PTR is a Fortran 2003, the
C_SIZEOF a Fortran 2008 extension.  Both have been implemented in their
respective compilers for while (e.g. C_SIZEOF is available since
gfortran 4.6).  If this byte size evaluates to less than 8 we skip the
4 byte overflow tests in the compiled tests of size.f90 and
lbound-ubound.f90.  Similarly, in the lbound-ubound.exp testsfile we skip
the 4 byte overflow tests if the procedure is_64_target evaluates to false.

In size.f90, additionally, the to-be-allocated amount of bytes did not
fit into 4 byte signed integers for some of the arrays, as it was
approximately 4 times the maximum size of a 4 byte signed integer.  We
adapted the dimensions of the arrays in question as the meaningfulness
of the test does not suffer from this.

With this patch both test run fine with the unix/-m32 board and
gcc/gfortran (9.4) as well as the standard board file.

We also thought about completely removing the affected test from the
testsuite.  We decided against this as the 32 bit identification comes
with Fortran 2008 and removing tests would have decreased coverage.

A last change that happened with this patch was due to gfortran's and
ifx's type resolution when assigning big constants to Fortran Integer*8
variables.  Before the above changes this happened in a parameter
statement.  Here, both compilers happily accepted a line like

  integer*8, parameter :: var = 2147483647 + 5.

After this change the assignment is not anymore done as a parameter
anymore, as this triggered compile time overflow errors.  Instead,
the assignment is done dynamically, depending on the kind of machine one
is on.  Sadly, just changing this line to

  integer*8 :: var
  var = 2147483647 + 5

does not work with ifx (or flang for that matter, they behave similarly
here).  It will create an integer overflow in the addition as ifx deduces
the type the additon is done in as Integer*4.  So var will actually
contain the value -2147483644 after this.  The lines

  integer*8 :: var
  var = 2147483652

on the other hand fail to compile with gfortran (9.4.0) as the compiler
identifies an Integer overflow here.  Finally, to make this work with
all three compilers an additional parameter has been introduced

  integer*8, parameter :: helper = 2147483647
  integer*8 :: var
  var = helper + 5.

This works on all 3 compilers as expected.

Signed-off-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
---
 gdb/testsuite/gdb.fortran/lbound-ubound.F90 | 39 ++++++++++++---
 gdb/testsuite/gdb.fortran/lbound-ubound.exp | 13 +++--
 gdb/testsuite/gdb.fortran/size.f90          | 55 ++++++++++++++++-----
 3 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.fortran/lbound-ubound.F90 b/gdb/testsuite/gdb.fortran/lbound-ubound.F90
index aa5be85bb5..4a4474ad85 100644
--- a/gdb/testsuite/gdb.fortran/lbound-ubound.F90
+++ b/gdb/testsuite/gdb.fortran/lbound-ubound.F90
@@ -49,6 +49,8 @@ end subroutine do_test
 ! Start of test program.
 !
 program test
+  use ISO_C_BINDING, only: C_NULL_PTR, C_SIZEOF
+
   interface
      subroutine do_test (lb, ub)
        integer*4, dimension (:) :: lb
@@ -74,8 +76,19 @@ program test
   integer, parameter :: b1_o = 127 + 2
   integer, parameter :: b2 = 32767 - 10
   integer, parameter :: b2_o = 32767 + 3
-  integer*8, parameter :: b4 = 2147483647 - 10
-  integer*8, parameter :: b4_o = 2147483647 + 5
+
+  ! This tests the GDB overflow behavior when using a KIND parameter too small
+  ! to hold the actual output argument.  This is done for 1, 2, and 4 byte
+  ! overflow.  On 32-bit machines most compilers will complain when trying to
+  ! allocate an array with ranges outside the 4 byte integer range.
+  ! We take the byte size of a C pointer as indication as to whether or not we
+  ! are on a 32 bit machine an skip the 4 byte overflow tests in that case.
+  integer, parameter :: bytes_c_ptr = C_SIZEOF(C_NULL_PTR)
+
+  integer*8, parameter :: max_signed_4byte_int = 2147483647
+  integer*8, parameter :: b4 = max_signed_4byte_int - 10
+  integer*8 :: b4_o
+  logical :: is_64_bit
 
   integer, allocatable :: array_1d_1bytes_overflow (:)
   integer, allocatable :: array_1d_2bytes_overflow (:)
@@ -84,6 +97,15 @@ program test
   integer, allocatable :: array_2d_2bytes_overflow (:,:)
   integer, allocatable :: array_3d_1byte_overflow (:,:,:)
 
+  ! Set the 4 byte overflow only on 64 bit machines.
+  if (bytes_c_ptr < 8) then
+    b4_o = 0
+    is_64_bit = .FALSE.
+  else
+    b4_o = max_signed_4byte_int + 5
+    is_64_bit = .TRUE.
+  end if
+
   ! Allocate or associate any variables as needed.
   allocate (other (-5:4, -2:7))
   pointer2d => tarray
@@ -91,8 +113,9 @@ program test
 
   allocate (array_1d_1bytes_overflow (-b1_o:-b1))
   allocate (array_1d_2bytes_overflow (b2:b2_o))
-  allocate (array_1d_4bytes_overflow (-b4_o:-b4))
-
+  if (is_64_bit) then
+    allocate (array_1d_4bytes_overflow (-b4_o:-b4))
+  end if
   allocate (array_2d_1byte_overflow (-b1_o:-b1,b1:b1_o))
   allocate (array_2d_2bytes_overflow (b2:b2_o,-b2_o:b2))
 
@@ -116,7 +139,9 @@ program test
   DO_TEST (array_1d_1bytes_overflow)
   DO_TEST (array_1d_2bytes_overflow)
 
-  DO_TEST (array_1d_4bytes_overflow)
+  if (is_64_bit) then
+    DO_TEST (array_1d_4bytes_overflow)
+  end if
   DO_TEST (array_2d_1byte_overflow)
   DO_TEST (array_2d_2bytes_overflow)
   DO_TEST (array_3d_1byte_overflow)
@@ -130,7 +155,9 @@ program test
   deallocate (array_2d_2bytes_overflow)
   deallocate (array_2d_1byte_overflow)
 
-  deallocate (array_1d_4bytes_overflow)
+  if (is_64_bit) then
+    deallocate (array_1d_4bytes_overflow)
+  end if
   deallocate (array_1d_2bytes_overflow)
   deallocate (array_1d_1bytes_overflow)
 
diff --git a/gdb/testsuite/gdb.fortran/lbound-ubound.exp b/gdb/testsuite/gdb.fortran/lbound-ubound.exp
index 334713666e..6be9d03af6 100644
--- a/gdb/testsuite/gdb.fortran/lbound-ubound.exp
+++ b/gdb/testsuite/gdb.fortran/lbound-ubound.exp
@@ -222,10 +222,15 @@ gdb_test "p lbound(array_1d_2bytes_overflow, 1, 2)" "= 32757"
 gdb_test "p ubound(array_1d_2bytes_overflow, 1, 2)" "= -32766"
 gdb_test "p ubound(array_1d_2bytes_overflow, 1, 4)" "= 32770"
 
-gdb_test "p lbound(array_1d_4bytes_overflow, 1, 4)" "= 2147483644"
-gdb_test "p lbound(array_1d_4bytes_overflow, 1, 8)" "= -2147483652"
-gdb_test "p ubound(array_1d_4bytes_overflow, 1, 4)" "= -2147483637"
-gdb_test "p lbound(array_1d_4bytes_overflow)" "= \\(2147483644\\)"
+# On 32-bit machines most compilers will complain when trying to allocate an
+# array with ranges outside the 4 byte integer range.  As the behavior is
+# compiler implementation dependent, we do not run these test on 32 bit targets.
+if {[is_64_target]} {
+    gdb_test "p lbound(array_1d_4bytes_overflow, 1, 4)" "= 2147483644"
+    gdb_test "p lbound(array_1d_4bytes_overflow, 1, 8)" "= -2147483652"
+    gdb_test "p ubound(array_1d_4bytes_overflow, 1, 4)" "= -2147483637"
+    gdb_test "p lbound(array_1d_4bytes_overflow)" "= \\(2147483644\\)"
+}
 
 # Ensure we reached the final breakpoint.  If more tests have been added
 # to the test script, and this starts failing, then the safety 'while'
diff --git a/gdb/testsuite/gdb.fortran/size.f90 b/gdb/testsuite/gdb.fortran/size.f90
index c924d84673..57e8c5fc84 100644
--- a/gdb/testsuite/gdb.fortran/size.f90
+++ b/gdb/testsuite/gdb.fortran/size.f90
@@ -17,6 +17,7 @@
 ! Start of test program.
 !
 program test
+  use ISO_C_BINDING, only: C_NULL_PTR, C_SIZEOF
 
   ! Things to perform tests on.
   integer, target :: array_1d (1:10) = 0
@@ -30,7 +31,17 @@ program test
 
   integer, parameter :: b1_o = 127 + 1
   integer, parameter :: b2_o = 32767 + 3
-  integer*8, parameter :: b4_o = 2147483647 + 5
+
+  ! This test tests the GDB overflow behavior when using a KIND parameter
+  ! too small to hold the actual output argument.  This is done for 1, 2, and
+  ! 4 byte overflow.  On 32-bit machines most compilers will complain when
+  ! trying to allocate an array with ranges outside the 4 byte integer range.
+  ! We take the byte size of a C pointer as indication as to whether or not we
+  ! are on a 32 bit machine an skip the 4 byte overflow tests in that case.
+  integer, parameter :: bytes_c_ptr = C_SIZEOF(C_NULL_PTR)
+  integer*8, parameter :: max_signed_4byte_int = 2147483647
+  integer*8 :: b4_o
+  logical :: is_64_bit
 
   integer, allocatable :: array_1d_1byte_overflow (:)
   integer, allocatable :: array_1d_2bytes_overflow (:)
@@ -42,12 +53,22 @@ program test
   ! Loop counters.
   integer :: s1, s2
 
+  ! Set the 4 byte overflow only on 64 bit machines.
+  if (bytes_c_ptr < 8) then
+    b4_o = 0
+    is_64_bit = .FALSE.
+  else
+    b4_o = max_signed_4byte_int + 5
+    is_64_bit = .TRUE.
+  end if
+
   allocate (array_1d_1byte_overflow (1:b1_o))
   allocate (array_1d_2bytes_overflow (1:b2_o))
-  allocate (array_1d_4bytes_overflow (1:b4_o))
-
+  if (is_64_bit) then
+    allocate (array_1d_4bytes_overflow (b4_o-b2_o:b4_o))
+  end if
   allocate (array_2d_1byte_overflow (1:b1_o, 1:b1_o))
-  allocate (array_2d_2bytes_overflow (1:b2_o, 1:b2_o))
+  allocate (array_2d_2bytes_overflow (b2_o-b1_o:b2_o, b2_o-b1_o:b2_o))
 
   allocate (array_3d_1byte_overflow (1:b1_o, 1:b1_o, 1:b1_o))
 
@@ -123,8 +144,10 @@ program test
   call test_size_4 (size (array_1d_1byte_overflow, 1))
   call test_size_4 (size (array_1d_2bytes_overflow, 1))
 
-  call test_size_4 (size (array_1d_4bytes_overflow))
-  call test_size_4 (size (array_1d_4bytes_overflow, 1))
+  if (is_64_bit) then
+    call test_size_4 (size (array_1d_4bytes_overflow))
+    call test_size_4 (size (array_1d_4bytes_overflow, 1))
+  end if
 
   call test_size_4 (size (array_2d_1byte_overflow, 1))
   call test_size_4 (size (array_2d_1byte_overflow, 2))
@@ -139,7 +162,9 @@ program test
 
   call test_size_1 (size (array_1d_1byte_overflow, 1, 1))
   call test_size_1 (size (array_1d_2bytes_overflow, 1, 1))
-  call test_size_1 (size (array_1d_4bytes_overflow, 1, 1))
+  if (is_64_bit) then
+    call test_size_1 (size (array_1d_4bytes_overflow, 1, 1))
+  end if
 
   call test_size_1 (size (array_2d_1byte_overflow, 1, 1))
   call test_size_1 (size (array_2d_1byte_overflow, 2, 1))
@@ -153,7 +178,9 @@ program test
   ! Kind 2.
   call test_size_2 (size (array_1d_1byte_overflow, 1, 2))
   call test_size_2 (size (array_1d_2bytes_overflow, 1, 2))
-  call test_size_2 (size (array_1d_4bytes_overflow, 1, 2))
+  if (is_64_bit) then
+    call test_size_2 (size (array_1d_4bytes_overflow, 1, 2))
+  end if
 
   call test_size_2 (size (array_2d_1byte_overflow, 1, 2))
   call test_size_2 (size (array_2d_1byte_overflow, 2, 2))
@@ -167,7 +194,9 @@ program test
   ! Kind 4.
   call test_size_4 (size (array_1d_1byte_overflow, 1, 4))
   call test_size_4 (size (array_1d_2bytes_overflow, 1, 4))
-  call test_size_4 (size (array_1d_4bytes_overflow, 1, 4))
+  if (is_64_bit) then
+    call test_size_4 (size (array_1d_4bytes_overflow, 1, 4))
+  end if
 
   call test_size_4 (size (array_2d_1byte_overflow, 1, 4))
   call test_size_4 (size (array_2d_1byte_overflow, 2, 4))
@@ -181,7 +210,9 @@ program test
   ! Kind 8.
   call test_size_8 (size (array_1d_1byte_overflow, 1, 8))
   call test_size_8 (size (array_1d_2bytes_overflow, 1, 8))
-  call test_size_8 (size (array_1d_4bytes_overflow, 1, 8))
+  if (is_64_bit) then
+    call test_size_8 (size (array_1d_4bytes_overflow, 1, 8))
+  end if
 
   call test_size_8 (size (array_2d_1byte_overflow, 1, 8))
   call test_size_8 (size (array_2d_1byte_overflow, 2, 8))
@@ -202,7 +233,9 @@ program test
   deallocate (array_2d_2bytes_overflow)
   deallocate (array_2d_1byte_overflow)
 
-  deallocate (array_1d_4bytes_overflow)
+  if (is_64_bit) then
+    deallocate (array_1d_4bytes_overflow)
+  end if
   deallocate (array_1d_2bytes_overflow)
   deallocate (array_1d_1byte_overflow)
 
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix testsuite regressions for unix/-m32 board
  2022-04-22 10:52 ` [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix testsuite regressions for unix/-m32 board Nils-Christian Kempke
@ 2022-05-05  6:26   ` Tom de Vries
  2022-05-10 12:52     ` Kempke, Nils-Christian
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2022-05-05  6:26 UTC (permalink / raw)
  To: Nils-Christian Kempke, gdb-patches

On 4/22/22 12:52, Nils-Christian Kempke wrote:
> This commit fixes the regressions noted in
> 
>    https://sourceware.org/bugzilla/show_bug.cgi?id=29053
> 
> and
> 
>    https://sourceware.org/bugzilla/show_bug.cgi?id=29054
> 
> introduced by 891e4190ba705373eec7b374209478215fff5401.
> 

Hi Nils,

Thanks for fixing this.

FWIW, a style that seems to have been adapted recently in git log 
messages is:
...
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29053
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29054
...
as the end of the message, you could think about adapting this style.

> Reason for the failures was, that on a 32 bit machine the maximum
> array length as well as the maximum allocatable memory for arrays
> (in bytes) both seem to be limited by the maximum value of a 4
> byte (signed) Fortran integer.  This lead to compiler errors/unexpected
> behavior when compiling/running the test with the -m32 board.  This
> behavior is compiler dependent and can differ for different compiler
> implementations, but generally, it seemed like a good idea to simply
> avoid such situations.
> 
> The affected tests check for GDB's overflow behavior when using KIND
> parameters with GDB implemented Fortran intrinsic functions.  If these
> KIND parameters are too small to fit the actual intrinsic function's
> result, an overflow is expected.  This was done for 1, 2, and 4
> byte overflows.  The last one caused problems, as it tried to allocate
> arrays of length/byte-size bigger than the 4 byte signed integers which
> would then be used with the LBOUND/UBOUND/SIZE intrinsics.
> 
> The tests were adapted to only execute the 4 byte overflow tests when
> running on targets with 64 bit.  For this, the compiled tests evaluate the
> byte size of a C_NULL_PTR via C_SIZEOF, both defined in the ISO_C_BINDING
> module.  The ISO_C_BINDING constant C_NULL_PTR is a Fortran 2003, the
> C_SIZEOF a Fortran 2008 extension.  Both have been implemented in their
> respective compilers for while (e.g. C_SIZEOF is available since
> gfortran 4.6).  If this byte size evaluates to less than 8 we skip the
> 4 byte overflow tests in the compiled tests of size.f90 and
> lbound-ubound.f90.  Similarly, in the lbound-ubound.exp testsfile we skip
> the 4 byte overflow tests if the procedure is_64_target evaluates to false.
> 
> In size.f90, additionally, the to-be-allocated amount of bytes did not
> fit into 4 byte signed integers for some of the arrays, as it was
> approximately 4 times the maximum size of a 4 byte signed integer.  We
> adapted the dimensions of the arrays in question as the meaningfulness
> of the test does not suffer from this.
> 
> With this patch both test run fine with the unix/-m32 board and
> gcc/gfortran (9.4) as well as the standard board file.
> 
> We also thought about completely removing the affected test from the
> testsuite.  We decided against this as the 32 bit identification comes
> with Fortran 2008 and removing tests would have decreased coverage.
> 
> A last change that happened with this patch was due to gfortran's and
> ifx's type resolution when assigning big constants to Fortran Integer*8
> variables.  Before the above changes this happened in a parameter
> statement.  Here, both compilers happily accepted a line like
> 
>    integer*8, parameter :: var = 2147483647 + 5.
> 
> After this change the assignment is not anymore done as a parameter
> anymore, as this triggered compile time overflow errors.  Instead,
> the assignment is done dynamically, depending on the kind of machine one
> is on.  Sadly, just changing this line to
> 
>    integer*8 :: var
>    var = 2147483647 + 5
> 
> does not work with ifx (or flang for that matter, they behave similarly
> here).  It will create an integer overflow in the addition as ifx deduces
> the type the additon is done in as Integer*4.  So var will actually
> contain the value -2147483644 after this.  The lines
> 
>    integer*8 :: var
>    var = 2147483652
> 
> on the other hand fail to compile with gfortran (9.4.0) as the compiler
> identifies an Integer overflow here.  Finally, to make this work with
> all three compilers an additional parameter has been introduced
> 
>    integer*8, parameter :: helper = 2147483647
>    integer*8 :: var
>    var = helper + 5.
> 
> This works on all 3 compilers as expected.
> 

My knowledge of fortran is basic at best, but the explanation is clear 
and detailed, and enough for me to understand the patch.

So, LGTM.

Thanks,
- Tom

> Signed-off-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
> ---
>   gdb/testsuite/gdb.fortran/lbound-ubound.F90 | 39 ++++++++++++---
>   gdb/testsuite/gdb.fortran/lbound-ubound.exp | 13 +++--
>   gdb/testsuite/gdb.fortran/size.f90          | 55 ++++++++++++++++-----
>   3 files changed, 86 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.fortran/lbound-ubound.F90 b/gdb/testsuite/gdb.fortran/lbound-ubound.F90
> index aa5be85bb5..4a4474ad85 100644
> --- a/gdb/testsuite/gdb.fortran/lbound-ubound.F90
> +++ b/gdb/testsuite/gdb.fortran/lbound-ubound.F90
> @@ -49,6 +49,8 @@ end subroutine do_test
>   ! Start of test program.
>   !
>   program test
> +  use ISO_C_BINDING, only: C_NULL_PTR, C_SIZEOF
> +
>     interface
>        subroutine do_test (lb, ub)
>          integer*4, dimension (:) :: lb
> @@ -74,8 +76,19 @@ program test
>     integer, parameter :: b1_o = 127 + 2
>     integer, parameter :: b2 = 32767 - 10
>     integer, parameter :: b2_o = 32767 + 3
> -  integer*8, parameter :: b4 = 2147483647 - 10
> -  integer*8, parameter :: b4_o = 2147483647 + 5
> +
> +  ! This tests the GDB overflow behavior when using a KIND parameter too small
> +  ! to hold the actual output argument.  This is done for 1, 2, and 4 byte
> +  ! overflow.  On 32-bit machines most compilers will complain when trying to
> +  ! allocate an array with ranges outside the 4 byte integer range.
> +  ! We take the byte size of a C pointer as indication as to whether or not we
> +  ! are on a 32 bit machine an skip the 4 byte overflow tests in that case.
> +  integer, parameter :: bytes_c_ptr = C_SIZEOF(C_NULL_PTR)
> +
> +  integer*8, parameter :: max_signed_4byte_int = 2147483647
> +  integer*8, parameter :: b4 = max_signed_4byte_int - 10
> +  integer*8 :: b4_o
> +  logical :: is_64_bit
>   
>     integer, allocatable :: array_1d_1bytes_overflow (:)
>     integer, allocatable :: array_1d_2bytes_overflow (:)
> @@ -84,6 +97,15 @@ program test
>     integer, allocatable :: array_2d_2bytes_overflow (:,:)
>     integer, allocatable :: array_3d_1byte_overflow (:,:,:)
>   
> +  ! Set the 4 byte overflow only on 64 bit machines.
> +  if (bytes_c_ptr < 8) then
> +    b4_o = 0
> +    is_64_bit = .FALSE.
> +  else
> +    b4_o = max_signed_4byte_int + 5
> +    is_64_bit = .TRUE.
> +  end if
> +
>     ! Allocate or associate any variables as needed.
>     allocate (other (-5:4, -2:7))
>     pointer2d => tarray
> @@ -91,8 +113,9 @@ program test
>   
>     allocate (array_1d_1bytes_overflow (-b1_o:-b1))
>     allocate (array_1d_2bytes_overflow (b2:b2_o))
> -  allocate (array_1d_4bytes_overflow (-b4_o:-b4))
> -
> +  if (is_64_bit) then
> +    allocate (array_1d_4bytes_overflow (-b4_o:-b4))
> +  end if
>     allocate (array_2d_1byte_overflow (-b1_o:-b1,b1:b1_o))
>     allocate (array_2d_2bytes_overflow (b2:b2_o,-b2_o:b2))
>   
> @@ -116,7 +139,9 @@ program test
>     DO_TEST (array_1d_1bytes_overflow)
>     DO_TEST (array_1d_2bytes_overflow)
>   
> -  DO_TEST (array_1d_4bytes_overflow)
> +  if (is_64_bit) then
> +    DO_TEST (array_1d_4bytes_overflow)
> +  end if
>     DO_TEST (array_2d_1byte_overflow)
>     DO_TEST (array_2d_2bytes_overflow)
>     DO_TEST (array_3d_1byte_overflow)
> @@ -130,7 +155,9 @@ program test
>     deallocate (array_2d_2bytes_overflow)
>     deallocate (array_2d_1byte_overflow)
>   
> -  deallocate (array_1d_4bytes_overflow)
> +  if (is_64_bit) then
> +    deallocate (array_1d_4bytes_overflow)
> +  end if
>     deallocate (array_1d_2bytes_overflow)
>     deallocate (array_1d_1bytes_overflow)
>   
> diff --git a/gdb/testsuite/gdb.fortran/lbound-ubound.exp b/gdb/testsuite/gdb.fortran/lbound-ubound.exp
> index 334713666e..6be9d03af6 100644
> --- a/gdb/testsuite/gdb.fortran/lbound-ubound.exp
> +++ b/gdb/testsuite/gdb.fortran/lbound-ubound.exp
> @@ -222,10 +222,15 @@ gdb_test "p lbound(array_1d_2bytes_overflow, 1, 2)" "= 32757"
>   gdb_test "p ubound(array_1d_2bytes_overflow, 1, 2)" "= -32766"
>   gdb_test "p ubound(array_1d_2bytes_overflow, 1, 4)" "= 32770"
>   
> -gdb_test "p lbound(array_1d_4bytes_overflow, 1, 4)" "= 2147483644"
> -gdb_test "p lbound(array_1d_4bytes_overflow, 1, 8)" "= -2147483652"
> -gdb_test "p ubound(array_1d_4bytes_overflow, 1, 4)" "= -2147483637"
> -gdb_test "p lbound(array_1d_4bytes_overflow)" "= \\(2147483644\\)"
> +# On 32-bit machines most compilers will complain when trying to allocate an
> +# array with ranges outside the 4 byte integer range.  As the behavior is
> +# compiler implementation dependent, we do not run these test on 32 bit targets.
> +if {[is_64_target]} {
> +    gdb_test "p lbound(array_1d_4bytes_overflow, 1, 4)" "= 2147483644"
> +    gdb_test "p lbound(array_1d_4bytes_overflow, 1, 8)" "= -2147483652"
> +    gdb_test "p ubound(array_1d_4bytes_overflow, 1, 4)" "= -2147483637"
> +    gdb_test "p lbound(array_1d_4bytes_overflow)" "= \\(2147483644\\)"
> +}
>   
>   # Ensure we reached the final breakpoint.  If more tests have been added
>   # to the test script, and this starts failing, then the safety 'while'
> diff --git a/gdb/testsuite/gdb.fortran/size.f90 b/gdb/testsuite/gdb.fortran/size.f90
> index c924d84673..57e8c5fc84 100644
> --- a/gdb/testsuite/gdb.fortran/size.f90
> +++ b/gdb/testsuite/gdb.fortran/size.f90
> @@ -17,6 +17,7 @@
>   ! Start of test program.
>   !
>   program test
> +  use ISO_C_BINDING, only: C_NULL_PTR, C_SIZEOF
>   
>     ! Things to perform tests on.
>     integer, target :: array_1d (1:10) = 0
> @@ -30,7 +31,17 @@ program test
>   
>     integer, parameter :: b1_o = 127 + 1
>     integer, parameter :: b2_o = 32767 + 3
> -  integer*8, parameter :: b4_o = 2147483647 + 5
> +
> +  ! This test tests the GDB overflow behavior when using a KIND parameter
> +  ! too small to hold the actual output argument.  This is done for 1, 2, and
> +  ! 4 byte overflow.  On 32-bit machines most compilers will complain when
> +  ! trying to allocate an array with ranges outside the 4 byte integer range.
> +  ! We take the byte size of a C pointer as indication as to whether or not we
> +  ! are on a 32 bit machine an skip the 4 byte overflow tests in that case.
> +  integer, parameter :: bytes_c_ptr = C_SIZEOF(C_NULL_PTR)
> +  integer*8, parameter :: max_signed_4byte_int = 2147483647
> +  integer*8 :: b4_o
> +  logical :: is_64_bit
>   
>     integer, allocatable :: array_1d_1byte_overflow (:)
>     integer, allocatable :: array_1d_2bytes_overflow (:)
> @@ -42,12 +53,22 @@ program test
>     ! Loop counters.
>     integer :: s1, s2
>   
> +  ! Set the 4 byte overflow only on 64 bit machines.
> +  if (bytes_c_ptr < 8) then
> +    b4_o = 0
> +    is_64_bit = .FALSE.
> +  else
> +    b4_o = max_signed_4byte_int + 5
> +    is_64_bit = .TRUE.
> +  end if
> +
>     allocate (array_1d_1byte_overflow (1:b1_o))
>     allocate (array_1d_2bytes_overflow (1:b2_o))
> -  allocate (array_1d_4bytes_overflow (1:b4_o))
> -
> +  if (is_64_bit) then
> +    allocate (array_1d_4bytes_overflow (b4_o-b2_o:b4_o))
> +  end if
>     allocate (array_2d_1byte_overflow (1:b1_o, 1:b1_o))
> -  allocate (array_2d_2bytes_overflow (1:b2_o, 1:b2_o))
> +  allocate (array_2d_2bytes_overflow (b2_o-b1_o:b2_o, b2_o-b1_o:b2_o))
>   
>     allocate (array_3d_1byte_overflow (1:b1_o, 1:b1_o, 1:b1_o))
>   
> @@ -123,8 +144,10 @@ program test
>     call test_size_4 (size (array_1d_1byte_overflow, 1))
>     call test_size_4 (size (array_1d_2bytes_overflow, 1))
>   
> -  call test_size_4 (size (array_1d_4bytes_overflow))
> -  call test_size_4 (size (array_1d_4bytes_overflow, 1))
> +  if (is_64_bit) then
> +    call test_size_4 (size (array_1d_4bytes_overflow))
> +    call test_size_4 (size (array_1d_4bytes_overflow, 1))
> +  end if
>   
>     call test_size_4 (size (array_2d_1byte_overflow, 1))
>     call test_size_4 (size (array_2d_1byte_overflow, 2))
> @@ -139,7 +162,9 @@ program test
>   
>     call test_size_1 (size (array_1d_1byte_overflow, 1, 1))
>     call test_size_1 (size (array_1d_2bytes_overflow, 1, 1))
> -  call test_size_1 (size (array_1d_4bytes_overflow, 1, 1))
> +  if (is_64_bit) then
> +    call test_size_1 (size (array_1d_4bytes_overflow, 1, 1))
> +  end if
>   
>     call test_size_1 (size (array_2d_1byte_overflow, 1, 1))
>     call test_size_1 (size (array_2d_1byte_overflow, 2, 1))
> @@ -153,7 +178,9 @@ program test
>     ! Kind 2.
>     call test_size_2 (size (array_1d_1byte_overflow, 1, 2))
>     call test_size_2 (size (array_1d_2bytes_overflow, 1, 2))
> -  call test_size_2 (size (array_1d_4bytes_overflow, 1, 2))
> +  if (is_64_bit) then
> +    call test_size_2 (size (array_1d_4bytes_overflow, 1, 2))
> +  end if
>   
>     call test_size_2 (size (array_2d_1byte_overflow, 1, 2))
>     call test_size_2 (size (array_2d_1byte_overflow, 2, 2))
> @@ -167,7 +194,9 @@ program test
>     ! Kind 4.
>     call test_size_4 (size (array_1d_1byte_overflow, 1, 4))
>     call test_size_4 (size (array_1d_2bytes_overflow, 1, 4))
> -  call test_size_4 (size (array_1d_4bytes_overflow, 1, 4))
> +  if (is_64_bit) then
> +    call test_size_4 (size (array_1d_4bytes_overflow, 1, 4))
> +  end if
>   
>     call test_size_4 (size (array_2d_1byte_overflow, 1, 4))
>     call test_size_4 (size (array_2d_1byte_overflow, 2, 4))
> @@ -181,7 +210,9 @@ program test
>     ! Kind 8.
>     call test_size_8 (size (array_1d_1byte_overflow, 1, 8))
>     call test_size_8 (size (array_1d_2bytes_overflow, 1, 8))
> -  call test_size_8 (size (array_1d_4bytes_overflow, 1, 8))
> +  if (is_64_bit) then
> +    call test_size_8 (size (array_1d_4bytes_overflow, 1, 8))
> +  end if
>   
>     call test_size_8 (size (array_2d_1byte_overflow, 1, 8))
>     call test_size_8 (size (array_2d_1byte_overflow, 2, 8))
> @@ -202,7 +233,9 @@ program test
>     deallocate (array_2d_2bytes_overflow)
>     deallocate (array_2d_1byte_overflow)
>   
> -  deallocate (array_1d_4bytes_overflow)
> +  if (is_64_bit) then
> +    deallocate (array_1d_4bytes_overflow)
> +  end if
>     deallocate (array_1d_2bytes_overflow)
>     deallocate (array_1d_1byte_overflow)
>   

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix testsuite regressions for unix/-m32 board
  2022-05-05  6:26   ` Tom de Vries
@ 2022-05-10 12:52     ` Kempke, Nils-Christian
  0 siblings, 0 replies; 4+ messages in thread
From: Kempke, Nils-Christian @ 2022-05-10 12:52 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Hi Tom,


> -----Original Message-----
> From: Tom de Vries <tdevries@suse.de>
> Sent: Thursday, May 5, 2022 8:26 AM
> To: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix
> testsuite regressions for unix/-m32 board
> 
> On 4/22/22 12:52, Nils-Christian Kempke wrote:
> > This commit fixes the regressions noted in
> >
> >    https://sourceware.org/bugzilla/show_bug.cgi?id=29053
> >
> > and
> >
> >    https://sourceware.org/bugzilla/show_bug.cgi?id=29054
> >
> > introduced by 891e4190ba705373eec7b374209478215fff5401.
> >
> 
> Hi Nils,
> 
> Thanks for fixing this.
> 
> FWIW, a style that seems to have been adapted recently in git log
> messages is:
> ...
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29053
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29054
> ...
> as the end of the message, you could think about adapting this style.
> 

Yes, you are right. I'll change it for this commit too. I'll anyways have to
rebase it so I can also adapt the message here before pushing.

I'll update it within the hour and push the patch,

Thanks!
Nils

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-05-10 12:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22 10:52 [PATCH 0/1][PR fortran/29053&29054] Fix regressions in GDB intrinsic handling Nils-Christian Kempke
2022-04-22 10:52 ` [PATCH 1/1][PR fortran/29053&29054] gdb/testsuite: fix testsuite regressions for unix/-m32 board Nils-Christian Kempke
2022-05-05  6:26   ` Tom de Vries
2022-05-10 12:52     ` Kempke, Nils-Christian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).