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 19/20] abg-writer.cc: fix write_elf_symbol_reference loop
Date: Wed, 27 Jan 2021 12:58:52 +0000	[thread overview]
Message-ID: <20210127125853.886677-20-maennich@google.com> (raw)
In-Reply-To: <20210127125853.886677-1-maennich@google.com>

From: Giuliano Procida <gprocida@google.com>

The function write_elf_symbol_reference iterates through aliases,
looking for an unsuppressed alias to use. The existing code went wrong
in the case when aliases are present. In the case of all symbols
suppressed, it would also have selected the last alias, rather than
the first, if the data structure invariants had matched the code's
expectations.

The main symbol is always distinguished. When aliases are absent, the
sole symbol's next pointer is null, but when aliases are present, they
form a circular list. This makes traversal of aliases a bit tricky.

The code now picks the first symbol from the following:

- the given symbol, if unsuppressed
- the main symbol, if unsuppressed
- the unsuppressed aliases in the remainder of the alias chain
- the main symbol (suppressed)

The given symbol, which need not be the same as the main symbol, will
be tested twice, if suppressed, but addressing this would make the
code even more elaborate and fragile.

The last case may be unreachable if symbol suppression triggers when
all aliases are suppressed.

I left this change stand-alone for easier review and to credit Giuliano for his
work on it, though it fixes a previous commit.

	* src/abg-writer.cc (write_elf_symbol_reference): Check main
	symbol and aliases with more care.

Fixes: commmit ("dwarf-reader/writer: consider aliases when dealing with suppressions")
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-writer.cc | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index c2fdd8f0a043..4985174d3abd 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -1728,10 +1728,28 @@ write_elf_symbol_aliases(const elf_symbol& sym, ostream& out)
 static bool
 write_elf_symbol_reference(const elf_symbol& sym, ostream& o)
 {
-  auto actual_sym = &sym;
-  while (actual_sym->is_suppressed() && actual_sym->has_aliases())
-    actual_sym = actual_sym->get_next_alias().get();
-  o << " elf-symbol-id='" << actual_sym->get_id_string() << "'";
+  const elf_symbol* main = sym.get_main_symbol().get();
+  const elf_symbol* alias = &sym;
+  bool found = !alias->is_suppressed();
+  // If the symbol itself is suppressed, check the alias chain.
+  if (!found)
+    {
+      alias = main;
+      found = !alias->is_suppressed();
+    }
+  // If the main symbol is suppressed, search the remainder of the chain.
+  while (!found)
+    {
+      alias = alias->get_next_alias().get();
+      // Two separate termination conditions at present.
+      if (!alias || alias == main)
+        break;
+      found = !alias->is_suppressed();
+    }
+  // If all aliases are suppressed, just stick with the main symbol.
+  if (!found)
+    alias = main;
+  o << " elf-symbol-id='" << alias->get_id_string() << "'";
   return true;
 }
 
-- 
2.30.0.280.ga3ce27912f-goog


  parent reply	other threads:[~2021-01-27 12:59 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   ` [PATCH v2 21/21] reader/symtab: Improve handling for suppressed aliases Matthias Maennich
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   ` Matthias Maennich [this message]
2021-03-17 16:11     ` [PATCH 19/20] abg-writer.cc: fix write_elf_symbol_reference loop 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=20210127125853.886677-20-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).