Re: [patch] fix exp/12117

[Doug Evans <dje@google.com>]

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

On Wed, Oct 13, 2010 at 12:03 PM, Tom Tromey <tromey@redhat.com> wrote:
> 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.

I'm aware of that.

How about this?
[testsuite results not in yet]

2010-10-13  Doug Evans  <dje@google.com>

        PR exp/12117
        * c-typeprint.c (c_type_print_base, case TYPE_CODE_TYPEDEF):
Verify
        assumptions of when this case happens.  Print "<unnamed
typedef>".
        * gdbtypes.c (check_typedef): Clean up function comment.
        Keep track of instance flags as we strip typedefs and create a
new
        type to preserve them if necessary.

        testsuite/
        * gdb.cp/pr12117.cc: New file.
        * gdb.cp/pr12117.exp: New file.

[-- Attachment #2: gdb-101013-12117-2.patch.txt --]
[-- Type: text/plain, Size: 7062 bytes --]

2010-10-13  Doug Evans  <dje@google.com>

	PR exp/12117
	* c-typeprint.c (c_type_print_base, case TYPE_CODE_TYPEDEF): Verify
	assumptions of when this case happens.  Print "<unnamed typedef>".
	* gdbtypes.c (check_typedef): Clean up function comment.
	Keep track of instance flags as we strip typedefs and create a new
	type to preserve them if necessary.

	testsuite/
	* gdb.cp/pr12117.cc: New file.
	* gdb.cp/pr12117.exp: New file.

Index: c-typeprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-typeprint.c,v
retrieving revision 1.62
diff -u -p -r1.62 c-typeprint.c
--- c-typeprint.c	13 Oct 2010 15:10:10 -0000	1.62
+++ c-typeprint.c	13 Oct 2010 20:47:13 -0000
@@ -707,6 +707,13 @@ c_type_print_base (struct type *type, st
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_TYPEDEF:
+      /* If we get here, the typedef doesn't have a name, and we couldn't
+	 resolve TYPE_TARGET_TYPE.  Not much we can do.  */
+      gdb_assert (TYPE_NAME (type) == NULL);
+      gdb_assert (TYPE_TARGET_TYPE (type) == NULL);
+      fprintf_filtered (stream, _("<unnamed typedef>"));
+      break;
+
     case TYPE_CODE_ARRAY:
     case TYPE_CODE_PTR:
     case TYPE_CODE_MEMBERPTR:
Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.201
diff -u -p -r1.201 gdbtypes.c
--- gdbtypes.c	12 Oct 2010 20:58:17 -0000	1.201
+++ gdbtypes.c	13 Oct 2010 20:47:13 -0000
@@ -1347,7 +1347,17 @@ stub_noname_complaint (void)
   complaint (&symfile_complaints, _("stub type has NULL name"));
 }
 
-/* Added by Bryan Boreham, Kewill, Sun Sep 17 18:07:17 1989.
+/* Find the real type of TYPE.  This function returns the real type,
+   after removing all layers of typedefs and completing opaque or stub
+   types.  Completion changes the TYPE argument, but stripping of
+   typedefs does not.
+
+   NOTE: This will return a typedef if TYPE_TARGET_TYPE for the typedef has
+   not been computed and we're either in the middle of reading symbols, or
+   there was no name for the typedef in the debug info.
+
+   If TYPE is a TYPE_CODE_TYPEDEF, its length is updated to the length of
+   the target type.
 
    If this is a stubbed struct (i.e. declared as struct foo *), see if
    we can find a full definition in some other file. If so, copy this
@@ -1355,26 +1365,16 @@ stub_noname_complaint (void)
    (but not any code) that if we don't find a full definition, we'd
    set a flag so we don't spend time in the future checking the same
    type.  That would be a mistake, though--we might load in more
-   symbols which contain a full definition for the type.
-
-   This used to be coded as a macro, but I don't think it is called 
-   often enough to merit such treatment.
-
-   Find the real type of TYPE.  This function returns the real type,
-   after removing all layers of typedefs and completing opaque or stub
-   types.  Completion changes the TYPE argument, but stripping of
-   typedefs does not.
-
-   If TYPE is a TYPE_CODE_TYPEDEF, its length is (also) set to the length of
-   the target type instead of zero.  However, in the case of TYPE_CODE_TYPEDEF
-   check_typedef can still return different type than the original TYPE
-   pointer.  */
+   symbols which contain a full definition for the type.  */
 
 struct type *
 check_typedef (struct type *type)
 {
   struct type *orig_type = type;
   int is_const, is_volatile;
+  /* While we're removing typedefs, we don't want to lose qualifiers.
+     E.g., const/volatile.  */
+  int instance_flags = TYPE_INSTANCE_FLAGS (type);
 
   gdb_assert (type);
 
@@ -1407,8 +1407,16 @@ check_typedef (struct type *type)
 	    TYPE_TARGET_TYPE (type) = alloc_type_arch (get_type_arch (type));
 	}
       type = TYPE_TARGET_TYPE (type);
+      instance_flags |= TYPE_INSTANCE_FLAGS (type);
     }
 
+  /* If necessary, create a new type to preserve any instance flags we lost
+     while stripping typedefs.  */
+  if (instance_flags != TYPE_INSTANCE_FLAGS (type))
+    type = make_qualified_type (type,
+				instance_flags | TYPE_INSTANCE_FLAGS (type),
+				NULL);
+
   is_const = TYPE_CONST (type);
   is_volatile = TYPE_VOLATILE (type);
 
Index: testsuite/gdb.cp/pr12117.cc
===================================================================
RCS file: testsuite/gdb.cp/pr12117.cc
diff -N testsuite/gdb.cp/pr12117.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr12117.cc	13 Oct 2010 20:47:13 -0000
@@ -0,0 +1,17 @@
+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;
+}
Index: testsuite/gdb.cp/pr12117.exp
===================================================================
RCS file: testsuite/gdb.cp/pr12117.exp
diff -N testsuite/gdb.cp/pr12117.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr12117.exp	13 Oct 2010 20:47:13 -0000
@@ -0,0 +1,55 @@
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib "cp-support.exp"
+
+set testfile "pr12117"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+    untested ${testfile}.exp
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+gdb_test "whatis v_my_int" "my_int"
+gdb_test "ptype v_my_int" "int"
+
+gdb_test "whatis v_const_my_int" "const_my_int"
+gdb_test "ptype v_const_my_int" "const int"
+
+gdb_test "whatis v_volatile_my_int" "volatile_my_int"
+gdb_test "ptype v_volatile_my_int" "volatile int"
+
+gdb_test "whatis v_const_volatile_my_int" "const_volatile_my_int"
+gdb_test "ptype v_const_volatile_my_int" "const volatile int"
+
+gdb_test "whatis v_volatile_const_my_int" "volatile_const_my_int"
+setup_kfail "gcc/45997" *-*-*
+gdb_test "ptype v_volatile_const_my_int" "const volatile int"