diff mbox series

[v2,2/3] abg-reader: ensure corpus always has a symtab reader

Message ID 20210527085305.1400592-3-gprocida@google.com
State Under Review
Headers
Series always create fresh corpus objects |

Commit Message

Giuliano Procida May 27, 2021, 8:53 a.m. UTC 
  In the presence of an empty abi-corpus element and with the following
change to always allocate a fresh corpus object, such objects can
sometimes be left without a symtab reader, instead of inheritng one
from the previous corpus.

The reader is called to obtain sorted lists of symbols during ABI
comparisons. The simplest way to avoid a crash is to maintain the
invariant that a reader object is always present.

With this change, if there is bad XML preventing symbols from being
read, no error is raised as before, but the logic has been tweaked so
that abi-instr parsing will nevertheless be attempted.

	* src/abg-reader.cc (read_symbol_db_from_input): Fix
	documentation for this function. Allow "successful parsing" to
	include the case where no symbols were present in the input.
	(read_corpus_from_input): Unconditionally set a symtab reader
	on the corpus object. Unconditionally parse the abi-instr of a
	corpus.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-reader.cc | 59 ++++++++++++++---------------------------------
 1 file changed, 17 insertions(+), 42 deletions(-)

Comments

Dodji Seketeli June 10, 2021, 4:53 p.m. UTC | #1 
Giuliano Procida <gprocida@google.com> a écrit:

> In the presence of an empty abi-corpus element and with the following
> change to always allocate a fresh corpus object, such objects can
> sometimes be left without a symtab reader, instead of inheritng one
> from the previous corpus.
>
> The reader is called to obtain sorted lists of symbols during ABI
> comparisons. The simplest way to avoid a crash is to maintain the
> invariant that a reader object is always present.
>
> With this change, if there is bad XML preventing symbols from being
> read, no error is raised as before, but the logic has been tweaked so
> that abi-instr parsing will nevertheless be attempted.
>
> 	* src/abg-reader.cc (read_symbol_db_from_input): Fix
> 	documentation for this function. Allow "successful parsing" to
> 	include the case where no symbols were present in the input.
> 	(read_corpus_from_input): Unconditionally set a symtab reader
> 	on the corpus object. Unconditionally parse the abi-instr of a
> 	corpus.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

[...]

Cheers,
diff mbox series

Patch

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 0449718d..9bc43bbf 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -1608,8 +1608,8 @@  read_translation_unit_from_input(read_context&	ctxt)
   return tu;
 }
 
-/// Parse the input XML document containing a function symbols
-/// or a variable symbol database.
+/// Parse the input XML document that may contain function symbol and
+/// variable symbol databases.
 ///
 /// A function symbols database is an XML element named
 /// "elf-function-symbols" and a variable symbols database is an XML
@@ -1618,12 +1618,11 @@  read_translation_unit_from_input(read_context&	ctxt)
 ///
 /// @param ctxt the read_context to use for the parsing.
 ///
-/// @param function_symbols is true if this function should look for a
-/// function symbols database, false if it should look for a variable
-/// symbols database.
+/// @param fn_symdb any resulting function symbol database object, if
+/// elf-function-symbols was present.
 ///
-/// @param symdb the resulting symbol database object.  This is set
-/// iff the function return true.
+/// @param var_symdb any resulting variable symbol database object, if
+/// elf-variable-symbols was present.
 ///
 /// @return true upon successful parsing, false otherwise.
 static bool
@@ -1635,8 +1634,6 @@  read_symbol_db_from_input(read_context&		 ctxt,
   if (!reader)
     return false;
 
-  bool found = false;
-
   if (!ctxt.get_corpus_node())
     for (;;)
       {
@@ -1663,17 +1660,9 @@  read_symbol_db_from_input(read_context&		 ctxt,
 	  return false;
 
 	if (has_fn_syms)
-	  {
-	    fn_symdb = build_elf_symbol_db(ctxt, node, true);
-	    if (fn_symdb)
-	      found = true;
-	  }
+	  fn_symdb = build_elf_symbol_db(ctxt, node, true);
 	else if (has_var_syms)
-	  {
-	    var_symdb = build_elf_symbol_db(ctxt, node, false);
-	    if (var_symdb)
-	      found = true;
-	  }
+	  var_symdb = build_elf_symbol_db(ctxt, node, false);
 
 	xmlTextReaderNext(reader.get());
       }
@@ -1690,22 +1679,16 @@  read_symbol_db_from_input(read_context&		 ctxt,
 	  else
 	    break;
 	  if (has_fn_syms)
-	    {
-	      fn_symdb = build_elf_symbol_db(ctxt, n, true);
-	      found = true;
-	    }
+	    fn_symdb = build_elf_symbol_db(ctxt, n, true);
 	  else if (has_var_syms)
-	    {
-	      var_symdb = build_elf_symbol_db(ctxt, n, false);
-	      found = true;
-	    }
+	    var_symdb = build_elf_symbol_db(ctxt, n, false);
 	  else
 	    break;
 	}
       ctxt.set_corpus_node(n);
     }
 
-  return found;
+  return true;
 }
 
 /// From an "elf-needed" XML_ELEMENT node, build a vector of strings
@@ -2017,24 +2000,16 @@  read_corpus_from_input(read_context& ctxt)
   string_elf_symbols_map_sptr fn_sym_db, var_sym_db;
 
   // Read the symbol databases.
-  bool is_ok = read_symbol_db_from_input(ctxt, fn_sym_db, var_sym_db);
-  if (is_ok)
-    {
-      // Note that it's possible that both fn_sym_db and var_sym_db
-      // are nil, due to potential suppression specifications.  That's
-      // fine.
-      corp.set_symtab(symtab_reader::symtab::load(fn_sym_db, var_sym_db));
-    }
+  read_symbol_db_from_input(ctxt, fn_sym_db, var_sym_db);
+  // Note that it's possible that both fn_sym_db and var_sym_db are nil,
+  // due to potential suppression specifications.  That's fine.
+  corp.set_symtab(symtab_reader::symtab::load(fn_sym_db, var_sym_db));
 
   ctxt.get_environment()->canonicalization_is_done(false);
 
   // Read the translation units.
-  do
-    {
-      translation_unit_sptr tu = read_translation_unit_from_input(ctxt);
-      is_ok = bool(tu);
-    }
-  while (is_ok);
+  while (read_translation_unit_from_input(ctxt))
+    ;
 
   if (ctxt.tracking_non_reachable_types())
     {