Re: [PATCH 2/2] DWARF: track chained DIE declaration-only status

[Dodji Seketeli <dodji@seketeli.org>]

Giuliano Procida <gprocida@google.com> a écrit:

[...]

> +++ b/src/abg-dwarf-reader.cc

> @@ -13055,7 +13060,8 @@ static enum_type_decl_sptr
>  build_enum_type(read_context&	ctxt,
>  		Dwarf_Die*	die,
>  		scope_decl*	scope,
> -		size_t		where_offset)
> +		size_t		where_offset,
> +		bool		is_declaration_only)

This new parameter needs documentation.

>  {
>    enum_type_decl_sptr result;
>    if (!die)
> @@ -13068,7 +13074,6 @@ build_enum_type(read_context&	ctxt,
>    string name, linkage_name;
>    location loc;
>    die_loc_and_name(ctxt, die, loc, name, linkage_name);
> -  bool is_declaration_only = die_is_declaration_only(die);
>  
>    bool is_anonymous = false;
>    // If the enum is anonymous, let's give it a name.
> @@ -13541,7 +13546,8 @@ add_or_update_class_type(read_context&	 ctxt,
>  			 bool		 is_struct,
>  			 class_decl_sptr klass,
>  			 bool		 called_from_public_decl,
> -			 size_t	 where_offset)
> +			 size_t		 where_offset,
> +			 bool		 is_declaration_only)

Likewise.

[...]

> @@ -13908,12 +13912,13 @@ add_or_update_class_type(read_context&	 ctxt,
>  /// e.g, DW_TAG_partial_unit that can be included in several places in
>  /// the DIE tree.
>  static union_decl_sptr
> -add_or_update_union_type(read_context&	ctxt,
> -			 Dwarf_Die*	die,
> -			 scope_decl*	scope,
> +add_or_update_union_type(read_context&	 ctxt,
> +			 Dwarf_Die*	 die,
> +			 scope_decl*	 scope,
>  			 union_decl_sptr union_type,
> -			 bool		called_from_public_decl,
> -			 size_t	where_offset)
> +			 bool		 called_from_public_decl,
> +			 size_t		 where_offset,
> +			 bool		 is_declaration_only)

Likewise.

[...]


> @@ -15183,7 +15187,8 @@ build_var_decl(read_context&	ctxt,
>  static bool
>  function_is_suppressed(const read_context& ctxt,
>  		       const scope_decl* scope,
> -		       Dwarf_Die *function_die)
> +		       Dwarf_Die *function_die,
> +		       bool is_declaration_only)

Likewise.

[...]


> @@ -15256,10 +15260,11 @@ build_or_get_fn_decl_if_not_suppressed(read_context&	  ctxt,
>  				       scope_decl	  *scope,
>  				       Dwarf_Die	  *fn_die,
>  				       size_t		  where_offset,
> +				       bool		  is_declaration_only,
>  				       function_decl_sptr result)
>  {

Likewise.

[...]

> @@ -16232,6 +16237,7 @@ build_ir_node_from_die(read_context&	ctxt,
>  		       scope_decl*	scope,
>  		       bool		called_from_public_decl,
>  		       size_t		where_offset,
> +		       bool		is_declaration_only,
>  		       bool		is_required_decl_spec)
>  {

Likewise.

>    type_or_decl_base_sptr result;
> @@ -16268,6 +16274,12 @@ build_ir_node_from_die(read_context&	ctxt,
>        return result;
>      }
>  
> +  // This is *the* bit of code that ensures we have the right notion
> +  // of "declared" at any point in a DIE chain formed from
> +  // DW_AT_abstract_origin and DW_AT_specification links. There should
> +  // be no other callers of die_is_declaration_only.
> +  is_declaration_only = is_declaration_only && die_is_declaration_only(die);
> +

Agreed.

[...]


> 	* src/abg-dwarf-reader.cc (add_or_update_class_type): Add an
> 	is_declaration_only argument. Use this in favour of the
> 	die_is_declaration_only helper function.
> 	(add_or_update_union_type): Ditto.
> 	(function_is_suppressed): Ditto.
> 	(build_or_get_fn_decl_if_not_suppressed): Ditto.
> 	(build_enum_type): Ditto.
> 	(build_ir_node_from_die): To the main overload, add
> 	is_declaration_only argument and default this to true.
> 	Update this to false if the given DIE is not declaration
> 	only and pass this on in recusrive calls and calls to
> 	build_enum_type, add_or_update_union_type,
> 	add_or_update_class_type and
> 	build_or_get_fn_decl_if_not_suppressed.
> 	* tests/data/test-annotate/test17-pr19027.so.abi: Update
> 	test. This is mostly the removal of is-declaration-only
> 	attributes, removal of unreachable parts of the type graph and
> 	type id renumbering.
> 	* tests/data/test-annotate/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
> 	Likewise.
> 	* tests/data/test-annotate/test19-pr19023-libtcmalloc_and_profiler.so.abi:
> 	Likewise.
> 	* tests/data/test-annotate/test20-pr19025-libvtkParallelCore-6.1.so.abi:
> 	Likewise.
> 	* tests/data/test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1-report-0.txt:
> 	Likewise.
> 	* tests/data/test-read-dwarf/test17-pr19027.so.abi: Likewise.
> 	* tests/data/test-read-dwarf/test18-pr19037-libvtkRenderingLIC-6.1.so.abi:
> 	Likewise.
> 	* tests/data/test-read-dwarf/test19-pr19023-libtcmalloc_and_profiler.so.abi:
> 	Likewise.
> 	* tests/data/test-read-dwarf/test20-pr19025-libvtkParallelCore-6.1.so.abi:
> 	Likewise.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

I have applied this with the changes I noted above.  Thanks!

Cheers,

-- 
		Dodji