[RFA] Add handling of null Ada arrays

[RFA] Add handling of null Ada arrays

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

Hello,

In Ada, arrays can be empty by setting the high bound to a smaller value
than the low bound. For instance, a typical declaration would be:

   type Empty_Array is array (1 .. 0) of Integer;

The attached patch fixes a bug when trying to print a variable
of such a type:

   (gdb) print my_table
   object size is larger than varsize-limit

The problem is that we compute the array length using the bounds
from the index type without checking first that the high bound
is larger or equal to the small bound.  There were a couple of
places were we did that, so we made some adjustments there.

2008-01-09  Joel Brobecker  <brobecker@adacore.com>

        * gdbtypes.c (create_array_type): Add handling of null Ada arrays.
        (check_typedef): Likewise.

I was able to write a testcase too:

2008-01-09  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/null_array: New test program.
        * gdb.ada/null_array.exp: New testcase.

Tested on x86-linux, no regression.
OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: null_array.diff --]
[-- Type: text/plain, Size: 1908 bytes --]

Index: gdbtypes.c
===================================================================
--- gdbtypes.c	(revision 113)
+++ gdbtypes.c	(revision 116)
@@ -811,8 +811,14 @@ create_array_type (struct type *result_t
   if (get_discrete_bounds (range_type, &low_bound, &high_bound) < 0)
     low_bound = high_bound = 0;
   CHECK_TYPEDEF (element_type);
-  TYPE_LENGTH (result_type) =
-    TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
+  /* Be careful when setting the array length.  Ada arrays can be
+     empty arrays with the high_bound being smaller than the low_bound.
+     In such cases, the array length should be zero.  */
+  if (high_bound < low_bound)
+    TYPE_LENGTH (result_type) = 0;
+  else
+    TYPE_LENGTH (result_type) =
+      TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
   TYPE_NFIELDS (result_type) = 1;
   TYPE_FIELDS (result_type) =
     (struct field *) TYPE_ALLOC (result_type, sizeof (struct field));
@@ -1492,11 +1498,19 @@ check_typedef (struct type *type)
 		   == TYPE_CODE_RANGE))
 	{
 	  /* Now recompute the length of the array type, based on its
-	     number of elements and the target type's length.  */
-	  TYPE_LENGTH (type) =
-	    ((TYPE_FIELD_BITPOS (range_type, 1)
-	      - TYPE_FIELD_BITPOS (range_type, 0) + 1)
-	     * TYPE_LENGTH (target_type));
+	     number of elements and the target type's length.
+	     Watch out for Ada null Ada arrays where the high bound
+	     is smaller than the low bound.  */
+	  const int low_bound = TYPE_FIELD_BITPOS (range_type, 0);
+	  const int high_bound = TYPE_FIELD_BITPOS (range_type, 1);
+	  int nb_elements;
+	
+	  if (high_bound < low_bound)
+	    nb_elements = 0;
+	  else
+	    nb_elements = high_bound - low_bound + 1;
+	
+	  TYPE_LENGTH (type) = nb_elements * TYPE_LENGTH (target_type);
 	  TYPE_FLAGS (type) &= ~TYPE_FLAG_TARGET_STUB;
 	}
       else if (TYPE_CODE (type) == TYPE_CODE_RANGE)

[-- Attachment #3: null_array-tc.diff --]
[-- Type: text/plain, Size: 5097 bytes --]

Index: gdb.ada/null_array.exp
===================================================================
--- gdb.ada/null_array.exp	(revision 0)
+++ gdb.ada/null_array.exp	(revision 115)
@@ -0,0 +1,50 @@
+# Copyright 2008 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "null_array"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "START" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+# Test printing and type-printing of a tagged type that is not
+# class-wide.
+
+gdb_test "print my_table" \
+         "\\(\\)" \
+         "print my_table"
+
+gdb_test "ptype my_table" \
+         "type = array \\(10 \\.\\. 1\\) of integer" \
+         "ptype my_table"
+
Index: gdb.ada/null_array/pck.adb
===================================================================
--- gdb.ada/null_array/pck.adb	(revision 0)
+++ gdb.ada/null_array/pck.adb	(revision 115)
@@ -0,0 +1,28 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+
+   function Ident (I : Integer) return Integer is
+   begin
+      return I;
+   end Ident;
+
+   procedure Do_Nothing (A : System.Address) is
+   begin
+      null;
+   end Do_Nothing;
+
+end Pck;
Index: gdb.ada/null_array/pck.ads
===================================================================
--- gdb.ada/null_array/pck.ads	(revision 0)
+++ gdb.ada/null_array/pck.ads	(revision 115)
@@ -0,0 +1,24 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with System;
+
+package Pck is
+
+   function Ident (I : Integer) return Integer;
+
+   procedure Do_Nothing (A : System.Address);
+
+end Pck;
Index: gdb.ada/null_array/foo.adb
===================================================================
--- gdb.ada/null_array/foo.adb	(revision 0)
+++ gdb.ada/null_array/foo.adb	(revision 115)
@@ -0,0 +1,24 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   type Table is array (Integer range <>) of Integer;
+
+   My_Table : Table (Ident (10) .. Ident (1));
+begin
+   Do_Nothing (My_Table'Address);  -- START
+end Foo;

RE: [RFA] Add handling of null Ada arrays

[Pierre Muller]

  At least in pascal language it is quite
common to use things like 
type
  BigArray = Array [1..0xffffffff] of integer;
if you want to be sure that you will never get into 
troubles due to range checking...

  Of course you cannot allocate a pointer to such a type
directly, and it would eat up a lot of memory space.
  But this kind of types always gave problems
inside gdb, because when you wanted to look at
the value gdb would try to copy the whole array...
even if cases where it would only display the first elements,
which is kind of silly, no?

  Do not interpret this remark as an argument against
your patch, I just wanted to state that arrays with
strange bound values have several problems...

Pierre Muller
Pascal language maintainer.


> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: Wednesday, January 09, 2008 7:34 AM
> To: gdb-patches@sourceware.org
> Subject: [RFA] Add handling of null Ada arrays
> 
> Hello,
> 
> In Ada, arrays can be empty by setting the high bound to a smaller
> value than the low bound. For instance, a typical declaration would be:
> 
>    type Empty_Array is array (1 .. 0) of Integer;
> 
> The attached patch fixes a bug when trying to print a variable of such
> a type:
> 
>    (gdb) print my_table
>    object size is larger than varsize-limit
> 
> The problem is that we compute the array length using the bounds from
> the index type without checking first that the high bound is larger or
> equal to the small bound.  There were a couple of places were we did
> that, so we made some adjustments there.
> 
> 2008-01-09  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdbtypes.c (create_array_type): Add handling of null Ada
> arrays.
>         (check_typedef): Likewise.
> 
> I was able to write a testcase too:
> 
> 2008-01-09  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdb.ada/null_array: New test program.
>         * gdb.ada/null_array.exp: New testcase.
> 
> Tested on x86-linux, no regression.
> OK to commit?
> 
> Thanks,
> --
> Joel

Re: [RFA] Add handling of null Ada arrays

[Daniel Jacobowitz]

On Tue, Jan 08, 2008 at 10:33:42PM -0800, Joel Brobecker wrote:
> 2008-01-09  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdbtypes.c (create_array_type): Add handling of null Ada arrays.
>         (check_typedef): Likewise.

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Add handling of null Ada arrays

[Joel Brobecker]

> > 2008-01-09  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * gdbtypes.c (create_array_type): Add handling of null Ada arrays.
> >         (check_typedef): Likewise.
> 
> OK.

Thanks, Daniel. Now checked in.

-- 
Joel

Re: [RFA] Add handling of null Ada arrays

[Joel Brobecker]

>   At least in pascal language it is quite
> common to use things like 
> type
>   BigArray = Array [1..0xffffffff] of integer;
> if you want to be sure that you will never get into 
> troubles due to range checking...

That's ... strange :).

>   Of course you cannot allocate a pointer to such a type
> directly, and it would eat up a lot of memory space.

Right. If you did that in Ada, you would immediately trigger
an exception. In fact, I believe that GNAT detects static cases
like this and replaces the code with an exception raise.

How does the above work in Pascal? If the program hasn't "allocated
a pointer to such type", does it mean that the size of the array is
dynamic and grown as needed? I think it's be interesting to understand
how it works underneath if we want to improve the situation for Pascal
too.

On my end, my Pascal days are long behind me (I think the last time
I wrote some Pascal was in 92 or 93), but I thought that Pascal arrays
were allocated when declared and that the bounds could not be changed
after...

> But this kind of types always gave problems inside gdb, because when
> you wanted to look at the value gdb would try to copy the whole
> array...  even if cases where it would only display the first
> elements, which is kind of silly, no?

We had the exact same issue with Ada, actually. Paul Hilfinger
fixed it for us.

The good news is that my patch should avoid GDB trying to allocate
0xffffffff bytes for your array, which usually leads to a debugger
crash ;-).

-- 
Joel

Re: [RFA] Add handling of null Ada arrays

[Jim Blandy]

"Pierre Muller" <muller at ics.u-strasbg.fr> writes:
>   At least in pascal language it is quite
> common to use things like 
> type
>   BigArray = Array [1..0xffffffff] of integer;
> if you want to be sure that you will never get into 
> troubles due to range checking...
>
>   Of course you cannot allocate a pointer to such a type
> directly, and it would eat up a lot of memory space.
>   But this kind of types always gave problems
> inside gdb, because when you wanted to look at
> the value gdb would try to copy the whole array...
> even if cases where it would only display the first elements,
> which is kind of silly, no?

Interesting.  I see print_command_1 calls record_latest_value, which
says:

  /* We don't want this value to have anything to do with the inferior anymore.
     In particular, "set $1 = 50" should not affect the variable from which
     the value was taken, and fast watchpoints should be able to assume that
     a value on the value history never changes.  */
  if (value_lazy (val))
    value_fetch_lazy (val);

If GDB were to support those types of values nicely, how would GDB
save such values in the value history without reading the whole thing?

Even if that could be addressed, the common val_print routine expects
to have the whole object in memory.  That's a shame.