Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] fix exp/12117
@ 2010-10-13 17:53 Doug Evans
  2010-10-13 18:03 ` Doug Evans
  2010-10-13 18:44 ` Tom Tromey
  0 siblings, 2 replies; 12+ messages in thread
From: Doug Evans @ 2010-10-13 17:53 UTC (permalink / raw)
  To: gdb-patches

Hi.

This patch fixes pr exp/12117.
http://sourceware.org/bugzilla/show_bug.cgi?id=12117

I will check this in in two days if there are no objections.

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

	PR exp/12117
	* c-typeprint.c (c_print_type): Call CHECK_TYPEDEF after calling
	c_type_print_base.
	(c_type_print_modifier_flags): New function.
	(c_type_print_modifier): Call it.
	(c_type_print_base): Collect instance_flags while we're stripping
	typedefs.  Change calls to c_type_print_modifier to call
	c_type_print_modifier_flags instead.
	(c_type_print_base, case TYPE_CODE_TYPEDEF): Verify assumptions of
	when this case happens.  Print "<unnamed typedef>".
	* gdbtypes.h (strip_typedefs): Declare.
	* gdbtypes.c (strip_typedefs): New function.
	(check_typedef): Call it.

	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 17:29:30 -0000
@@ -53,10 +53,13 @@ c_print_type (struct type *type, const c
   int demangled_args;
   int need_post_space;
 
+  c_type_print_base (type, stream, show, level);
+
+  /* Do this *after* calling c_type_print_base so that it can see any
+     const/volatile flags that may be encoded in a typedef.  */
   if (show > 0)
     CHECK_TYPEDEF (type);
 
-  c_type_print_base (type, stream, show, level);
   code = TYPE_CODE (type);
   if ((varstring != NULL && *varstring != '\0')
   /* Need a space if going to print stars or brackets;
@@ -313,23 +316,20 @@ c_type_print_varspec_prefix (struct type
 
 /* Print out "const" and "volatile" attributes,
    and address space id if present.
-   TYPE is a pointer to the type being printed out.
+   FLAGS is the TYPE_INSTANCE_FLAGS flags of the type.
    STREAM is the output destination.
    NEED_PRE_SPACE = 1 indicates an initial white space is needed.
    NEED_POST_SPACE = 1 indicates a final white space is needed.  */
 
 static void
-c_type_print_modifier (struct type *type, struct ui_file *stream,
-		       int need_pre_space, int need_post_space)
+c_type_print_modifier_flags (int flags, struct ui_file *stream,
+			     struct gdbarch *gdbarch,
+			     int need_pre_space, int need_post_space)
 {
   int did_print_modifier = 0;
   const char *address_space_id;
 
-  /* We don't print `const' qualifiers for references --- since all
-     operators affect the thing referenced, not the reference itself,
-     every reference is `const'.  */
-  if (TYPE_CONST (type)
-      && TYPE_CODE (type) != TYPE_CODE_REF)
+  if (flags & TYPE_INSTANCE_FLAG_CONST)
     {
       if (need_pre_space)
 	fprintf_filtered (stream, " ");
@@ -337,7 +337,7 @@ c_type_print_modifier (struct type *type
       did_print_modifier = 1;
     }
 
-  if (TYPE_VOLATILE (type))
+  if (flags & TYPE_INSTANCE_FLAG_VOLATILE)
     {
       if (did_print_modifier || need_pre_space)
 	fprintf_filtered (stream, " ");
@@ -345,8 +345,7 @@ c_type_print_modifier (struct type *type
       did_print_modifier = 1;
     }
 
-  address_space_id = address_space_int_to_name (get_type_arch (type),
-						TYPE_INSTANCE_FLAGS (type));
+  address_space_id = address_space_int_to_name (gdbarch, flags);
   if (address_space_id)
     {
       if (did_print_modifier || need_pre_space)
@@ -359,6 +358,24 @@ c_type_print_modifier (struct type *type
     fprintf_filtered (stream, " ");
 }
 
+/* Wrapper for c_type_print_modifier_flags.
+   Print the modifier flags for TYPE.  */
+
+static void
+c_type_print_modifier (struct type *type, struct ui_file *stream,
+		       int need_pre_space, int need_post_space)
+{
+  int instance_flags = TYPE_INSTANCE_FLAGS (type);
+
+  /* We don't print `const' qualifiers for references --- since all
+     operators affect the thing referenced, not the reference itself,
+     every reference is `const'.  */
+  if (TYPE_CODE (type) == TYPE_CODE_REF)
+    instance_flags &= ~TYPE_INSTANCE_FLAG_CONST;
+
+  c_type_print_modifier_flags (instance_flags, stream, get_type_arch (type),
+			       need_pre_space, need_post_space);
+}
 
 /* Print out the arguments of TYPE, which should have TYPE_CODE_METHOD
    or TYPE_CODE_FUNC, to STREAM.  Artificial arguments, such as "this"
@@ -678,6 +695,8 @@ c_type_print_base (struct type *type, st
   section_type;
   int need_access_label = 0;
   int j, len2;
+  int instance_flags;
+  struct gdbarch *gdbarch;
 
   QUIT;
 
@@ -702,11 +721,33 @@ c_type_print_base (struct type *type, st
       return;
     }
 
+  /* Handle typedefs specially because we want to strip them, but as we're
+     stripping them we need to keep track of any instance-related flags,
+     e.g., const/volatile.  Furthermore, we would like to print the flags
+     once, and in a consistent order, so just accumulate them as we're
+     stripping.  */
+  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
+    type = strip_typedefs (type, &instance_flags);
+  else
+    instance_flags = 0;
+
+  /* We've already stripped typedefs, but we still need to try to
+     update stubbed structs, etc.  */
   CHECK_TYPEDEF (type);
 
+  instance_flags |= TYPE_INSTANCE_FLAGS (type);
+  gdbarch = get_type_arch (type);
+
   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:
@@ -718,7 +759,7 @@ c_type_print_base (struct type *type, st
       break;
 
     case TYPE_CODE_STRUCT:
-      c_type_print_modifier (type, stream, 0, 1);
+      c_type_print_modifier_flags (instance_flags, stream, gdbarch, 0, 1);
       if (TYPE_DECLARED_CLASS (type))
 	fprintf_filtered (stream, "class ");
       else
@@ -726,7 +767,7 @@ c_type_print_base (struct type *type, st
       goto struct_union;
 
     case TYPE_CODE_UNION:
-      c_type_print_modifier (type, stream, 0, 1);
+      c_type_print_modifier_flags (instance_flags, stream, gdbarch, 0, 1);
       fprintf_filtered (stream, "union ");
 
     struct_union:
@@ -1083,7 +1124,7 @@ c_type_print_base (struct type *type, st
       break;
 
     case TYPE_CODE_ENUM:
-      c_type_print_modifier (type, stream, 0, 1);
+      c_type_print_modifier_flags (instance_flags, stream, gdbarch, 0, 1);
       fprintf_filtered (stream, "enum ");
       /* Print the tag name if it exists.
          The aCC compiler emits a spurious 
@@ -1157,7 +1198,7 @@ c_type_print_base (struct type *type, st
          is no type name, then complain. */
       if (TYPE_NAME (type) != NULL)
 	{
-	  c_type_print_modifier (type, stream, 0, 1);
+	  c_type_print_modifier_flags (instance_flags, stream, gdbarch, 0, 1);
 	  fputs_filtered (TYPE_NAME (type), stream);
 	}
       else
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 17:29:30 -0000
@@ -1347,36 +1347,21 @@ stub_noname_complaint (void)
   complaint (&symfile_complaints, _("stub type has NULL name"));
 }
 
-/* Added by Bryan Boreham, Kewill, Sun Sep 17 18:07:17 1989.
+/* Strip all typedefs from TYPE.
+   Return the real type if possible.
 
-   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
-   definition, so we can use it in future.  There used to be a comment
-   (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.
+   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.
 
-   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.  */
+   Any instance flags (e.g. const/volatile/address-space) that were stripped
+   away are stored in *INSTANCE_FLAGSP if non-NULL.  */
 
 struct type *
-check_typedef (struct type *type)
+strip_typedefs (struct type *type, int *instance_flagsp)
 {
-  struct type *orig_type = type;
-  int is_const, is_volatile;
-
-  gdb_assert (type);
+  if (instance_flagsp != NULL)
+    *instance_flagsp = 0;
 
   while (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
     {
@@ -1406,9 +1391,50 @@ check_typedef (struct type *type)
 	  else					/* TYPE_CODE_UNDEF */
 	    TYPE_TARGET_TYPE (type) = alloc_type_arch (get_type_arch (type));
 	}
+
+      if (instance_flagsp != NULL)
+	*instance_flagsp |= TYPE_INSTANCE_FLAGS (type);
+
       type = TYPE_TARGET_TYPE (type);
     }
 
+  return type;
+}
+
+/* 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
+   definition, so we can use it in future.  There used to be a comment
+   (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.  */
+
+struct type *
+check_typedef (struct type *type)
+{
+  struct type *orig_type = type;
+  int is_const, is_volatile;
+
+  gdb_assert (type);
+
+  type = strip_typedefs (type, NULL);
+
+  /* If we still have a typedef, there's nothing more we can do.  */
+  if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
+    return type;
+
   is_const = TYPE_CONST (type);
   is_volatile = TYPE_VOLATILE (type);
 
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.135
diff -u -p -r1.135 gdbtypes.h
--- gdbtypes.h	6 Oct 2010 08:44:14 -0000	1.135
+++ gdbtypes.h	13 Oct 2010 17:29:30 -0000
@@ -1361,6 +1361,8 @@ extern struct type *check_typedef (struc
     (TYPE) = check_typedef (TYPE);		\
   } while (0)
 
+extern struct type *strip_typedefs (struct type *, int *);
+
 extern void check_stub_method_group (struct type *, int);
 
 extern char *gdb_mangle_name (struct type *, int, int);
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 17:29:31 -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 17:29:31 -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"


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 17:53 [patch] fix exp/12117 Doug Evans
@ 2010-10-13 18:03 ` Doug Evans
  2010-10-13 18:44 ` Tom Tromey
  1 sibling, 0 replies; 12+ messages in thread
From: Doug Evans @ 2010-10-13 18:03 UTC (permalink / raw)
  To: gdb-patches

On Wed, Oct 13, 2010 at 10:53 AM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> This patch fixes pr exp/12117.
> http://sourceware.org/bugzilla/show_bug.cgi?id=12117
>
> I will check this in in two days if there are no objections.
>
> 2010-10-13  Doug Evans  <dje@google.com>
>
>        PR exp/12117
>        * c-typeprint.c (c_print_type): Call CHECK_TYPEDEF after calling
>        c_type_print_base.
>        (c_type_print_modifier_flags): New function.
>        (c_type_print_modifier): Call it.
>        (c_type_print_base): Collect instance_flags while we're stripping
>        typedefs.  Change calls to c_type_print_modifier to call
>        c_type_print_modifier_flags instead.
>        (c_type_print_base, case TYPE_CODE_TYPEDEF): Verify assumptions of
>        when this case happens.  Print "<unnamed typedef>".
>        * gdbtypes.h (strip_typedefs): Declare.
>        * gdbtypes.c (strip_typedefs): New function.
>        (check_typedef): Call it.
>
>        testsuite/
>        * gdb.cp/pr12117.cc: New file.
>        * gdb.cp/pr12117.exp: New file.

Blech, I told myself not to forget adding this note this time ... :-(

Tested on amd64-linux, no regressions.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 17:53 [patch] fix exp/12117 Doug Evans
  2010-10-13 18:03 ` Doug Evans
@ 2010-10-13 18:44 ` Tom Tromey
  2010-10-13 18:55   ` Doug Evans
  1 sibling, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2010-10-13 18:44 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> This patch fixes pr exp/12117.
Doug> http://sourceware.org/bugzilla/show_bug.cgi?id=12117

Doug> I will check this in in two days if there are no objections.

It looks reasonable to me.

Not an objection, but it seems odd that check_typedef can strip
qualifiers from a type.  It looks like it tries to do the right thing in
some cases, but not all.  Couldn't it just call make_qualified_type at
the right point?  Or unconditionally use that instead of checking
TYPE_OBJFILE?

Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 18:44 ` Tom Tromey
@ 2010-10-13 18:55   ` Doug Evans
  2010-10-13 19:04     ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2010-10-13 18:55 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Wed, Oct 13, 2010 at 11:44 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> This patch fixes pr exp/12117.
> Doug> http://sourceware.org/bugzilla/show_bug.cgi?id=12117
>
> Doug> I will check this in in two days if there are no objections.
>
> It looks reasonable to me.
>
> Not an objection, but it seems odd that check_typedef can strip
> qualifiers from a type.  It looks like it tries to do the right thing in
> some cases, but not all.  Couldn't it just call make_qualified_type at
> the right point?  Or unconditionally use that instead of checking
> TYPE_OBJFILE?

No disagreement that it's odd, and there may yet be more bugs in this area.
I thought of calling make_qualified_type, but I wasn't entirely
comfortable with it.
a) it seems like it's not just c/v, e.g., it's also the address space
[and perhaps here's a case where there are more bugs in this area :-)]
b) it seems odd to have to build such types on the fly
[I can imagine a proliferation of such objects that aren't attached to
anything concrete in the debug info, and just cause confusion]


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 18:55   ` Doug Evans
@ 2010-10-13 19:04     ` Tom Tromey
  2010-10-13 20:55       ` Doug Evans
  2010-10-13 21:10       ` Jan Kratochvil
  0 siblings, 2 replies; 12+ messages in thread
From: Tom Tromey @ 2010-10-13 19:04 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

>>>>> "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.

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.

Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 19:04     ` Tom Tromey
@ 2010-10-13 20:55       ` Doug Evans
  2010-10-13 21:10       ` Jan Kratochvil
  1 sibling, 0 replies; 12+ messages in thread
From: Doug Evans @ 2010-10-13 20:55 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

[-- 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"

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 19:04     ` Tom Tromey
  2010-10-13 20:55       ` Doug Evans
@ 2010-10-13 21:10       ` Jan Kratochvil
  2010-10-14  1:55         ` Doug Evans
  2010-10-15 17:41         ` Tom Tromey
  1 sibling, 2 replies; 12+ messages in thread
From: Jan Kratochvil @ 2010-10-13 21:10 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Doug Evans, gdb-patches

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"


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 21:10       ` Jan Kratochvil
@ 2010-10-14  1:55         ` Doug Evans
  2010-10-14  9:35           ` Jan Kratochvil
  2010-10-15 17:41         ` Tom Tromey
  1 sibling, 1 reply; 12+ messages in thread
From: Doug Evans @ 2010-10-14  1:55 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches

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

On Wed, Oct 13, 2010 at 2:10 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> 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]

I often just go with the flow for things such as this, in the absence
of specific rules.
If we want to declare a specific rule of no more such prNNN naming,
that's fine by me.

> @@ -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

I left this out for a later pass to be conservative and avoid having
to deal with any unintended consequences.
But if one wants to handle this in the same patch, ok.

> @@ -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)

Ditto.

> @@ -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

This part doesn't feel right.
For example, when a stub is later filled in the accumulated cv
qualifiers will get filled in too, but that's wrong.
[Right?]

Plus there's more than just const/volatile to preserve.
[Right?]

I like this patch better.


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

        PR exp/12117
        * 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/ptype-cv-cp.cc: New file.
        * gdb.cp/ptype-cv-cp.exp: New file.

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

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

	PR exp/12117
	* 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/ptype-cv-cp.cc: New file.
	* gdb.cp/ptype-cv-cp.exp: New file.

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	14 Oct 2010 01:44:11 -0000
@@ -1347,7 +1347,21 @@ 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.
+
+   Instance flags (e.g. const/volatile) are preserved as typedefs are
+   stripped.  If necessary a new qualified form of the underlying type
+   is created.
+
+   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 +1369,15 @@ 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);
 
@@ -1388,7 +1391,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_qualified_type (type, instance_flags, NULL);
 
 	  name = type_name_no_tag (type);
 	  /* FIXME: shouldn't we separately check the TYPE_NAME and
@@ -1398,7 +1401,7 @@ check_typedef (struct type *type)
 	  if (name == NULL)
 	    {
 	      stub_noname_complaint ();
-	      return type;
+	      return make_qualified_type (type, instance_flags, NULL);
 	    }
 	  sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
 	  if (sym)
@@ -1407,10 +1410,33 @@ 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);
+      /* Preserve the instance flags as we traverse down the typedef chain.
+
+	 Handling address spaces/classes is nasty, what do we do if there's a
+	 conflict?
+	 E.g., what if an outer typedef marks the type as class_1 and an inner
+	 typedef marks the type as class_2?
+	 This is the wrong place to do such error checking.  We leave it to
+	 the code that created the typedef in the first place to flag the
+	 error.  We just pick the outer address space (akin to letting the
+	 outer cast in a chain of casting win), instead of assuming
+	 "it can't happen".  */
+      {
+	const int ALL_SPACES = (TYPE_INSTANCE_FLAG_CODE_SPACE
+				| TYPE_INSTANCE_FLAG_DATA_SPACE);
+	const int ALL_CLASSES = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;
+	int new_instance_flags = TYPE_INSTANCE_FLAGS (type);
+
+	/* Treat code vs data spaces and address classes separately.  */
+	if ((instance_flags & ALL_SPACES) != 0)
+	  new_instance_flags &= ~ALL_SPACES;
+	if ((instance_flags & ALL_CLASSES) != 0)
+	  new_instance_flags &= ~ALL_CLASSES;
+
+	instance_flags |= new_instance_flags;
+      }
+    }
 
   /* If this is a struct/class/union with no fields, then check
      whether a full definition exists somewhere else.  This is for
@@ -1428,7 +1454,7 @@ check_typedef (struct type *type)
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_qualified_type (type, instance_flags, NULL);
 	}
       newtype = lookup_transparent_type (name);
 
@@ -1445,7 +1471,9 @@ check_typedef (struct type *type)
 	     move over any other types NEWTYPE refers to, which could
 	     be an unbounded amount of stuff.  */
 	  if (TYPE_OBJFILE (newtype) == TYPE_OBJFILE (type))
-	    make_cv_type (is_const, is_volatile, newtype, &type);
+	    type = make_qualified_type (newtype,
+					TYPE_INSTANCE_FLAGS (type),
+					type);
 	  else
 	    type = newtype;
 	}
@@ -1464,17 +1492,18 @@ check_typedef (struct type *type)
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_qualified_type (type, instance_flags, NULL);
 	}
       sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
       if (sym)
         {
           /* Same as above for opaque types, we can replace the stub
-             with the complete type only if they are int the same
+             with the complete type only if they are in the same
              objfile.  */
 	  if (TYPE_OBJFILE (SYMBOL_TYPE(sym)) == TYPE_OBJFILE (type))
-            make_cv_type (is_const, is_volatile, 
-			  SYMBOL_TYPE (sym), &type);
+            type = make_qualified_type (SYMBOL_TYPE (sym),
+					TYPE_INSTANCE_FLAGS (type),
+					type);
 	  else
 	    type = SYMBOL_TYPE (sym);
         }
@@ -1534,8 +1563,12 @@ check_typedef (struct type *type)
 	  TYPE_TARGET_STUB (type) = 0;
 	}
     }
+
+  type = make_qualified_type (type, instance_flags, NULL);
+
   /* Cache TYPE_LENGTH for future use.  */
   TYPE_LENGTH (orig_type) = TYPE_LENGTH (type);
+
   return type;
 }
 
Index: testsuite/gdb.cp/ptype-cv-cp.cc
===================================================================
RCS file: testsuite/gdb.cp/ptype-cv-cp.cc
diff -N testsuite/gdb.cp/ptype-cv-cp.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ptype-cv-cp.cc	14 Oct 2010 00:33:32 -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/ptype-cv-cp.exp
===================================================================
RCS file: testsuite/gdb.cp/ptype-cv-cp.exp
diff -N testsuite/gdb.cp/ptype-cv-cp.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/ptype-cv-cp.exp	14 Oct 2010 00:33:32 -0000
@@ -0,0 +1,41 @@
+# 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 }
+
+set testfile "ptype-cv-cp"
+set srcfile ${testfile}.cc
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+    return -1
+}
+
+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"

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-14  1:55         ` Doug Evans
@ 2010-10-14  9:35           ` Jan Kratochvil
  2010-10-15 17:42             ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2010-10-14  9:35 UTC (permalink / raw)
  To: Doug Evans; +Cc: Tom Tromey, gdb-patches

On Thu, 14 Oct 2010 03:55:05 +0200, Doug Evans wrote:
> This part doesn't feel right.
> For example, when a stub is later filled in the accumulated cv
> qualifiers will get filled in too, but that's wrong.
> [Right?]

Yes, I agree.


> Plus there's more than just const/volatile to preserve.
> [Right?]

Yes, I agree.


> I like this patch better.

testcase: Without the "type = " anchor for example "volatile int" would match
even `const volatile int' output.

Added the gdbtypes.h comment, I did not realize this instance flags
inheritance logic before at least myself.

Added testcase GPL comment.


Thanks,
Jan


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

	PR exp/12117
	* 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.
	* gdbtypes.h (type) <instance_flags>: Extend the comment.

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

--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1347,7 +1347,21 @@ 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.
+
+   Instance flags (e.g. const/volatile) are preserved as typedefs are
+   stripped.  If necessary a new qualified form of the underlying type
+   is created.
+
+   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 +1369,15 @@ 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);
 
@@ -1388,7 +1391,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_qualified_type (type, instance_flags, NULL);
 
 	  name = type_name_no_tag (type);
 	  /* FIXME: shouldn't we separately check the TYPE_NAME and
@@ -1398,7 +1401,7 @@ check_typedef (struct type *type)
 	  if (name == NULL)
 	    {
 	      stub_noname_complaint ();
-	      return type;
+	      return make_qualified_type (type, instance_flags, NULL);
 	    }
 	  sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
 	  if (sym)
@@ -1407,10 +1410,33 @@ 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);
+      /* Preserve the instance flags as we traverse down the typedef chain.
+
+	 Handling address spaces/classes is nasty, what do we do if there's a
+	 conflict?
+	 E.g., what if an outer typedef marks the type as class_1 and an inner
+	 typedef marks the type as class_2?
+	 This is the wrong place to do such error checking.  We leave it to
+	 the code that created the typedef in the first place to flag the
+	 error.  We just pick the outer address space (akin to letting the
+	 outer cast in a chain of casting win), instead of assuming
+	 "it can't happen".  */
+      {
+	const int ALL_SPACES = (TYPE_INSTANCE_FLAG_CODE_SPACE
+				| TYPE_INSTANCE_FLAG_DATA_SPACE);
+	const int ALL_CLASSES = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;
+	int new_instance_flags = TYPE_INSTANCE_FLAGS (type);
+
+	/* Treat code vs data spaces and address classes separately.  */
+	if ((instance_flags & ALL_SPACES) != 0)
+	  new_instance_flags &= ~ALL_SPACES;
+	if ((instance_flags & ALL_CLASSES) != 0)
+	  new_instance_flags &= ~ALL_CLASSES;
+
+	instance_flags |= new_instance_flags;
+      }
+    }
 
   /* If this is a struct/class/union with no fields, then check
      whether a full definition exists somewhere else.  This is for
@@ -1428,7 +1454,7 @@ check_typedef (struct type *type)
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_qualified_type (type, instance_flags, NULL);
 	}
       newtype = lookup_transparent_type (name);
 
@@ -1445,7 +1471,9 @@ check_typedef (struct type *type)
 	     move over any other types NEWTYPE refers to, which could
 	     be an unbounded amount of stuff.  */
 	  if (TYPE_OBJFILE (newtype) == TYPE_OBJFILE (type))
-	    make_cv_type (is_const, is_volatile, newtype, &type);
+	    type = make_qualified_type (newtype,
+					TYPE_INSTANCE_FLAGS (type),
+					type);
 	  else
 	    type = newtype;
 	}
@@ -1464,17 +1492,18 @@ check_typedef (struct type *type)
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_qualified_type (type, instance_flags, NULL);
 	}
       sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
       if (sym)
         {
           /* Same as above for opaque types, we can replace the stub
-             with the complete type only if they are int the same
+             with the complete type only if they are in the same
              objfile.  */
 	  if (TYPE_OBJFILE (SYMBOL_TYPE(sym)) == TYPE_OBJFILE (type))
-            make_cv_type (is_const, is_volatile, 
-			  SYMBOL_TYPE (sym), &type);
+            type = make_qualified_type (SYMBOL_TYPE (sym),
+					TYPE_INSTANCE_FLAGS (type),
+					type);
 	  else
 	    type = SYMBOL_TYPE (sym);
         }
@@ -1534,8 +1563,12 @@ check_typedef (struct type *type)
 	  TYPE_TARGET_STUB (type) = 0;
 	}
     }
+
+  type = make_qualified_type (type, instance_flags, NULL);
+
   /* Cache TYPE_LENGTH for future use.  */
   TYPE_LENGTH (orig_type) = TYPE_LENGTH (type);
+
   return type;
 }
 
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -623,7 +623,14 @@ struct type
   struct type *chain;
 
   /* Flags specific to this instance of the type, indicating where
-     on the ring we are.  */
+     on the ring we are.
+
+     For TYPE_CODE_TYPEDEF the flags of the typedef type should be binary
+     or-ed with the target type, with a special for about address class and
+     space class.  For example if this typedef does not specify any new
+     qualifiers, TYPE_INSTANCE_FLAGS is 0 and the instance flags are
+     completely inherited from the targe type.  No qualifiers can be cleared
+     by the typedef.  See also check_typedef.  */
   int instance_flags;
 
   /* Length of storage for a value of this type.  This is what
--- /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,41 @@
+# 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 }
+
+set testfile "ptype-cv-cp"
+set srcfile ${testfile}.cc
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {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"


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-13 21:10       ` Jan Kratochvil
  2010-10-14  1:55         ` Doug Evans
@ 2010-10-15 17:41         ` Tom Tromey
  1 sibling, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2010-10-15 17:41 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Doug Evans, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.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 :-)]

Tom> Yeah.  Actually, this one seems like it could cause real problems
Tom> somewhere.

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

Sorry, all I meant here is that stripping the address space qualifiers
in check_typedef is likely to be a source of bugs, because gdb calls
check_typedef all over the place, and presumably these bits translate
into some important target-specific difference in how memory is
accessed.

Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-14  9:35           ` Jan Kratochvil
@ 2010-10-15 17:42             ` Tom Tromey
  2010-10-15 17:49               ` Jan Kratochvil
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2010-10-15 17:42 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Doug Evans, gdb-patches

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> --- a/gdb/gdbtypes.h
Jan> +     For TYPE_CODE_TYPEDEF the flags of the typedef type should be binary
Jan> +     or-ed with the target type, with a special for about address class and

"special case"

Jan> +     space class.  For example if this typedef does not specify any new
Jan> +     qualifiers, TYPE_INSTANCE_FLAGS is 0 and the instance flags are
Jan> +     completely inherited from the targe type.  No qualifiers can be cleared

Typo, "target"

Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] fix exp/12117
  2010-10-15 17:42             ` Tom Tromey
@ 2010-10-15 17:49               ` Jan Kratochvil
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kratochvil @ 2010-10-15 17:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Doug Evans, gdb-patches

On Fri, 15 Oct 2010 19:41:50 +0200, Tom Tromey wrote:
> "special case"
> Typo, "target"

Sorry; considering as generally agreed upon so checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-10/msg00093.html

--- src/gdb/ChangeLog	2010/10/15 15:42:50	1.12261
+++ src/gdb/ChangeLog	2010/10/15 17:48:40	1.12262
@@ -1,3 +1,12 @@
+2010-10-15  Doug Evans  <dje@google.com>
+	    Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	PR exp/12117
+	* 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.
+	* gdbtypes.h (type) <instance_flags>: Extend the comment.
+
 2010-10-15  Pierre Muller  <muller@ics.u-strasbg.fr>
 
 	* p-lang.c (is_pascal_string_type): Use TYPE_FIELD_NAME accessor.
--- src/gdb/testsuite/ChangeLog	2010/10/14 16:13:42	1.2480
+++ src/gdb/testsuite/ChangeLog	2010/10/15 17:48:47	1.2481
@@ -1,3 +1,10 @@
+2010-10-13  Doug Evans  <dje@google.com>
+	    Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	PR exp/12117
+	* gdb.cp/ptype-cv-cp.cc: New file.
+	* gdb.cp/ptype-cv-cp.exp: New file.
+
 2010-10-14  Sami Wagiaalla  <swagiaal@redhat.com>
 
 	* gdb.cp/converts.cc: New test program.
--- src/gdb/gdbtypes.c	2010/10/14 16:13:42	1.202
+++ src/gdb/gdbtypes.c	2010/10/15 17:48:47	1.203
@@ -1347,7 +1347,21 @@
   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.
+
+   Instance flags (e.g. const/volatile) are preserved as typedefs are
+   stripped.  If necessary a new qualified form of the underlying type
+   is created.
+
+   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 +1369,15 @@
    (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);
 
@@ -1388,7 +1391,7 @@
 	  /* 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_qualified_type (type, instance_flags, NULL);
 
 	  name = type_name_no_tag (type);
 	  /* FIXME: shouldn't we separately check the TYPE_NAME and
@@ -1398,7 +1401,7 @@
 	  if (name == NULL)
 	    {
 	      stub_noname_complaint ();
-	      return type;
+	      return make_qualified_type (type, instance_flags, NULL);
 	    }
 	  sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
 	  if (sym)
@@ -1407,10 +1410,33 @@
 	    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);
+      /* Preserve the instance flags as we traverse down the typedef chain.
+
+	 Handling address spaces/classes is nasty, what do we do if there's a
+	 conflict?
+	 E.g., what if an outer typedef marks the type as class_1 and an inner
+	 typedef marks the type as class_2?
+	 This is the wrong place to do such error checking.  We leave it to
+	 the code that created the typedef in the first place to flag the
+	 error.  We just pick the outer address space (akin to letting the
+	 outer cast in a chain of casting win), instead of assuming
+	 "it can't happen".  */
+      {
+	const int ALL_SPACES = (TYPE_INSTANCE_FLAG_CODE_SPACE
+				| TYPE_INSTANCE_FLAG_DATA_SPACE);
+	const int ALL_CLASSES = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;
+	int new_instance_flags = TYPE_INSTANCE_FLAGS (type);
+
+	/* Treat code vs data spaces and address classes separately.  */
+	if ((instance_flags & ALL_SPACES) != 0)
+	  new_instance_flags &= ~ALL_SPACES;
+	if ((instance_flags & ALL_CLASSES) != 0)
+	  new_instance_flags &= ~ALL_CLASSES;
+
+	instance_flags |= new_instance_flags;
+      }
+    }
 
   /* If this is a struct/class/union with no fields, then check
      whether a full definition exists somewhere else.  This is for
@@ -1428,7 +1454,7 @@
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_qualified_type (type, instance_flags, NULL);
 	}
       newtype = lookup_transparent_type (name);
 
@@ -1445,7 +1471,9 @@
 	     move over any other types NEWTYPE refers to, which could
 	     be an unbounded amount of stuff.  */
 	  if (TYPE_OBJFILE (newtype) == TYPE_OBJFILE (type))
-	    make_cv_type (is_const, is_volatile, newtype, &type);
+	    type = make_qualified_type (newtype,
+					TYPE_INSTANCE_FLAGS (type),
+					type);
 	  else
 	    type = newtype;
 	}
@@ -1464,17 +1492,18 @@
       if (name == NULL)
 	{
 	  stub_noname_complaint ();
-	  return type;
+	  return make_qualified_type (type, instance_flags, NULL);
 	}
       sym = lookup_symbol (name, 0, STRUCT_DOMAIN, 0);
       if (sym)
         {
           /* Same as above for opaque types, we can replace the stub
-             with the complete type only if they are int the same
+             with the complete type only if they are in the same
              objfile.  */
 	  if (TYPE_OBJFILE (SYMBOL_TYPE(sym)) == TYPE_OBJFILE (type))
-            make_cv_type (is_const, is_volatile, 
-			  SYMBOL_TYPE (sym), &type);
+            type = make_qualified_type (SYMBOL_TYPE (sym),
+					TYPE_INSTANCE_FLAGS (type),
+					type);
 	  else
 	    type = SYMBOL_TYPE (sym);
         }
@@ -1534,8 +1563,12 @@
 	  TYPE_TARGET_STUB (type) = 0;
 	}
     }
+
+  type = make_qualified_type (type, instance_flags, NULL);
+
   /* Cache TYPE_LENGTH for future use.  */
   TYPE_LENGTH (orig_type) = TYPE_LENGTH (type);
+
   return type;
 }
 
--- src/gdb/gdbtypes.h	2010/10/14 16:13:42	1.136
+++ src/gdb/gdbtypes.h	2010/10/15 17:48:47	1.137
@@ -623,7 +623,14 @@
   struct type *chain;
 
   /* Flags specific to this instance of the type, indicating where
-     on the ring we are.  */
+     on the ring we are.
+
+     For TYPE_CODE_TYPEDEF the flags of the typedef type should be binary
+     or-ed with the target type, with a special case for address class and
+     space class.  For example if this typedef does not specify any new
+     qualifiers, TYPE_INSTANCE_FLAGS is 0 and the instance flags are
+     completely inherited from the target type.  No qualifiers can be cleared
+     by the typedef.  See also check_typedef.  */
   int instance_flags;
 
   /* Length of storage for a value of this type.  This is what
--- src/gdb/testsuite/gdb.cp/ptype-cv-cp.cc
+++ src/gdb/testsuite/gdb.cp/ptype-cv-cp.cc	2010-10-15 17:49:13.050888000 +0000
@@ -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;
+}
--- src/gdb/testsuite/gdb.cp/ptype-cv-cp.exp
+++ src/gdb/testsuite/gdb.cp/ptype-cv-cp.exp	2010-10-15 17:49:13.394523000 +0000
@@ -0,0 +1,41 @@
+# 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 }
+
+set testfile "ptype-cv-cp"
+set srcfile ${testfile}.cc
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {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"


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-10-15 17:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-13 17:53 [patch] fix exp/12117 Doug Evans
2010-10-13 18:03 ` Doug Evans
2010-10-13 18:44 ` Tom Tromey
2010-10-13 18:55   ` Doug Evans
2010-10-13 19:04     ` Tom Tromey
2010-10-13 20:55       ` Doug Evans
2010-10-13 21:10       ` Jan Kratochvil
2010-10-14  1:55         ` Doug Evans
2010-10-14  9:35           ` Jan Kratochvil
2010-10-15 17:42             ` Tom Tromey
2010-10-15 17:49               ` Jan Kratochvil
2010-10-15 17:41         ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox