Don't inherit range-type signed-ness from underlying type
authorTom Tromey <tromey@adacore.com>
Mon, 26 Oct 2020 18:54:19 +0000 (12:54 -0600)
committerTom Tromey <tromey@adacore.com>
Mon, 26 Oct 2020 19:00:41 +0000 (13:00 -0600)
A recent commit changed gdb to inherit the signed-ness of a range type
from its underlying type:

    commit cfabbd351a174406fd5aa063303f5c8bf9266bbc
    Author: Tom Tromey <tom@tromey.com>
    Date:   Sat Oct 17 11:41:59 2020 -0600

      Make range types inherit signed-ness from base type

This passed testing -- but unfortunately, additional testing at
AdaCore showed that this change was incorrect.  GNAT, at least, can
emit an unsigned range type whose underlying type is signed.

This patch reverts the code change from the above.  I chose not to
reintroduce the FIXME comments, because now we know that they are
incorrect.  Instead, this patch also adds a comment to
create_range_type.

A new test case is included as well.

2020-10-26  Tom Tromey  <tromey@adacore.com>

* gdbtypes.c (create_range_type): Revert previous patch.  Add
comment.

gdb/testsuite/ChangeLog
2020-10-26  Tom Tromey  <tromey@adacore.com>

* gdb.ada/unsigned_range/foo.adb: New file.
* gdb.ada/unsigned_range/pack.adb: New file.
* gdb.ada/unsigned_range/pack.ads: New file.
* gdb.ada/unsigned_range.exp: New file.

gdb/ChangeLog
gdb/gdbtypes.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.ada/unsigned_range.exp [new file with mode: 0644]
gdb/testsuite/gdb.ada/unsigned_range/foo.adb [new file with mode: 0644]
gdb/testsuite/gdb.ada/unsigned_range/pack.adb [new file with mode: 0644]
gdb/testsuite/gdb.ada/unsigned_range/pack.ads [new file with mode: 0644]

index 00920974c0b42520513bc700a17964d5124d83fa..43a80fb3b3e8f55393a16c06077b58c27d8de1e2 100644 (file)
@@ -1,3 +1,8 @@
+2020-10-26  Tom Tromey  <tromey@adacore.com>
+
+       * gdbtypes.c (create_range_type): Revert previous patch.  Add
+       comment.
+
 2020-10-26  Pedro Alves  <pedro@palves.net>
 
        * nat/linux-waitpid.c: Include "gdbsupport/eintr.h".
index 33d3a464f5ba4eb944b08c5950f5f0ac74989703..0940fa597fbcd58640aeaa11de787a1a0fb2728a 100644 (file)
@@ -950,7 +950,22 @@ create_range_type (struct type *result_type, struct type *index_type,
 
   result_type->set_bounds (bounds);
 
-  result_type->set_is_unsigned (index_type->is_unsigned ());
+  /* Note that the signed-ness of a range type can't simply be copied
+     from the underlying type.  Consider a case where the underlying
+     type is 'int', but the range type can hold 0..65535, and where
+     the range is further specified to fit into 16 bits.  In this
+     case, if we copy the underlying type's sign, then reading some
+     range values will cause an unwanted sign extension.  So, we have
+     some heuristics here instead.  */
+  if (low_bound->kind () == PROP_CONST && low_bound->const_val () >= 0)
+    result_type->set_is_unsigned (true);
+  /* Ada allows the declaration of range types whose upper bound is
+     less than the lower bound, so checking the lower bound is not
+     enough.  Make sure we do not mark a range type whose upper bound
+     is negative as unsigned.  */
+  if (high_bound->kind () == PROP_CONST && high_bound->const_val () < 0)
+    result_type->set_is_unsigned (false);
+
   result_type->set_endianity_is_not_default
     (index_type->endianity_is_not_default ());
 
index 11b0f5bf2dbbb4eb411ead4e744c169b9154ba21..eff4972b8c8e470ee2a632735778e31b1a985a1e 100644 (file)
@@ -1,3 +1,10 @@
+2020-10-26  Tom Tromey  <tromey@adacore.com>
+
+       * gdb.ada/unsigned_range/foo.adb: New file.
+       * gdb.ada/unsigned_range/pack.adb: New file.
+       * gdb.ada/unsigned_range/pack.ads: New file.
+       * gdb.ada/unsigned_range.exp: New file.
+
 2020-10-26  Tom de Vries  <tdevries@suse.de>
 
        * lib/gdb.exp (INTERNAL_GDBFLAGS): Set heigth and width.
diff --git a/gdb/testsuite/gdb.ada/unsigned_range.exp b/gdb/testsuite/gdb.ada/unsigned_range.exp
new file mode 100644 (file)
index 0000000..476aa8b
--- /dev/null
@@ -0,0 +1,32 @@
+# Copyright 2020 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "BREAK" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+gdb_test "print Value" \
+    [string_to_regexp " = (one => 8000, two => 51000)"]
diff --git a/gdb/testsuite/gdb.ada/unsigned_range/foo.adb b/gdb/testsuite/gdb.ada/unsigned_range/foo.adb
new file mode 100644 (file)
index 0000000..52c669a
--- /dev/null
@@ -0,0 +1,39 @@
+--  Copyright 2020 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 Pack; use Pack;
+
+procedure Foo is
+   type U_W is range 0 .. 65535;
+   for  U_W'Size use 16;
+
+   type R_C is
+   record
+      One : U_W;
+      Two : U_W;
+   end record;
+
+   for R_C use
+   record at mod 2;
+      One at 0 range  0 .. 15;
+      Two at 2 range  0 .. 15;
+   end record;
+   for R_C'size use 2*16;
+
+   Value: R_C := (One => 8000, Two => 51000);
+
+begin
+   Do_Nothing (Value'Address); --  BREAK
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/unsigned_range/pack.adb b/gdb/testsuite/gdb.ada/unsigned_range/pack.adb
new file mode 100644 (file)
index 0000000..626ea60
--- /dev/null
@@ -0,0 +1,23 @@
+--  Copyright 2020 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 Pack is
+
+   procedure Do_Nothing (A : System.Address) is
+   begin
+      null;
+   end Do_Nothing;
+
+end Pack;
diff --git a/gdb/testsuite/gdb.ada/unsigned_range/pack.ads b/gdb/testsuite/gdb.ada/unsigned_range/pack.ads
new file mode 100644 (file)
index 0000000..f744c53
--- /dev/null
@@ -0,0 +1,19 @@
+--  Copyright 2020 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 Pack is
+   procedure Do_Nothing (A : System.Address);
+end Pack;