Add --drop-undefined-syms to abidw.

Message ID 20200413014058.57109-1-mark@klomp.org
State Committed
Headers
Series Add --drop-undefined-syms to abidw. |

Commit Message

Mark Wielaard April 13, 2020, 1:40 a.m. UTC
  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.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 doc/manuals/abidw.rst      |  7 +++++++
 include/abg-dwarf-reader.h |  4 ++++
 src/abg-dwarf-reader.cc    | 40 ++++++++++++++++++++++++++++++++++++--
 src/abg-reader.cc          | 21 +++++++++++++++++++-
 tools/abidw.cc             |  8 +++++++-
 5 files changed, 76 insertions(+), 4 deletions(-)
  

Comments

Dodji Seketeli April 15, 2020, 12:48 p.m. UTC | #1
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,
  

Patch

diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index 05a90003..6cc4693c 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -146,6 +146,13 @@  Options
     representation build by Libabigail to represent the ABI and will
     not end up in the abi XML file.
 
+  * ``--drop-undefined-syms``
+
+    With this option functions or variables for which the (exported)
+    ELF symbol is undefined are dropped from the internal
+    representation build by Libabigail to represent the ABI and will
+    not end up in the abi XML file.
+
   * ``--no-linux-kernel-mode``
 
     Without this option, if abipkgiff detects that the binaries it is
diff --git a/include/abg-dwarf-reader.h b/include/abg-dwarf-reader.h
index 3b783638..d0329aed 100644
--- a/include/abg-dwarf-reader.h
+++ b/include/abg-dwarf-reader.h
@@ -188,6 +188,10 @@  void
 set_show_stats(read_context& ctxt,
 	       bool f);
 
+void
+set_drop_undefined_syms(read_context& ctxt,
+			bool f);
+
 void
 set_do_log(read_context& ctxt, bool f);
 
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
@@ -3259,6 +3259,7 @@  public:
   string			elf_architecture_;
   corpus::exported_decls_builder* exported_decls_builder_;
   options_type			options_;
+  bool				drop_undefined_syms_;
   read_context();
 
 public:
@@ -3427,6 +3428,7 @@  public:
     options_.env = environment;
     options_.load_in_linux_kernel_mode = linux_kernel_mode;
     options_.load_all_types = load_all_types;
+    drop_undefined_syms_ = false;
     load_in_linux_kernel_mode(linux_kernel_mode);
   }
 
@@ -3518,6 +3520,23 @@  public:
   env(ir::environment* env)
   {options_.env = env;}
 
+  /// Getter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @return true iff we are dropping functions and variables that have
+  /// undefined symbols.
+  bool
+  drop_undefined_syms() const
+  {return drop_undefined_syms_;}
+
+  /// Setter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @param f the new value of the flag.
+  void
+  drop_undefined_syms(bool f)
+  {drop_undefined_syms_ = f;}
+
   /// Getter of the suppression specifications to be used during
   /// ELF/DWARF parsing.
   ///
@@ -9494,6 +9513,18 @@  void
 set_show_stats(read_context& ctxt, bool f)
 {ctxt.show_stats(f);}
 
+/// Setter of the "drop_undefined_syms" flag.
+///
+/// This flag tells if we should drop functions or variables
+/// with undefined symbols.
+///
+/// @param ctxt the read context to consider for this flag.
+///
+/// @param f the value of the flag.
+void
+set_drop_undefined_syms(read_context& ctxt, bool f)
+{ctxt.drop_undefined_syms(f);}
+
 /// Setter of the "do_log" flag.
 ///
 /// 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())
     {
       Dwarf_Addr fn_addr;
       if (!ctxt.get_function_address(function_die, fn_addr))
@@ -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())
     {
       Dwarf_Addr var_addr = 0;
       if (!ctxt.get_variable_address(variable_die, var_addr))
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 340223d0..dcaa27e1 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -131,6 +131,7 @@  private:
   corpus::exported_decls_builder*			m_exported_decls_builder;
   suppr::suppressions_type				m_supprs;
   bool							m_tracking_non_reachable_types;
+  bool							m_drop_undefined_syms;
 
   read_context();
 
@@ -141,7 +142,8 @@  public:
       m_reader(reader),
       m_corp_node(),
       m_exported_decls_builder(),
-      m_tracking_non_reachable_types()
+      m_tracking_non_reachable_types(),
+      m_drop_undefined_syms()
   {}
 
   /// Getter for the flag that tells us if we are tracking types that
@@ -162,6 +164,23 @@  public:
   tracking_non_reachable_types(bool f)
   {m_tracking_non_reachable_types = f;}
 
+  /// Getter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @return true iff we are dropping functions and variables that have
+  /// undefined symbols.
+  bool
+  drop_undefined_syms() const
+  {return m_drop_undefined_syms;}
+
+  /// Setter for the flag that tells us if we are dropping functions
+  /// and variables that have undefined symbols.
+  ///
+  /// @param f the new value of the flag.
+  void
+  drop_undefined_syms(bool f)
+  {m_drop_undefined_syms = f;}
+
   /// Getter of the path to the ABI file.
   ///
   /// @return the path to the native xml abi file.
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 8b163ccd..72a8b0f1 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -110,6 +110,7 @@  struct options
   bool			annotate;
   bool			do_log;
   bool			drop_private_types;
+  bool			drop_undefined_syms;
 
   options()
     : display_version(),
@@ -128,7 +129,8 @@  struct options
       abidiff(),
       annotate(),
       do_log(),
-      drop_private_types(false)
+      drop_private_types(false),
+      drop_undefined_syms(false)
   {}
 
   ~options()
@@ -161,6 +163,7 @@  display_usage(const string& prog_name, ostream& out)
     << "  --no-show-locs  do not show location information\n"
     << "  --short-locs  only print filenames rather than paths\n"
     << "  --drop-private-types  drop private types from representation\n"
+    << "  --drop-undefined-syms  drop undefined symbols from representation\n"
     << "  --no-comp-dir-path  do not show compilation path information\n"
     << "  --check-alternate-debug-info <elf-path>  check alternate debug info "
     "of <elf-path>\n"
@@ -299,6 +302,8 @@  parse_command_line(int argc, char* argv[], options& opts)
 	opts.load_all_types = true;
       else if (!strcmp(argv[i], "--drop-private-types"))
 	opts.drop_private_types = true;
+      else if (!strcmp(argv[i], "--drop-undefined-syms"))
+	opts.drop_undefined_syms = true;
       else if (!strcmp(argv[i], "--no-linux-kernel-mode"))
 	opts.linux_kernel_mode = false;
       else if (!strcmp(argv[i], "--abidiff"))
@@ -785,6 +790,7 @@  main(int argc, char* argv[])
 						opts.load_all_types,
 						opts.linux_kernel_mode);
       read_context& ctxt = *c;
+      set_drop_undefined_syms(ctxt, opts.drop_undefined_syms);
       set_show_stats(ctxt, opts.show_stats);
       set_suppressions(ctxt, opts);
       abigail::dwarf_reader::set_do_log(ctxt, opts.do_log);