[03/20] abg-ir: elf_symbol: add is_suppressed field

Message ID 20210127125853.886677-4-maennich@google.com
State Committed
Headers
Series Refactor (k)symtab reader |

Commit Message

Matthias Männich Jan. 27, 2021, 12:58 p.m. UTC
  In the context of libabigail and a single library run (when reading from
dwarf or from xml), a symbol is either suppressed or it is not. While
one could argue that this is a property of the read_context, the
read_context might not be around anymore when the symbol still is.
Hence, persist the 'is_suppressed' state along with the symbol itself.

        * include/abg-ir.h (elf_symbol::elf_symbol): Add is_suppressed
        parameter.
        (elf_symbol::create): Likewise.
        (elf_symbol::is_suppressed): New getter declaration.
        (elf_symbol::set_is_suppressed): New setter declaration.
        * src/abg-ir.cc (elf_symbol::priv::priv): Add is_suppressed
        parameter.
        (elf_symbol::priv::is_suppressed_): New field.
        (elf_symbol::elf_symbol): Add is_suppressed parameter.
        (elf_symbol::create): Likewise.
        (elf_symbol::is_suppressed): New getter implementation.
        (elf_symbol::set_is_suppressed): New setter implementation.

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
---
 include/abg-ir.h | 12 ++++++++++--
 src/abg-ir.cc    | 29 ++++++++++++++++++++++-------
 2 files changed, 32 insertions(+), 9 deletions(-)
  

Comments

Dodji Seketeli March 9, 2021, 6:03 p.m. UTC | #1
Matthias Maennich <maennich@google.com> a écrit:

> In the context of libabigail and a single library run (when reading from
> dwarf or from xml), a symbol is either suppressed or it is not. While
> one could argue that this is a property of the read_context, the
> read_context might not be around anymore when the symbol still is.
> Hence, persist the 'is_suppressed' state along with the symbol itself.

In the future, if needed be, maybe well rather make the "applied" suppression
specifications be part of the ABI corpus they are related to.  That way,
the is_suppressed becomes a function, rather than a static property,
similarly to what is done for abigail::comparison::diff::is_suppressed().

This could be useful for usecases where we'd want to apply a new
suppression specification to the IR on the fly and see the impact.  I
guess when we have that usecase, then we'll change this.  I just thought
I'd share this :-)

[...]

> +bool
> +elf_symbol::is_suppressed() const
> +{return priv_->is_suppressed_;}
> +
> +void
> +elf_symbol::set_is_suppressed(bool is_suppressed)
> +{priv_->is_suppressed_ = is_suppressed;}
> +

I have added doxygen comments to these two accessors.

[...]

> Reviewed-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Matthias Maennich <maennich@google.com>

Applied to master.

Thanks!
  

Patch

diff --git a/include/abg-ir.h b/include/abg-ir.h
index d82c83592f82..23ba16bd327b 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -839,7 +839,8 @@  private:
 	     const version&	ve,
 	     visibility		vi,
 	     bool		is_linux_string_cst = false,
-	     bool		is_in_ksymtab = false);
+	     bool		is_in_ksymtab = false,
+	     bool		is_suppressed = false);
 
   elf_symbol(const elf_symbol&);
 
@@ -863,7 +864,8 @@  public:
 	 const version&	    ve,
 	 visibility	    vi,
 	 bool		    is_linux_string_cst = false,
-	 bool		    is_in_ksymtab = false);
+	 bool		    is_in_ksymtab = false,
+	 bool		    is_suppressed = false);
 
   const environment*
   get_environment() const;
@@ -937,6 +939,12 @@  public:
   void
   set_is_in_ksymtab(bool is_in_ksymtab);
 
+  bool
+  is_suppressed() const;
+
+  void
+  set_is_suppressed(bool is_suppressed);
+
   const elf_symbol_sptr
   get_main_symbol() const;
 
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 22532cc87dfd..1ba95dd6c14f 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1311,6 +1311,7 @@  struct elf_symbol::priv
   bool			is_common_;
   bool			is_linux_string_cst_;
   bool			is_in_ksymtab_;
+  bool			is_suppressed_;
   elf_symbol_wptr	main_symbol_;
   elf_symbol_wptr	next_alias_;
   elf_symbol_wptr	next_common_instance_;
@@ -1326,7 +1327,8 @@  struct elf_symbol::priv
       is_defined_(false),
       is_common_(false),
       is_linux_string_cst_(false),
-      is_in_ksymtab_(false)
+      is_in_ksymtab_(false),
+      is_suppressed_(false)
   {}
 
   priv(const environment*	  e,
@@ -1340,7 +1342,8 @@  struct elf_symbol::priv
        const elf_symbol::version& ve,
        elf_symbol::visibility	  vi,
        bool			  is_linux_string_cst,
-       bool			  is_in_ksymtab)
+       bool			  is_in_ksymtab,
+       bool			  is_suppressed)
     : env_(e),
       index_(i),
       size_(s),
@@ -1352,7 +1355,8 @@  struct elf_symbol::priv
       is_defined_(d),
       is_common_(c),
       is_linux_string_cst_(is_linux_string_cst),
-      is_in_ksymtab_(is_in_ksymtab)
+      is_in_ksymtab_(is_in_ksymtab),
+      is_suppressed_(is_suppressed)
   {
     if (!is_common_)
       is_common_ = type_ == COMMON_TYPE;
@@ -1409,7 +1413,8 @@  elf_symbol::elf_symbol(const environment* e,
 		       const version&	  ve,
 		       visibility	  vi,
 		       bool		  is_linux_string_cst,
-		       bool		  is_in_ksymtab)
+		       bool		  is_in_ksymtab,
+		       bool		  is_suppressed)
   : priv_(new priv(e,
 		   i,
 		   s,
@@ -1421,7 +1426,8 @@  elf_symbol::elf_symbol(const environment* e,
 		   ve,
 		   vi,
 		   is_linux_string_cst,
-		   is_in_ksymtab))
+		   is_in_ksymtab,
+		   is_suppressed))
 {}
 
 /// Factory of instances of @ref elf_symbol.
@@ -1479,11 +1485,12 @@  elf_symbol::create(const environment* e,
 		   const version&     ve,
 		   visibility	      vi,
 		   bool		      is_linux_string_cst,
-		   bool		      is_in_ksymtab)
+		   bool		      is_in_ksymtab,
+		   bool		      is_suppressed)
 {
   elf_symbol_sptr sym(new elf_symbol(e, i, s, n, t, b, d, c, ve, vi,
 				     is_linux_string_cst,
-				     is_in_ksymtab));
+				     is_in_ksymtab, is_suppressed));
   sym->priv_->main_symbol_ = sym;
   return sym;
 }
@@ -1711,6 +1718,14 @@  void
 elf_symbol::set_is_in_ksymtab(bool is_in_ksymtab)
 {priv_->is_in_ksymtab_ = is_in_ksymtab;}
 
+bool
+elf_symbol::is_suppressed() const
+{return priv_->is_suppressed_;}
+
+void
+elf_symbol::set_is_suppressed(bool is_suppressed)
+{priv_->is_suppressed_ = is_suppressed;}
+
 /// @name Elf symbol aliases
 ///
 /// An alias A for an elf symbol S is a symbol that is defined at the