Record/output access specifiers for class typedefs

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(+)

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