Record/output access specifiers for class typedefs

Message ID 1498416734-14498-1-git-send-email-keiths@redhat.com
State New, archived
Headers

Commit Message

Keith Seitz June 25, 2017, 6:52 p.m. UTC
  We do not currently record/output accessibility for typedefs defined
in a class:

(gdb) list
71	struct typedef_struct {
72	public:
73	  typedef int public_int;
74	  public_int a;
75	protected:
76	  typedef int protected_int;
77	  protected_int b;
78	private:
79	  typedef int private_int;
80	  private_int c;
81	protected:
82	  typedef float protected_float;
83	  protected_float d;
84	private:
85	  typedef float private_float;
86	  private_float e;
87	public:
88	  typedef float public_float;
89	  public_float f;
90	};
(gdb) ptype typedef_struct
type = struct typedef_struct {
  public:
    public_int a;
  protected:
    public_int b;
  private:
    public_int c;
  protected:
    protected_float d;
  private:
    protected_float e;
  public:
    protected_float f;

    typedef int public_int;
    typedef int protected_int;
    typedef int private_int;
    typedef float protected_float;
    typedef float private_float;
    typedef float public_float;
}

This patch modifies the DWARF reader to record accessibility when reading
in typedef DIEs.

gdb/ChangeLog:
yyyy-mm-dd  Keith Seitz  <keiths@redhat.com>

	* c-typeprint.c (c_type_print_base): Output access specifiers
	for any class-defined typedefs.
	* dwarf2read.c (dwarf2_add_typedef): Record DW_AT_accessibility.
	* gdbtypes.h (struct typedef_field) <is_public,is_protected>
	<is_private,dummy>: New fields.
	(TYPE_TYPEDEF_FIELD_PUBLIC,TYPE_TYPEDEF_FIELD_PROTECTED)
	(TYPE_TYPEDEF_FIELD_PRIVATE): New macros.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Keith Seitz  <keiths@redhat.com>

	* gdb.cp/classes.cc (struct typedef_struct): New struct.
	(typedef_s): New variable.
	* gdb.cp/classes.exp (ptype_class_objects): Add test for
	typedef_struct.
---
 gdb/c-typeprint.c                | 29 +++++++++++++++++++++++++++++
 gdb/dwarf2read.c                 | 23 +++++++++++++++++++++++
 gdb/gdbtypes.h                   | 18 ++++++++++++++++++
 gdb/testsuite/gdb.cp/classes.cc  | 22 ++++++++++++++++++++++
 gdb/testsuite/gdb.cp/classes.exp | 17 +++++++++++++++++
 5 files changed, 109 insertions(+)
  

Comments

Pedro Alves June 26, 2017, 10:25 a.m. UTC | #1
On 06/25/2017 07:52 PM, Keith Seitz wrote:
> We do not currently record/output accessibility for typedefs defined
> in a class:
> 
> (gdb) list
> 71	struct typedef_struct {
> 72	public:
> 73	  typedef int public_int;
> 74	  public_int a;
> 75	protected:
> 76	  typedef int protected_int;
> 77	  protected_int b;
> 78	private:
> 79	  typedef int private_int;
> 80	  private_int c;
> 81	protected:
> 82	  typedef float protected_float;
> 83	  protected_float d;
> 84	private:
> 85	  typedef float private_float;
> 86	  private_float e;
> 87	public:
> 88	  typedef float public_float;
> 89	  public_float f;
> 90	};
> (gdb) ptype typedef_struct
> type = struct typedef_struct {
>   public:
>     public_int a;
>   protected:
>     public_int b;
>   private:
>     public_int c;
>   protected:
>     protected_float d;
>   private:
>     protected_float e;
>   public:
>     protected_float f;
> 
>     typedef int public_int;
>     typedef int protected_int;
>     typedef int private_int;
>     typedef float protected_float;
>     typedef float private_float;
>     typedef float public_float;
> }
> 
> This patch modifies the DWARF reader to record accessibility when reading
> in typedef DIEs.

As general principle, please also show in the commit log what
output looks like after the patch.


On 06/25/2017 07:52 PM, Keith Seitz wrote:

> +  /* Save accessibility.  */
> +  struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu);
> +  enum dwarf_access_attribute accessibility;
> +
> +  if (attr != NULL)
> +    accessibility = (enum dwarf_access_attribute) DW_UNSND (attr);
> +  else
> +    accessibility = dwarf2_default_access_attribute (die, cu);
> +  switch (accessibility)
> +    {
> +    case DW_ACCESS_public:
> +      fp->is_public = 1;
> +      break;
> +    case DW_ACCESS_private:
> +      fp->is_private = 1;
> +      break;
> +    case DW_ACCESS_protected:
> +      fp->is_protected = 1;
> +      break;
> +    default:
> +      gdb_assert_not_reached ("unexpected accessibility attribute");

Please don't add assertions that can trigger with
invalid/broken DWARF.  Call complaint instead.


> +		  if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i))
> +		    {
> +		      if (section_type != s_protected)
> +			{
> +			  section_type = s_protected;
> +			  fprintfi_filtered (level + 2, stream,
> +					     "protected:\n");
> +			}
> +		    }
> +		  else if (TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
> +		    {
> +		      if (section_type != s_private)
> +			{
> +			  section_type = s_private;
> +			  fprintfi_filtered (level + 2, stream,
> +					     "private:\n");
> +			}
> +		    }
> +		  else
> +		    {
> +		      gdb_assert (TYPE_TYPEDEF_FIELD_PUBLIC (type, i));

Won't this assertion fail with debug formats other than DWARF?
E.g., stabs?

> +		      if (section_type != s_public)
> +			{
> +			  section_type = s_public;
> +			  fprintfi_filtered (level + 2, stream,
> +					     "public:\n");
> +			}
> +		    }
> +


> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -884,6 +884,18 @@ struct typedef_field
>    /* * Type this typedef named NAME represents.  */
>  
>    struct type *type;
> +
> +  /* * True if this field was declared public, false otherwise.  */
> +  unsigned int is_public : 1;
> +
> +  /* * True if this field was declared protected, false otherwise.  */
> +  unsigned int is_protected : 1;
> +
> +  /* * True if this field was declared private, false otherwise.  */
> +  unsigned int is_private : 1;
> +
> +  /* * Unused.  */
> +  unsigned int dummy : 13;

Is this really 13 bits?  Looks to me 29 bits on 32-bit archs (and due 
to padding, really 61 bits on 64-bit archs)?

>  };

For completeness, do you have a sense of whether this is a struct
that might have a significant impact on gdb's memory consumption?
Did you try measuring it with some large program, say, Firefox,
with -readnow ?

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 9e197f5..b38d3b4 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1319,6 +1319,35 @@  c_type_print_base (struct type *type, struct ui_file *stream,
 		  gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF);
 		  target = TYPE_TARGET_TYPE (target);
 
+		  if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i))
+		    {
+		      if (section_type != s_protected)
+			{
+			  section_type = s_protected;
+			  fprintfi_filtered (level + 2, stream,
+					     "protected:\n");
+			}
+		    }
+		  else if (TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
+		    {
+		      if (section_type != s_private)
+			{
+			  section_type = s_private;
+			  fprintfi_filtered (level + 2, stream,
+					     "private:\n");
+			}
+		    }
+		  else
+		    {
+		      gdb_assert (TYPE_TYPEDEF_FIELD_PUBLIC (type, i));
+		      if (section_type != s_public)
+			{
+			  section_type = s_public;
+			  fprintfi_filtered (level + 2, stream,
+					     "public:\n");
+			}
+		    }
+
 		  print_spaces_filtered (level + 4, stream);
 		  fprintf_filtered (stream, "typedef ");
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2369d4b..922e81d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -12987,6 +12987,29 @@  dwarf2_add_typedef (struct field_info *fip, struct die_info *die,
 
   fp->type = read_type_die (die, cu);
 
+  /* Save accessibility.  */
+  struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu);
+  enum dwarf_access_attribute accessibility;
+
+  if (attr != NULL)
+    accessibility = (enum dwarf_access_attribute) DW_UNSND (attr);
+  else
+    accessibility = dwarf2_default_access_attribute (die, cu);
+  switch (accessibility)
+    {
+    case DW_ACCESS_public:
+      fp->is_public = 1;
+      break;
+    case DW_ACCESS_private:
+      fp->is_private = 1;
+      break;
+    case DW_ACCESS_protected:
+      fp->is_protected = 1;
+      break;
+    default:
+      gdb_assert_not_reached ("unexpected accessibility attribute");
+    }
+
   new_field->next = fip->typedef_field_list;
   fip->typedef_field_list = new_field;
   fip->typedef_field_list_count++;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 6f896db..4efaab2 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -884,6 +884,18 @@  struct typedef_field
   /* * Type this typedef named NAME represents.  */
 
   struct type *type;
+
+  /* * True if this field was declared public, false otherwise.  */
+  unsigned int is_public : 1;
+
+  /* * True if this field was declared protected, false otherwise.  */
+  unsigned int is_protected : 1;
+
+  /* * True if this field was declared private, false otherwise.  */
+  unsigned int is_private : 1;
+
+  /* * Unused.  */
+  unsigned int dummy : 13;
 };
 
 /* * C++ language-specific information for TYPE_CODE_STRUCT and
@@ -1429,6 +1441,12 @@  extern void set_type_vptr_basetype (struct type *, struct type *);
   TYPE_TYPEDEF_FIELD (thistype, n).type
 #define TYPE_TYPEDEF_FIELD_COUNT(thistype) \
   TYPE_CPLUS_SPECIFIC (thistype)->typedef_field_count
+#define TYPE_TYPEDEF_FIELD_PUBLIC(thistype, n)	\
+  TYPE_TYPEDEF_FIELD (thistype, n).is_public
+#define TYPE_TYPEDEF_FIELD_PROTECTED(thistype, n) \
+  TYPE_TYPEDEF_FIELD (thistype, n).is_protected
+#define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n)	\
+  TYPE_TYPEDEF_FIELD (thistype, n).is_private
 
 #define TYPE_IS_OPAQUE(thistype) \
   (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \
diff --git a/gdb/testsuite/gdb.cp/classes.cc b/gdb/testsuite/gdb.cp/classes.cc
index 2a81473..1aa9f84 100644
--- a/gdb/testsuite/gdb.cp/classes.cc
+++ b/gdb/testsuite/gdb.cp/classes.cc
@@ -68,6 +68,27 @@  struct mixed_protection_struct {
   int i;
 };
 
+struct typedef_struct {
+public:
+  typedef int public_int;
+  public_int a;
+protected:
+  typedef int protected_int;
+  protected_int b;
+private:
+  typedef int private_int;
+  private_int c;
+protected:
+  typedef float protected_float;
+  protected_float d;
+private:
+  typedef float private_float;
+  private_float e;
+public:
+  typedef float public_float;
+  public_float f;
+};
+
 class public_class {
  public:
   int a;
@@ -624,3 +645,4 @@  protected_class protected_c;
 default_private_class default_private_c;
 explicit_private_class explicit_private_c;
 mixed_protection_class mixed_protection_c;
+typedef_struct typedef_s;
diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
index 256fa68..eda2327 100644
--- a/gdb/testsuite/gdb.cp/classes.exp
+++ b/gdb/testsuite/gdb.cp/classes.exp
@@ -84,6 +84,23 @@  proc test_ptype_class_objects {} {
 	    { field protected "int i;" }
 	}
 
+    cp_test_ptype_class \
+	"struct typedef_struct" "" "struct" "typedef_struct" \
+	{
+	    { field public    "typedef_struct::public_int a;" }
+	    { field protected "typedef_struct::protected_int b;" }
+	    { field private   "typedef_struct::private_int c;" }
+	    { field protected "typedef_struct::protected_float d;" }
+	    { field private   "typedef_struct::private_float e;" }
+	    { field public    "typedef_struct::public_float f;" }
+	    { typedef public    "typedef int public_int;" }
+	    { typedef protected "typedef int protected_int;" }
+	    { typedef private   "typedef int private_int;" }
+	    { typedef protected "typedef float protected_float;" }
+	    { typedef private   "typedef float private_float;" }
+	    { typedef public    "typedef float public_float;" }
+	}
+
     # All that again with "class" instead of "struct".
     # gdb does not care about the difference anyways.