[commit/Ada] Fix unconstrained packed array size

[commit/Ada] Fix unconstrained packed array size

[Joel Brobecker]

When ada-lang transforms an array descriptor type (an XUP structure)
into an array type, the size of the array type is computed by using
the element size, and multiplying it by the number of elements in
that array. This does not work, however, for packed arrays, where
the *packed* size in bits needs to be used.

This usually does not cause any problem, because we end up reading
more memory than needed.  However, we have observed on LynxOS
a memory error while trying to read the entire array, because
the larger-than-needed read tried to read past the end of the stack
into inaccessible memory.

This patch fixes the problem by correctly computing the array size
in bytes in the case of packed arrays.

gdb/ChangeLog:

        * ada-lang.c (ada_type_of_array): Fix the size of the array
        in the case of an unconstrained packed array.

gdb/testsuite/ChangeLog:

        * gdb.ada/packed_array: Expand testcase to test printing of
        unconstrained packed array.

Tested on x86_64-linux, checked in.

---
 gdb/ChangeLog                              |    5 +++++
 gdb/ada-lang.c                             |   21 +++++++++++++++++++--
 gdb/testsuite/ChangeLog                    |    5 +++++
 gdb/testsuite/gdb.ada/packed_array.exp     |    6 ++++++
 gdb/testsuite/gdb.ada/packed_array/pa.adb  |   10 ++++++++++
 gdb/testsuite/gdb.ada/packed_array/pck.adb |   22 ++++++++++++++++++++++
 gdb/testsuite/gdb.ada/packed_array/pck.ads |   19 +++++++++++++++++++
 7 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/packed_array/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/packed_array/pck.ads

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 16add3b..09e1248 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2011-02-17  Joel Brobecker  <brobecker@adacore.com>
+
+	* ada-lang.c (ada_type_of_array): Fix the size of the array
+	in the case of an unconstrained packed array.
+
 2011-02-17  Yao Qi  <yao@codesourcery.com>
 
 	* common/Makefile.in: Add more targets for make.
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 890e091..467e4df 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -1823,8 +1823,25 @@ ada_type_of_array (struct value *arr, int bounds)
           elt_type = create_array_type (array_type, elt_type, range_type);
 
 	  if (ada_is_unconstrained_packed_array_type (value_type (arr)))
-	    TYPE_FIELD_BITSIZE (elt_type, 0) =
-	      decode_packed_array_bitsize (value_type (arr));
+	    {
+	      /* We need to store the element packed bitsize, as well as
+	         recompute the array size, because it was previously
+		 computed based on the unpacked element size.  */
+	      LONGEST lo = value_as_long (low);
+	      LONGEST hi = value_as_long (high);
+
+	      TYPE_FIELD_BITSIZE (elt_type, 0) =
+		decode_packed_array_bitsize (value_type (arr));
+	      /* If the array has no element, then the size is already
+	         zero, and does not need to be recomputed.  */
+	      if (lo < hi)
+		{
+		  int array_bitsize =
+		        (hi - lo + 1) * TYPE_FIELD_BITSIZE (elt_type, 0);
+
+		  TYPE_LENGTH (array_type) = (array_bitsize + 7) / 8;
+		}
+	    }
         }
 
       return lookup_pointer_type (elt_type);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e3315ad..bcafab8 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2011-02-17  Joel Brobecker  <brobecker@adacore.com>
 
+	* gdb.ada/packed_array: Expand testcase to test printing of
+	unconstrained packed array.
+
+2011-02-17  Joel Brobecker  <brobecker@adacore.com>
+
 	* gdb.dwarf2/dw2-ranges.exp: Simplify using clean_restart.
 
 2011-02-16  Pedro Alves  <pedro@codesourcery.com>
diff --git a/gdb/testsuite/gdb.ada/packed_array.exp b/gdb/testsuite/gdb.ada/packed_array.exp
index 1789aab..d7885bb 100644
--- a/gdb/testsuite/gdb.ada/packed_array.exp
+++ b/gdb/testsuite/gdb.ada/packed_array.exp
@@ -50,3 +50,9 @@ gdb_test "ptype &var" \
 gdb_test "print &var" \
          "= \\(access array \\(\\.\\.\\.\\) of boolean\\) \\(4 => true, false, true, false, true\\)" \
          "print &var"
+
+# Print the value of U_Var, an unconstrainted packed array.
+
+gdb_test "print u_var" \
+         "= \\(true, false, false, true, true, false\\)"
+
diff --git a/gdb/testsuite/gdb.ada/packed_array/pa.adb b/gdb/testsuite/gdb.ada/packed_array/pa.adb
index fbe681d..ca60c2d 100644
--- a/gdb/testsuite/gdb.ada/packed_array/pa.adb
+++ b/gdb/testsuite/gdb.ada/packed_array/pa.adb
@@ -14,6 +14,8 @@
 --  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 PA is
 
    type Packed_Array is array (4 .. 8) of Boolean;
@@ -21,9 +23,17 @@ procedure PA is
 
    Var : Packed_Array;
 
+   --  Unconstrained packed array (bounds are dynamic).
+   type Unconstrained_Packed_Array is array (Integer range <>) of Boolean;
+
+   U_Var : Unconstrained_Packed_Array (1 .. Ident (6));
+
 begin
 
    Var := (True, False, True, False, True);
+   U_Var := (True, False, False, True, True, False);
+
    Var (8) := False;  -- STOP
+   U_Var (U_Var'Last) := True;
 
 end PA;
diff --git a/gdb/testsuite/gdb.ada/packed_array/pck.adb b/gdb/testsuite/gdb.ada/packed_array/pck.adb
new file mode 100644
index 0000000..b396524
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/packed_array/pck.adb
@@ -0,0 +1,22 @@
+--  Copyright (C) 2011
+--  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;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/packed_array/pck.ads b/gdb/testsuite/gdb.ada/packed_array/pck.ads
new file mode 100644
index 0000000..87db78c
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/packed_array/pck.ads
@@ -0,0 +1,19 @@
+--  Copyright (C) 2011
+--  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 Pck is
+   function Ident (I : Integer) return Integer;
+end Pck;
-- 
1.7.1

Re: [commit/Ada] Fix unconstrained packed array size

[Jan Kratochvil]

On Thu, 17 Feb 2011 07:43:56 +0100, Joel Brobecker wrote:
> This patch fixes the problem by correctly computing the array size
> in bytes in the case of packed arrays.

FYI:
print u_var
$3 = (warning: unable to get bounds of array, assuming null array
)
(gdb) FAIL: gdb.ada/packed_array.exp: print u_var

GNU gdb (GDB) 7.2.50.20110217-cvs
FAIL GNAT 4.6.0 20110212 (experimental)
FAIL GNAT 4.5.3 20110124 (prerelease)
FAIL GNAT 4.4.6 20110124 (prerelease)


Thanks,
Jan

Re: [commit/Ada] Fix unconstrained packed array size

[Joel Brobecker]

> FYI:
> print u_var
> $3 = (warning: unable to get bounds of array, assuming null array
> )
> (gdb) FAIL: gdb.ada/packed_array.exp: print u_var

I was afraid something like that might happen. I am pretty sure
it is another one of these where the compiler is emitting incomplete
debugging info (it works for me). The problem is that our attempts
at improving the compiler are conditional on one patch that just
we just can't get anyone on gcc-patches to review :-(.  I'll try
to find the message and ping again.

In the meantime, I still think it's worthwhile for the community
if I add testcases whenever I can. But it's extra work for me
that I then do not use (AdaCore has its own testsuite), so if
it's felt as a nuisance, then let me know, and I will stop.

-- 
Joel

Re: [commit/Ada] Fix unconstrained packed array size

[Jan Kratochvil]

On Fri, 18 Feb 2011 04:24:26 +0100, Joel Brobecker wrote:
> I was afraid something like that might happen. I am pretty sure
> it is another one of these where the compiler is emitting incomplete
> debugging info (it works for me).

I expected something like that.  In such case there should be XFAIL (or maybe
a different pseudo-FAIL variant but XFAIL seems OK to me) like we did in:
	Re: Regression on gdb.ada/null_array.exp [Re: [patch] DW_AT_byte_size for array type entries]
	http://sourceware.org/ml/gdb-patches/2010-11/msg00093.html

Could you best describe the difference - probably in readelf -wi output - that
can be checked by the .exp file for XFAIL?


> In the meantime, I still think it's worthwhile for the community
> if I add testcases whenever I can.

Yes, I find that part fine when we do not necessarily even increase the number
of FAILs messing up all the various testsuite cross-checks.


Thanks,
Jan

Re: [commit/Ada] Fix unconstrained packed array size

[Joel Brobecker]

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

> I expected something like that.  In such case there should be XFAIL (or maybe
> a different pseudo-FAIL variant but XFAIL seems OK to me) like we did in:

Can you try the following? Looking at the output, providing the readelf
output is just going to clutter the testcase, IMO, so I left it out.
The ___XA type is missing, it's not malformed, and there isn't much
to add (again, IMO).

Thanks,
-- 
Joel

[-- Attachment #2: packed-array-xfail.diff --]
[-- Type: text/x-diff, Size: 1262 bytes --]

commit 182688b2f7709c2ea2e5963d4040aaf47b88c492
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Mon Feb 21 12:17:39 2011 +0400

    add xfail for "print u_var" test in gdb.ada/packed_array.exp
    
    gdb/testsuite/ChangeLog:
    
            * gdb.ada/packed_array.exp: Add xfail for "print u_var" if
            the debugger is unable to find the array bounds.

diff --git a/gdb/testsuite/gdb.ada/packed_array.exp b/gdb/testsuite/gdb.ada/packed_array.exp
index d7885bb..fc4dd17 100644
--- a/gdb/testsuite/gdb.ada/packed_array.exp
+++ b/gdb/testsuite/gdb.ada/packed_array.exp
@@ -53,6 +53,16 @@ gdb_test "print &var" \
 
 # Print the value of U_Var, an unconstrainted packed array.
 
-gdb_test "print u_var" \
-         "= \\(true, false, false, true, true, false\\)"
+set test "print u_var"
+gdb_test_multiple "$test" "$test" {
+    -re "= \\(true, false, false, true, true, false\\)\[\r\n\]+$gdb_prompt $" {
+        pass $test
+    }
+    -re "= \\(warning: unable to get bounds of array.*\\)\[\r\n\]+$gdb_prompt $" {
+        # The compiler forgot to emit the packed array's ___XA type,
+        # preventing us from determining the what the array bounds
+        # are.  Observed with (FSF GNU Ada 4.5.3 20110124).
+        xfail $test
+    }
+}
 

Re: [commit/Ada] Fix unconstrained packed array size

[Jan Kratochvil]

On Mon, 21 Feb 2011 09:21:21 +0100, Joel Brobecker wrote:
> > I expected something like that.  In such case there should be XFAIL (or maybe
> > a different pseudo-FAIL variant but XFAIL seems OK to me) like we did in:
> 
> Can you try the following? Looking at the output, providing the readelf
> output is just going to clutter the testcase, IMO, so I left it out.

OK, your choice.

> The ___XA type is missing, it's not malformed, and there isn't much
> to add (again, IMO).
[...]
>     gdb/testsuite/ChangeLog:
>     
>             * gdb.ada/packed_array.exp: Add xfail for "print u_var" if
>             the debugger is unable to find the array bounds.

OK, I find it better this way.


Thanks,
Jan

Re: [commit/Ada] Fix unconstrained packed array size

[Joel Brobecker]

> >     gdb/testsuite/ChangeLog:
> >     
> >             * gdb.ada/packed_array.exp: Add xfail for "print u_var" if
> >             the debugger is unable to find the array bounds.
> 
> OK, I find it better this way.

Thanks (and thanks for sending the binaries, allowing me to reproduce
the problem). The patch is now in.

-- 
Joel