Re: [PATCH] Add --drop-undefined-syms to abidw.

[Dodji Seketeli <dodji@seketeli.org>]

Hello Mark,

Mark Wielaard <mark@klomp.org> a écrit:

> Add a drop_undefined_syms properties to the read_context that
> indicates the reader wants to drop any undefined symbols (which don't
> have associated addresses in the corpus). Implement this for the
> dwarf_reader and abidw (when using the --drop-undefined-syms option).
>
> 	* include/abg-dwarf-reader.h (set_drop_undefined_syms):
> 	New declaration.
> 	* src/abg-dwarf-reader.cc (class read_context): Add private
> 	bool drop_undefined_syms_.
> 	(drop_undefined_syms): New getter and setter method.
> 	(set_drop_undefined_syms): New function.
> 	(function_is_suppressed): Check drop_undefined_syms on
> 	read_context.
> 	(variable_is_suppressed): Likewise.
> 	* src/abg-reader.cc (read_context): Add private bool
> 	m_drop_undefined_syms.
> 	(drop_undefined_syms): New getter and setter method.
> 	* tools/abidw.cc (struct options): Add drop_undefined_syms.
> 	(display_usage): Print --drop-undefined-syms.
> 	(parse_command_line): Parse --drop-undefined-syms.
> 	(main): Call set_drop_undefined_syms.

I have applied this to master.  I have just fixed a few things that I
discuss below.

[...]


> diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> index 249e196d..f74fb144 100644
> --- a/src/abg-dwarf-reader.cc
> +++ b/src/abg-dwarf-reader.cc

[...]


>  /// This flag tells if we should emit verbose logs for various
> @@ -16683,7 +16714,9 @@ function_is_suppressed(const read_context& ctxt,
>    string qualified_name = build_qualified_name(scope, fname);
>  
>    // A non-member function which symbol is not exported is suppressed.
> -  if (!is_class_type(scope) && !die_is_declaration_only(function_die))
> +  // Or if the reader context drops undefined symbols.
> +  if ((!is_class_type(scope) && !die_is_declaration_only(function_die))
> +      || ctxt.drop_undefined_syms())
>      {

For languages like C++, we don't want to remove member functions (those
that are at class scope) even if they don't have defined symbols (e.g,
inline member functions).  So here is how I am writing this:


  // A non-member non-static function which symbol is not exported is
  // suppressed.
  //
  // Note that if the non-member non-static function has an undefined
  // symbol, by default, it's not suppressed.  Unless we are asked to
  // drop undefined symbols too.
  if (!is_class_type(scope)
      && (!die_is_declaration_only(function_die)
	  || ctxt.drop_undefined_syms()))
    {

[...]


> @@ -16798,7 +16831,10 @@ variable_is_suppressed(const read_context& ctxt,
>    // another concrete variable, then this function looks at
>    // suppression specification specifications to know if its
>    // suppressed.
> -  if (!is_class_type(scope) && !is_required_decl_spec)
> +  //
> +  // Or if the reader context drops undefined symbols.
> +  if ((!is_class_type(scope) && !is_required_decl_spec)
> +      || ctxt.drop_undefined_syms())
>      {

So, right now, undefined variable (unlike for functions) are dropped
from the IR, unless they are data members or declaration of static data
members.  So we don't need the ctxt.drop_undefined_syms() here.  So I
re-wrote this as:


  // If a non member variable that is a declaration (has no defined
  // and exported symbol) and is not the specification of another
  // concrete variable, then it's suppressed.  This is a size
  // optimization; it removes useless declaration-only variables from
  // the IR.
  if (!is_class_type(scope) && !is_required_decl_spec)
    {

[...]


Thanks!

Cheers,

-- 
		Dodji