public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Matthias Maennich <maennich@google.com>
To: libabigail@sourceware.org
Cc: dodji@seketeli.org, gprocida@google.com, kernel-team@android.com,
	 maennich@google.com
Subject: [PATCH v2 21/21] reader/symtab: Improve handling for suppressed aliases
Date: Fri,  3 Jul 2020 18:46:51 +0200	[thread overview]
Message-ID: <20200703164651.1510825-22-maennich@google.com> (raw)
In-Reply-To: <20200703164651.1510825-1-maennich@google.com>

When reading from XML with a symbol whitelist that leads to suppression
of aliased symbols, abidiff would hit an assertion and crash when
looking up the aliased symbol due to it being suppressed. In the new
symtab reader we can still suppress a symbol without removing it from
the lookup. Make use of that property to fix this bug.

A test has been added for this as well.

	* src/abg-reader.cc (build_elf_symbol): Improve handling of
	suppressed aliased symbols when reading from XML.
	* src/abg-symtab-reader.cc (load): Likewise.
	* tests/data/Makefile.am: Add new test data files.
	* tests/data/test-abidiff-exit/test-missing-alias-report.txt: New test file.
	* tests/data/test-abidiff-exit/test-missing-alias.abi: Likewise.
	* tests/data/test-abidiff-exit/test-missing-alias.suppr: Likewise.
	* tests/test-abidiff-exit.cc: Add support for whitelists and add
	new testcase.

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-reader.cc                                |  7 +++++--
 src/abg-symtab-reader.cc                         | 16 ++++++++++++++--
 tests/data/Makefile.am                           |  3 +++
 .../test-missing-alias-report.txt                |  0
 .../test-abidiff-exit/test-missing-alias.abi     | 12 ++++++++++++
 .../test-abidiff-exit/test-missing-alias.suppr   |  4 ++++
 tests/test-abidiff-exit.cc                       |  9 +++++++++
 7 files changed, 47 insertions(+), 4 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-missing-alias-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-missing-alias.abi
 create mode 100644 tests/data/test-abidiff-exit/test-missing-alias.suppr

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 3f636d00f9b8..d71e2d43503a 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -2850,7 +2850,8 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node,
 
   elf_symbol::version version(version_string, is_default_version);
 
-  if (drop_if_suppressed && suppr::is_elf_symbol_suppressed(ctxt, name, type))
+  const bool is_suppressed = suppr::is_elf_symbol_suppressed(ctxt, name, type);
+  if (drop_if_suppressed && is_suppressed)
     return elf_symbol_sptr();
 
   const environment* env = ctxt.get_environment();
@@ -2860,6 +2861,8 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node,
 					 version, visibility,
 					 /*is_linux_string_cst=*/false);
 
+  e->set_is_suppressed(is_suppressed);
+
   if (crc != 0)
     e->set_crc(crc);
 
@@ -2950,7 +2953,7 @@ build_elf_symbol_db(read_context& ctxt,
   elf_symbol_sptr sym;
   for (xmlNodePtr n = node->children; n; n = n->next)
     {
-      if ((sym = build_elf_symbol(ctxt, n, /*drop_if_suppress=*/true)))
+      if ((sym = build_elf_symbol(ctxt, n, /*drop_if_suppress=*/false)))
 	{
 	  id_sym_map[sym->get_id_string()] = sym;
 	  xml_node_ptr_elf_symbol_map[n] = sym;
diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
index c50c0f643386..424817106e70 100644
--- a/src/abg-symtab-reader.cc
+++ b/src/abg-symtab-reader.cc
@@ -361,7 +361,13 @@ symtab::load_(string_elf_symbols_map_sptr function_symbol_map,
 	     end = function_symbol_map->end();
 	 it != end; ++it)
       {
-	symbols_.insert(symbols_.end(), it->second.begin(), it->second.end());
+	for (elf_symbols::const_iterator sym_it = it->second.begin(),
+					 sym_end = it->second.end();
+	     sym_it != sym_end; ++sym_it)
+	  {
+	    if (!(*sym_it)->is_suppressed())
+	      symbols_.push_back(*sym_it);
+	  }
 	ABG_ASSERT(name_symbol_map_.insert(*it).second);
       }
 
@@ -371,7 +377,13 @@ symtab::load_(string_elf_symbols_map_sptr function_symbol_map,
 	     end = variables_symbol_map->end();
 	 it != end; ++it)
       {
-	symbols_.insert(symbols_.end(), it->second.begin(), it->second.end());
+	for (elf_symbols::const_iterator sym_it = it->second.begin(),
+					 sym_end = it->second.end();
+	     sym_it != sym_end; ++sym_it)
+	  {
+	    if (!(*sym_it)->is_suppressed())
+	      symbols_.push_back(*sym_it);
+	  }
 	ABG_ASSERT(name_symbol_map_.insert(*it).second);
       }
 
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index f39a6d427b1d..bcfc3796d0bc 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -161,6 +161,9 @@ test-abidiff-exit/test-fun-param-v0.o \
 test-abidiff-exit/test-fun-param-v1.abi \
 test-abidiff-exit/test-fun-param-v1.c \
 test-abidiff-exit/test-fun-param-v1.o \
+test-abidiff-exit/test-missing-alias-report.txt \
+test-abidiff-exit/test-missing-alias.abi \
+test-abidiff-exit/test-missing-alias.suppr \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-missing-alias-report.txt b/tests/data/test-abidiff-exit/test-missing-alias-report.txt
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/data/test-abidiff-exit/test-missing-alias.abi b/tests/data/test-abidiff-exit/test-missing-alias.abi
new file mode 100644
index 000000000000..07a13f5e6d15
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-missing-alias.abi
@@ -0,0 +1,12 @@
+<abi-corpus path='test.so' soname='test.so'>
+  <elf-function-symbols>
+    <elf-symbol name='__foo' type='func-type' binding='global-binding' visibility='default-visibility' alias='foo' is-defined='yes'/>
+    <elf-symbol name='foo' type='func-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
+  </elf-function-symbols>
+  <abi-instr version='1.0' address-size='64' path='test.c' language='LANG_C89'>
+    <type-decl name='void' id='48b5725f'/>
+    <function-decl name='__foo' mangled-name='__foo' filepath='test.c' line='8' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='__foo'>
+      <return type-id='48b5725f'/>
+    </function-decl>
+  </abi-instr>
+</abi-corpus>
diff --git a/tests/data/test-abidiff-exit/test-missing-alias.suppr b/tests/data/test-abidiff-exit/test-missing-alias.suppr
new file mode 100644
index 000000000000..bcebae43a350
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-missing-alias.suppr
@@ -0,0 +1,4 @@
+[suppress_function]
+  symbol_name_not_regexp = ^__foo$
+  drop = true
+
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 4d9c19437e60..772baa78cf91 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -212,6 +212,15 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-fun-param-report.txt",
     "output/test-abidiff-exit/test-fun-param-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-missing-alias.abi",
+    "data/test-abidiff-exit/test-missing-alias.abi",
+    "data/test-abidiff-exit/test-missing-alias.suppr",
+    "",
+    abigail::tools_utils::ABIDIFF_OK,
+    "data/test-abidiff-exit/test-missing-alias-report.txt",
+    "output/test-abidiff-exit/test-missing-alias-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
-- 
2.27.0.212.ge8ba1cc988-goog


  parent reply	other threads:[~2020-07-03 16:48 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 21:42 [PATCH v1 00/16] Refactor (k)symtab reader Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 01/16] abg-cxx-compat: add simplified version of std::optional Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 02/16] abg-cxx-compat: more <functional> support: std::bind and friends Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 03/16] abg-ir: elf_symbol: add is_in_ksymtab field Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 04/16] abg-ir: elf_symbol: add is_suppressed field Matthias Maennich
2020-06-22  9:46   ` Giuliano Procida
2020-06-19 21:42 ` [PATCH v1 05/16] dwarf-reader split: create abg-symtab-reader.{h, cc} and test case Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 06/16] Refactor ELF symbol table reading by adding a new symtab reader Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 07/16] Integrate new symtab reader into corpus and read_context Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 08/16] corpus: make get_(undefined_)?_(var|fun)_symbols use the new symtab Matthias Maennich
2020-06-22  9:53   ` Giuliano Procida
2020-06-19 21:42 ` [PATCH v1 09/16] corpus: make get_unreferenced_(function|variable)_symbols " Matthias Maennich
2020-06-19 21:42 ` [PATCH v1 10/16] abg-reader: avoid using the (var|function)_symbol_map Matthias Maennich
2020-06-19 21:43 ` [PATCH v1 11/16] dwarf-reader: read_context: use new symtab in *_symbols_is_exported Matthias Maennich
2020-06-19 21:43 ` [PATCH v1 12/16] Switch kernel stuff over to new symtab and drop unused code Matthias Maennich
2020-06-19 21:43 ` [PATCH v1 13/16] abg-elf-helpers: migrate ppc64 specific helpers Matthias Maennich
2020-06-19 21:43 ` [PATCH v1 14/16] symtab_reader: add support for ppc64 ELFv1 binaries Matthias Maennich
2020-06-19 21:43 ` [PATCH v1 15/16] abg-corpus: remove symbol maps and their setters Matthias Maennich
2020-06-19 21:43 ` [PATCH v1 16/16] dwarf reader: drop (now) unused code related symbol table reading Matthias Maennich
2020-06-22  9:56   ` Giuliano Procida
2020-07-03 16:46 ` [PATCH v2 00/21] Refactor (k)symtab reader Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 01/21] abg-cxx-compat: add simplified version of std::optional Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 02/21] abg-cxx-compat: more <functional> support: std::bind and friends Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 03/21] abg-ir: elf_symbol: add is_in_ksymtab field Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 04/21] abg-ir: elf_symbol: add is_suppressed field Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 05/21] dwarf-reader split: create abg-symtab-reader.{h, cc} and test case Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 06/21] Refactor ELF symbol table reading by adding a new symtab reader Matthias Maennich
2020-07-20 15:39     ` Dodji Seketeli
2020-07-03 16:46   ` [PATCH v2 07/21] Integrate new symtab reader into corpus and read_context Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 08/21] corpus: make get_(undefined_)?_(var|fun)_symbols use the new symtab Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 09/21] corpus: make get_unreferenced_(function|variable)_symbols " Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 10/21] abg-reader: avoid using the (var|function)_symbol_map Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 11/21] dwarf-reader: read_context: use new symtab in *_symbols_is_exported Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 12/21] Switch kernel stuff over to new symtab and drop unused code Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 13/21] abg-elf-helpers: migrate ppc64 specific helpers Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 14/21] symtab_reader: add support for ppc64 ELFv1 binaries Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 15/21] abg-corpus: remove symbol maps and their setters Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 16/21] dwarf reader: drop now-unused code related to symbol table reading Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 17/21] test-symtab: add tests for whitelisted functions Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 18/21] symtab/dwarf-reader: allow hinting of main symbols for aliases Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 19/21] dwarf-reader/writer: consider aliases when dealing with suppressions Matthias Maennich
2020-07-03 16:46   ` [PATCH v2 20/21] symtab: Add support for MODVERSIONS (CRC checksums) Matthias Maennich
2020-07-03 16:46   ` Matthias Maennich [this message]
2020-07-20 14:27   ` [PATCH v2 00/21] Refactor (k)symtab reader Dodji Seketeli
2021-01-27 12:58 ` [PATCH 00/20] " Matthias Maennich
2021-01-27 12:58   ` [PATCH 01/20] abg-cxx-compat: add simplified version of std::optional Matthias Maennich
2021-03-09  9:43     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 02/20] abg-ir: elf_symbol: add is_in_ksymtab field Matthias Maennich
2021-03-09 14:05     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 03/20] abg-ir: elf_symbol: add is_suppressed field Matthias Maennich
2021-03-09 18:03     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 04/20] dwarf-reader split: create abg-symtab-reader.{h, cc} and test case Matthias Maennich
2021-03-10 18:00     ` [PATCH 04/20] dwarf-reader split: create abg-symtab-reader.{h,cc} " Dodji Seketeli
2021-01-27 12:58   ` [PATCH 05/20] Refactor ELF symbol table reading by adding a new symtab reader Matthias Maennich
2021-03-12 11:18     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 06/20] Integrate new symtab reader into corpus and read_context Matthias Maennich
2021-03-12 15:04     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 07/20] corpus: make get_(undefined_)?_(var|fun)_symbols use the new symtab Matthias Maennich
2021-03-15 10:05     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 08/20] corpus: make get_unreferenced_(function|variable)_symbols " Matthias Maennich
2021-03-15 12:06     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 09/20] abg-reader: avoid using the (var|function)_symbol_map Matthias Maennich
2021-03-15 14:23     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 10/20] dwarf-reader: read_context: use new symtab in *_symbols_is_exported Matthias Maennich
2021-03-15 18:13     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 11/20] Switch kernel stuff over to new symtab and drop unused code Matthias Maennich
2021-03-16 10:38     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 12/20] abg-elf-helpers: migrate ppc64 specific helpers Matthias Maennich
2021-03-16 10:59     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 13/20] symtab_reader: add support for ppc64 ELFv1 binaries Matthias Maennich
2021-03-16 11:39     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 14/20] abg-corpus: remove symbol maps and their setters Matthias Maennich
2021-03-16 18:08     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 15/20] dwarf reader: drop (now) unused code related to symbol table reading Matthias Maennich
2021-03-16 18:42     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 16/20] test-symtab: add tests for whitelisted functions Matthias Maennich
2021-03-17 11:07     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 17/20] symtab/dwarf-reader: allow hinting of main symbols for aliases Matthias Maennich
2021-03-17 13:40     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 18/20] dwarf-reader/writer: consider aliases when dealing with suppressions Matthias Maennich
2021-03-17 15:44     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 19/20] abg-writer.cc: fix write_elf_symbol_reference loop Matthias Maennich
2021-03-17 16:11     ` Dodji Seketeli
2021-01-27 12:58   ` [PATCH 20/20] symtab: Add support for MODVERSIONS (CRC checksums) Matthias Maennich
2021-03-17 17:13     ` Dodji Seketeli
2021-03-17 23:29       ` Giuliano Procida
2021-03-18 22:10         ` Matthias Maennich
2021-03-19 16:55           ` Dodji Seketeli
2021-03-19 18:15     ` Dodji Seketeli
2021-03-29 13:19   ` [GIT PULL] Refactor (k)symtab reader Matthias Maennich
2021-04-02 14:28     ` Dodji Seketeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200703164651.1510825-22-maennich@google.com \
    --to=maennich@google.com \
    --cc=dodji@seketeli.org \
    --cc=gprocida@google.com \
    --cc=kernel-team@android.com \
    --cc=libabigail@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).