[PING #5] add support for dwarf AT_descriptive_type

[Joel Brobecker <brobecker at adacore dot com>]

Hello GCC Maintainers,

I would really appreciate a review of this patch. So far, there was
one request to change DW_AT_use_GNU_descriptive_type into
DW_AT_GNU_use_descriptive_type, which we can do, but other than that,
no real review.  Once this is in, we can start contributing other
patches that depends on this, and steadily improve the debugging info
generated by GCC for Ada code.

It really is an important patch for me, because I want the debugging
experience for people using the FSF version of the tools to improve.
Before I joined AdaCore, I used to depend on these versions.

Note: the new DWARF attributes are explained at
      http://gcc.gnu.org/wiki/DW_AT_GNU_descriptive_type

Thank you!

On Thu, Sep 02, 2010 at 04:38:49PM +0200, Olivier Hainque wrote:
> Hello,
> 
> Ada allows the definition of particularily complex types and we use a parallel
> type system to let GDB understand them.
> 
> Roughly, particularities of a complex type T are encoded in the name of fields
> in a so-called "descriptive" type associated with T. See ada/exp_dbug.ads for
> details.
> 
> For a few years now, we have been maintaining local changes to have
> these descriptive types emitted and easy to find by GDB. This is a
> proposal to integrate these changes.
> 
> The immediate benefit is improved support for Ada in the debugger, for
> example on the following kind of construct:
> 
>     procedure T is
>        procedure Tryblob (X : Integer) is
> 
> 	  type Blob (Size : integer) is record
> 	     Value : string (1 .. Size);
> 	  end record;
> 
> 	  B : Blob (Size => X);
> 
>        begin
> 	  B.Value := (others => 'A');
> 	  null; -- break here, then query on B
>        end;
>     begin
>        Tryblob (X => 5);
>     end;
> 
> With today's debug info, gdb 7.1 at the breakpoint yields
> 
>     (gdb) p b
>     $1 = (size => 5, value => "")
>     (gdb) ptype b.value
>     type = array (1 .. 0) of character
>  
> With the patch we get the following instead, as expected:
> 
>    (gdb) p b
>    $1 = (size => 5, value => "AAAAA")
>    (gdb) ptype b.value
>    type = array (1 .. size) of character
> 
> For efficiency purposes, we emit a dwarf attribute for a type T to
> designate its descriptive type when there is one. GDB has fallback
> strategies if this is not done, but these involve pretty expensive
> searches making the whole thing unuseable in practice on industrial
> applications.
> 
> There are provisions for this in dwarf2.h already:
> 
>     <<  /* GNAT descriptive type.
> 	See http://gcc.gnu.org/wiki/DW_AT_GNAT_descriptive_type .  */
> 	DW_AT_use_GNAT_descriptive_type = 0x2301,
> 	DW_AT_GNAT_descriptive_type	= 0x2302,
>     >>
> 
> The patch introduces a langhook to let dwarf2out query the front-end about a
> possible descriptive type, and from there generate the corresponding DIE and
> attribute as needed.
> 
> The query, the lazy DIE generation and the attribute reference are all
> factored in a new "add_descriptive_type_attribute" function, called
> from the few places where the need/existence of a potential
> descriptive type is expected.
> 
> In addition, even if only the Ada compiler uses this today, there is
> no strong reason for the attribute to be explictly GNAT specific, so
> the patch renames AT_GNAT_descriptive_type into AT_GNU_descriptive_type.
> 
> We'll take care of updating the wiki page if the change gets approved.
> 
> I realize that the addition of a langhook is in general not welcome for lto.
> My understanding is that it remains acceptable for debug info generation
> purposes.  If that is not the case, we'd be happy to discuss alternate
> possible schemes.
> 
> The attached patch was bootstrapped and regession tested on x86_64-suse-linux,
> both for gdb and gcc --languages=all,ada.
> 
> Thanks much in advance for your feedback,
> 
> Olivier
> 
> 2010-09-01  Olivier Hainque  <hainque@adacore.com>
>             Nicolas Setton  <setton@adacore.com>
>             Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	include/
> 	* dwarf2.h (dwarf_attribute): Replace the DW_AT_GNAT prefix by
> 	DW_AT_GNU in descriptive types attribute names.
> 
> 	gcc/
> 	* dwarf2out.c (dwarf_attr_name): Map DW_AT_GNU_descriptive_type.
> 	(add_descriptive_type_attribute): New function.
> 	(gen_array_type_die): Call it.
> 	(gen_enumeration_type_die): Likewise.
> 	(gen_struct_or_union_type_die): Likewise.
> 	(modified_type_die): Likewise.
> 	* langhooks.h (lang_hooks_for_types): New "descriptive_type" hook.
> 	* langhooks-def.h (LANG_HOOKS_DESCRIPTIVE_TYPE): Default to NULL_TREE.
> 	(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add LANG_HOOKS_DESCRIPTIVE_TYPE.
> 
> 	ada/
> 	* gcc-interface/trans.c (gigi): Remove internal call to
> 	dwarf2out_set_descriptive_type_func.
> 	* gcc-interface/utils.c (get_parallel_type): Move and rename into ...
> 	* gcc-interface/misc.c (gnat_descriptive_type): New function.
> 	(LANG_HOOKS_DESCRIPTIVE_TYPE): Redefine to gnat_descriptive_type.

> Index: include/dwarf2.h
> ===================================================================
> *** include/dwarf2.h	(revision 163722)
> --- include/dwarf2.h	(working copy)
> *************** enum dwarf_attribute
> *** 440,450 ****
>       DW_AT_GNU_template_name = 0x2110,
>       /* VMS extensions.  */
>       DW_AT_VMS_rtnbeg_pd_address = 0x2201,
> !     /* GNAT extensions.  */
> !     /* GNAT descriptive type.
> !        See http://gcc.gnu.org/wiki/DW_AT_GNAT_descriptive_type .  */
> !     DW_AT_use_GNAT_descriptive_type = 0x2301,
> !     DW_AT_GNAT_descriptive_type	= 0x2302,
>       /* UPC extension.  */
>       DW_AT_upc_threads_scaled = 0x3210,
>       /* PGI (STMicroelectronics) extensions.  */
> --- 440,449 ----
>       DW_AT_GNU_template_name = 0x2110,
>       /* VMS extensions.  */
>       DW_AT_VMS_rtnbeg_pd_address = 0x2201,
> !     /* Descriptive types.
> !        See http://gcc.gnu.org/wiki/DW_AT_GNU_descriptive_type .  */
> !     DW_AT_use_GNU_descriptive_type = 0x2301,
> !     DW_AT_GNU_descriptive_type = 0x2302,
>       /* UPC extension.  */
>       DW_AT_upc_threads_scaled = 0x3210,
>       /* PGI (STMicroelectronics) extensions.  */
> Index: dwarf2out.c
> ===================================================================
> *** dwarf2out.c	(revision 163722)
> --- dwarf2out.c	(working copy)
> *************** static bool add_location_or_const_value_
> *** 6263,6268 ****
> --- 6263,6269 ----
>   static bool tree_add_const_value_attribute (dw_die_ref, tree);
>   static bool tree_add_const_value_attribute_for_decl (dw_die_ref, tree);
>   static void add_name_attribute (dw_die_ref, const char *);
> + static void add_descriptive_type_attribute (dw_die_ref, tree, dw_die_ref);
>   static void add_comp_dir_attribute (dw_die_ref);
>   static void add_bound_info (dw_die_ref, enum dwarf_attribute, tree);
>   static void add_subscript_info (dw_die_ref, tree, bool);
> *************** dwarf_attr_name (unsigned int attr)
> *** 6909,6914 ****
> --- 6910,6917 ----
>         return "DW_AT_GNU_odr_signature";
>       case DW_AT_GNU_template_name:
>         return "DW_AT_GNU_template_name";
> +     case DW_AT_GNU_descriptive_type:
> +       return "DW_AT_GNU_descriptive_type";
>   
>       case DW_AT_VMS_rtnbeg_pd_address:
>         return "DW_AT_VMS_rtnbeg_pd_address";
> *************** modified_type_die (tree type, int is_con
> *** 12781,12786 ****
> --- 12784,12790 ----
>   	   useful source coordinates anyway.  */
>   	name = DECL_NAME (name);
>         add_name_attribute (mod_type_die, IDENTIFIER_POINTER (name));
> +       add_descriptive_type_attribute (mod_type_die, type, context_die);
>       }
>     /* This probably indicates a bug.  */
>     else if (mod_type_die && mod_type_die->die_tag == DW_TAG_base_type)
> *************** add_name_attribute (dw_die_ref die, cons
> *** 16997,17002 ****
> --- 17001,17032 ----
>       }
>   }
>   
> + /* Retrieve the descriptive type for TYPE, if any, make sure it has
> +    a DIE and attach a DW_AT_GNU_descriptive_type to the DIE of TYPE
> +    accordingly.  */
> + 
> + static void 
> + add_descriptive_type_attribute (dw_die_ref die, tree type,
> + 				dw_die_ref context_die)
> + {
> +   tree dtype;
> +   dw_die_ref dtype_die;
> + 
> +   dtype = lang_hooks.types.descriptive_type (type);
> +   if (!dtype)
> +     return;
> + 
> +   dtype_die = lookup_type_die (dtype);
> +   if (!dtype_die)
> +     {
> +       gen_type_die (dtype, context_die);
> +       dtype_die = lookup_type_die (dtype);
> +       gcc_assert (dtype_die);
> +     }
> + 
> +   add_AT_die_ref (die, DW_AT_GNU_descriptive_type, dtype_die);
> + }
> + 
>   /* Generate a DW_AT_comp_dir attribute for DIE.  */
>   
>   static void
> *************** gen_array_type_die (tree type, dw_die_re
> *** 17863,17868 ****
> --- 17893,17899 ----
>   
>     array_die = new_die (DW_TAG_array_type, scope_die, type);
>     add_name_attribute (array_die, type_tag (type));
> +   add_descriptive_type_attribute (array_die, type, context_die);
>     equate_type_number_to_die (type, array_die);
>   
>     if (TREE_CODE (type) == VECTOR_TYPE)
> *************** gen_enumeration_type_die (tree type, dw_
> *** 18160,18165 ****
> --- 18191,18197 ----
>   			  scope_die_for (type, context_die), type);
>         equate_type_number_to_die (type, type_die);
>         add_name_attribute (type_die, type_tag (type));
> +       add_descriptive_type_attribute (type_die, type, context_die);
>         if ((dwarf_version >= 4 || !dwarf_strict)
>   	  && ENUM_IS_SCOPED (type))
>   	add_AT_flag (type_die, DW_AT_enum_class, 1);
> *************** gen_struct_or_union_type_die (tree type,
> *** 19684,19690 ****
>         if (old_die)
>   	add_AT_specification (type_die, old_die);
>         else
> ! 	add_name_attribute (type_die, type_tag (type));
>       }
>     else
>       remove_AT (type_die, DW_AT_declaration);
> --- 19716,19725 ----
>         if (old_die)
>   	add_AT_specification (type_die, old_die);
>         else
> ! 	{
> ! 	  add_name_attribute (type_die, type_tag (type));
> ! 	  add_descriptive_type_attribute (type_die, type, context_die);
> ! 	}
>       }
>     else
>       remove_AT (type_die, DW_AT_declaration);
> Index: langhooks.h
> ===================================================================
> *** langhooks.h	(revision 163722)
> --- langhooks.h	(working copy)
> *************** struct lang_hooks_for_types
> *** 133,138 ****
> --- 133,142 ----
>     /* Fill in information for the debugger about the bounds of TYPE.  */
>     void (*get_subrange_bounds) (const_tree, tree *, tree *);
>   
> +   /* A type descriptive of TYPE complex layout particularities to help the
> +      debugger with e.g. variable length or self referential contructs.  */
> +   tree (*descriptive_type) (const_tree);
> + 
>     /* If we requested a pointer to a vector, build up the pointers that
>        we stripped off while looking for the inner type.  Similarly for
>        return values from functions.  The argument TYPE is the top of the
> Index: langhooks-def.h
> ===================================================================
> *** langhooks-def.h	(revision 163722)
> --- langhooks-def.h	(working copy)
> *************** extern tree lhd_make_node (enum tree_cod
> *** 177,182 ****
> --- 177,183 ----
>   #define LANG_HOOKS_TYPE_HASH_EQ		NULL
>   #define LANG_HOOKS_GET_ARRAY_DESCR_INFO	NULL
>   #define LANG_HOOKS_GET_SUBRANGE_BOUNDS	NULL
> + #define LANG_HOOKS_DESCRIPTIVE_TYPE	lhd_return_null_const_tree
>   #define LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE reconstruct_complex_type
>   #define LANG_HOOKS_HASH_TYPES		true
>   
> *************** extern tree lhd_make_node (enum tree_cod
> *** 195,200 ****
> --- 196,202 ----
>     LANG_HOOKS_TYPE_HASH_EQ, \
>     LANG_HOOKS_GET_ARRAY_DESCR_INFO, \
>     LANG_HOOKS_GET_SUBRANGE_BOUNDS, \
> +   LANG_HOOKS_DESCRIPTIVE_TYPE, \
>     LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE, \
>     LANG_HOOKS_HASH_TYPES \
>   }
> Index: ada/gcc-interface/utils.c
> ===================================================================
> *** ada/gcc-interface/utils.c	(revision 163722)
> --- ada/gcc-interface/utils.c	(working copy)
> *************** add_parallel_type (tree decl, tree paral
> *** 952,968 ****
>     SET_DECL_PARALLEL_TYPE (d, parallel_type);
>   }
>   
> - /* Return the parallel type associated to a type, if any.  */
> - 
> - tree
> - get_parallel_type (tree type)
> - {
> -   if (TYPE_STUB_DECL (type))
> -     return DECL_PARALLEL_TYPE (TYPE_STUB_DECL (type));
> -   else
> -     return NULL_TREE;
> - }
> - 
>   /* Utility function of above to merge LAST_SIZE, the previous size of a record
>      with FIRST_BIT and SIZE that describe a field.  SPECIAL is true if this
>      represents a QUAL_UNION_TYPE in which case we must look for COND_EXPRs and
> --- 952,957 ----
> Index: ada/gcc-interface/trans.c
> ===================================================================
> *** ada/gcc-interface/trans.c	(revision 163722)
> --- ada/gcc-interface/trans.c	(working copy)
> *************** gigi (Node_Id gnat_root, int max_gnat_no
> *** 295,301 ****
>         dwarf2out_set_type_encoding_func (extract_encoding);
>         dwarf2out_set_demangle_name_func (decode_name);
>       }
> -   dwarf2out_set_descriptive_type_func (get_parallel_type);
>   #endif
>   
>     /* Enable GNAT stack checking method if needed */
> --- 295,300 ----
> Index: ada/gcc-interface/misc.c
> ===================================================================
> *** ada/gcc-interface/misc.c	(revision 163722)
> --- ada/gcc-interface/misc.c	(working copy)
> *************** static void internal_error_function	(dia
> *** 78,83 ****
> --- 78,84 ----
>   					 const char *, va_list *);
>   static tree gnat_type_max_size		(const_tree);
>   static void gnat_get_subrange_bounds	(const_tree, tree *, tree *);
> + static tree gnat_descriptive_type	(const_tree);
>   static tree gnat_eh_personality		(void);
>   
>   /* Definitions for our language-specific hooks.  */
> *************** static tree gnat_eh_personality		(void);
> *** 128,133 ****
> --- 129,136 ----
>   #define LANG_HOOKS_TYPES_COMPATIBLE_P	gnat_types_compatible_p
>   #undef  LANG_HOOKS_GET_SUBRANGE_BOUNDS
>   #define LANG_HOOKS_GET_SUBRANGE_BOUNDS  gnat_get_subrange_bounds
> + #undef  LANG_HOOKS_DESCRIPTIVE_TYPE
> + #define LANG_HOOKS_DESCRIPTIVE_TYPE	gnat_descriptive_type
>   #undef  LANG_HOOKS_ATTRIBUTE_TABLE
>   #define LANG_HOOKS_ATTRIBUTE_TABLE	gnat_internal_attribute_table
>   #undef  LANG_HOOKS_BUILTIN_FUNCTION
> *************** gnat_dwarf_name (tree decl, int verbosit
> *** 584,589 ****
> --- 587,603 ----
>     return (const char *) IDENTIFIER_POINTER (DECL_NAME (decl));
>   }
>   
> + /* Return the descriptive type associated to TYPE, if any.  */
> + 
> + static tree
> + gnat_descriptive_type (const_tree type)
> + {
> +   if (TYPE_STUB_DECL (type))
> +     return DECL_PARALLEL_TYPE (TYPE_STUB_DECL (type));
> +   else
> +     return NULL_TREE;
> + }
> + 
>   /* Do nothing (return the tree node passed).  */
>   
>   static tree


-- 
Joel