Re: [patch] fix exp/12117

[Jan Kratochvil <jan.kratochvil@redhat.com>]

Hi,

I see your similar patch and I had this one prepared so I rather post it.

I also do not yet have the testsuite results.


On Wed, 13 Oct 2010 21:03:50 +0200, Tom Tromey wrote:
> >>>>> "Doug" == Doug Evans <dje@google.com> writes:
> 
> Doug> a) it seems like it's not just c/v, e.g., it's also the address space
> Doug> [and perhaps here's a case where there are more bugs in this area :-)]
> 
> Yeah.  Actually, this one seems like it could cause real problems
> somewhere.

I may see it too naively but isn't the attached patch OK?  I see no instance
flags or objfile ownership problem there.


> Doug> b) it seems odd to have to build such types on the fly
> Doug> [I can imagine a proliferation of such objects that aren't attached to
> Doug> anything concrete in the debug info, and just cause confusion]
> 
> I'm not sure I understand.  The qualified variants are all linked
> together and allocated alongside the main_type, so we don't have to
> worry about memory leaks or excess type reinstantiation.  In fact I
> think the user can cause this sort of type allocation by writing "const
> sometype" in an expression.

And the VLA patch creates whole new struct main_type's on the fly so just at
most quadrupling struct type should be IMO OK.

As I would still like to rather simplify the inferior types I would like to
avoid a further complexity there.

Also I do not like much the pr[0-9]+.exp names, one gets fimiliar with any
name but I never remember PR numbers.  [in fact an idea by Roland McGrath]

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.


gdb/
2010-10-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdbtypes.c (check_typedef): Initialize is_const and is_Variable
	first.  Binary or them with all dereferenced TYPE_TARGET_TYPEs.  Call
	make_cv_type with is_const and is_volatile for any returned type.

gdb/testsuite/
2010-10-13  Doug Evans  <dje@google.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.cp/ptype-cv-cp.cc: New files.
	* gdb.cp/ptype-cv-cp.exp: New files.

--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1374,7 +1374,8 @@ struct type *
 check_typedef (struct type *type)
 {
   struct type *orig_type = type;
-  int is_const, is_volatile;
+  int is_const = TYPE_CONST (type);
+  int is_volatile = TYPE_VOLATILE (type);
 
   gdb_assert (type);
 
@@ -1388,7 +1389,7 @@ check_typedef (struct type *type)
 	  /* It is dangerous to call lookup_symbol if we are currently
 	     reading a symtab.  Infinite recursion is one danger.  */
 	  if (currently_reading_symtab)
-	    return type;
+	    return make_cv_type (is_const, is_volatile, type, NULL);
 
 	  name = type_name_no_tag (type);
 	  /* FIXME: shouldn't we separately check the TYPE_NAME and
@@ -1398,7 +1399,7 @@ check_typedef (struct type *type)
 	  if (name == NULL)
 	    {
 	      stub_noname_complaint ();
-	      return type;
+	      return make_cv_type (is_const, is_volatile, type, NULL);
 	    }
 	  sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
 	  if (sym)
@@ -1407,11 +1408,10 @@ check_typedef (struct type *type)
 	    TYPE_TARGET_TYPE (type) = alloc_type_arch (get_type_arch (type));
 	}
       type = TYPE_TARGET_TYPE (type);
+      is_const |= TYPE_CONST (type);
+      is_volatile |= TYPE_VOLATILE (type);
     }
 
-  is_const = TYPE_CONST (type);
-  is_volatile = TYPE_VOLATILE (type);
-
   /* If this is a struct/class/union with no fields, then check
      whether a full definition exists somewhere else.  This is for
      systems where a type definition with no fields is issued for such
@@ -1428,7 +1428,7 @@ check_typedef (struct type *type)
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_cv_type (is_const, is_volatile, type, NULL);
 	}
       newtype = lookup_transparent_type (name);
 
@@ -1464,7 +1464,7 @@ check_typedef (struct type *type)
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_cv_type (is_const, is_volatile, type, NULL);
 	}
       sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
       if (sym)
@@ -1535,6 +1535,7 @@ check_typedef (struct type *type)
 	}
     }
   /* Cache TYPE_LENGTH for future use.  */
+  type = make_cv_type (is_const, is_volatile, type, NULL);
   TYPE_LENGTH (orig_type) = TYPE_LENGTH (type);
   return type;
 }
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/ptype-cv-cp.cc
@@ -0,0 +1,34 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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/>.  */
+
+typedef int my_int;
+typedef const my_int const_my_int;
+typedef volatile my_int volatile_my_int;
+typedef volatile const_my_int volatile_const_my_int;
+typedef const volatile_my_int const_volatile_my_int;
+
+my_int v_my_int (0);
+const_my_int v_const_my_int (1);
+volatile_my_int v_volatile_my_int (2);
+const_volatile_my_int v_const_volatile_my_int (3);
+volatile_const_my_int v_volatile_const_my_int (4);
+
+int
+main ()
+{
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/ptype-cv-cp.exp
@@ -0,0 +1,36 @@
+# Copyright 2010 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 { [skip_cplus_tests] } { continue }
+
+if { [prepare_for_testing ptype-cv-cp.exp "ptype-cv-cp" ptype-cv-cp.cc {debug c++}] } {
+    return -1
+}
+
+gdb_test "whatis v_my_int" "type = my_int"
+gdb_test "ptype v_my_int" "type = int"
+
+gdb_test "whatis v_const_my_int" "type = const_my_int"
+gdb_test "ptype v_const_my_int" "type = const int"
+
+gdb_test "whatis v_volatile_my_int" "type = volatile_my_int"
+gdb_test "ptype v_volatile_my_int" "type = volatile int"
+
+gdb_test "whatis v_const_volatile_my_int" "type = const_volatile_my_int"
+gdb_test "ptype v_const_volatile_my_int" "type = const volatile int"
+
+gdb_test "whatis v_volatile_const_my_int" "type = volatile_const_my_int"
+setup_kfail "gcc/45997" *-*-*
+gdb_test "ptype v_volatile_const_my_int" "type = const volatile int"