Bug 24552 - abidiff fails comparing a corpus against a corpus group

In this problem report, the issue is that when comparing two corpus
groups, especially when looking up function/variable symbols, the
get_fun_symbol_map() and get_var_symbol_map() member functions used
are corpus::get_{fun,var}_symbol_map, rather than
corpus_group::get_{fun, var}_symbol_map.  Note that the type
corpus_group inherits from the type corpus.  That leads to unexpected
comparison results, especially for symbols.

This patch fixes this by making the corpus::get_{fun, var}_symbol_map
member function be virtual and by using it during the lookup of
function/variable symbols.  That way, the right symbol map gets used.

	* include/abg-corpus.h (corpus{_group}::get_{fun,
	var}_symbol_map): Make these member functions virtual.
	* src/abg-corpus.cc (corpus::lookup_{function, variable}_symbol):
	Use the virtual corpus::get_{fun, var}_symbol_map() member
	function to get the symbols of the current corpus or corpus_group.
	* tests/data/Makefile.am: Add the new test input material below to
	source distribution.
	* tests/data/test-abidiff/test-PR24552-report0.txt: New test input.
	* tests/data/test-abidiff/test-PR24552-v0.abi: Likewise.
	* tests/data/test-abidiff/test-PR24552-v1.abi: Likewise.
	* tests/test-abidiff.cc (main): Support comparing corpus groups.
	(specs): Add the new test inputs to the harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

diff --git a/include/abg-corpus.h b/include/abg-corpus.h
index 1a25b4d..b336b93 100644 (file)
--- a/include/abg-corpus.h
+++ b/include/abg-corpus.h
@@ -164,7 +164,7 @@ public:
   const string_elf_symbols_map_sptr
   get_fun_symbol_map_sptr() const;
 
-  const string_elf_symbols_map_type&
+  virtual const string_elf_symbols_map_type&
   get_fun_symbol_map() const;
 
   const string_elf_symbols_map_sptr
@@ -182,7 +182,7 @@ public:
   const string_elf_symbols_map_sptr
   get_var_symbol_map_sptr() const;
 
-  const string_elf_symbols_map_type&
+  virtual const string_elf_symbols_map_type&
   get_var_symbol_map() const;
 
   const string_elf_symbols_map_sptr
@@ -373,10 +373,10 @@ public:
   virtual const corpus::variables&
   get_variables() const;
 
-  const string_elf_symbols_map_type&
+  virtual const string_elf_symbols_map_type&
   get_var_symbol_map() const;
 
-  const string_elf_symbols_map_type&
+  virtual const string_elf_symbols_map_type&
   get_fun_symbol_map() const;
 
   virtual const elf_symbols&

diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index 9c7657c..7d164bd 100644 (file)
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -1043,12 +1043,12 @@ corpus::get_sorted_undefined_var_symbols() const
 const elf_symbol_sptr
 corpus::lookup_function_symbol(const string& n) const
 {
-  if (!get_fun_symbol_map_sptr())
+  if (get_fun_symbol_map().empty())
     return elf_symbol_sptr();
 
   string_elf_symbols_map_type::const_iterator it =
-    get_fun_symbol_map_sptr()->find(n);
-  if ( it == get_fun_symbol_map_sptr()->end())
+    get_fun_symbol_map().find(n);
+  if ( it == get_fun_symbol_map().end())
     return elf_symbol_sptr();
   return it->second[0];
 }
@@ -1110,12 +1110,12 @@ const elf_symbol_sptr
 corpus::lookup_function_symbol(const string& symbol_name,
                               const elf_symbol::version& version) const
 {
-  if (!get_fun_symbol_map_sptr())
+  if (get_fun_symbol_map().empty())
     return elf_symbol_sptr();
 
   string_elf_symbols_map_type::const_iterator it =
-    get_fun_symbol_map_sptr()->find(symbol_name);
-  if ( it == get_fun_symbol_map_sptr()->end())
+    get_fun_symbol_map().find(symbol_name);
+  if ( it == get_fun_symbol_map().end())
     return elf_symbol_sptr();
 
   return find_symbol_by_version(version, it->second);
@@ -1139,12 +1139,12 @@ corpus::lookup_function_symbol(const elf_symbol& symbol) const
 const elf_symbol_sptr
 corpus::lookup_variable_symbol(const string& n) const
 {
-    if (!get_var_symbol_map_sptr())
+  if (get_var_symbol_map().empty())
     return elf_symbol_sptr();
 
   string_elf_symbols_map_type::const_iterator it =
-    get_var_symbol_map_sptr()->find(n);
-  if ( it == get_var_symbol_map_sptr()->end())
+    get_var_symbol_map().find(n);
+  if ( it == get_var_symbol_map().end())
     return elf_symbol_sptr();
   return it->second[0];
 }
@@ -1161,12 +1161,12 @@ const elf_symbol_sptr
 corpus::lookup_variable_symbol(const string& symbol_name,
                               const elf_symbol::version& version) const
 {
-    if (!get_var_symbol_map_sptr())
+  if (get_var_symbol_map().empty())
     return elf_symbol_sptr();
 
   string_elf_symbols_map_type::const_iterator it =
-    get_var_symbol_map_sptr()->find(symbol_name);
-  if ( it == get_var_symbol_map_sptr()->end())
+    get_var_symbol_map().find(symbol_name);
+  if ( it == get_var_symbol_map().end())
     return elf_symbol_sptr();
 
   return find_symbol_by_version(version, it->second);

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 49d1d21..3ad2bc7 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -81,6 +81,9 @@ test-abidiff/test-PR18166-libtirpc.so.report.txt \
 test-abidiff/test-PR18791-report0.txt   \
 test-abidiff/test-PR18791-v0.so.abi     \
 test-abidiff/test-PR18791-v1.so.abi     \
+test-abidiff/test-PR24552-report0.txt  \
+test-abidiff/test-PR24552-v0.abi       \
+test-abidiff/test-PR24552-v1.abi       \
 \
 test-abidiff-exit/test1-voffset-change-report0.txt \
 test-abidiff-exit/test1-voffset-change-report1.txt \

diff --git a/tests/data/test-abidiff/test-PR24552-report0.txt b/tests/data/test-abidiff/test-PR24552-report0.txt
new file mode 100644 (file)
index 0000000..a9d032e
--- /dev/null
+++ b/tests/data/test-abidiff/test-PR24552-report0.txt
@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+

diff --git a/tests/data/test-abidiff/test-PR24552-v0.abi b/tests/data/test-abidiff/test-PR24552-v0.abi
new file mode 100644 (file)
index 0000000..8577416
--- /dev/null
+++ b/tests/data/test-abidiff/test-PR24552-v0.abi
@@ -0,0 +1,11 @@
+<abi-corpus-group path='mypath' architecture='elf-arm-aarch64'>
+  <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
+    <elf-function-symbols>
+      <elf-symbol name='func' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    </elf-function-symbols>
+    <abi-instr version='1.0' address-size='64' path='file.h' comp-dir-path='common' language='LANG_C89'>
+      <function-decl name='func' filepath='file.h' line='22' column='1' visibility='default' binding='global' size-in-bits='64'>
+      </function-decl>
+    </abi-instr>
+  </abi-corpus>
+</abi-corpus-group>

diff --git a/tests/data/test-abidiff/test-PR24552-v1.abi b/tests/data/test-abidiff/test-PR24552-v1.abi
new file mode 100644 (file)
index 0000000..95e007a
--- /dev/null
+++ b/tests/data/test-abidiff/test-PR24552-v1.abi
@@ -0,0 +1,11 @@
+<abi-corpus-group path='mypath' architecture='elf-arm-aarch64'>
+  <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
+    <elf-function-symbols>
+      <elf-symbol name='func' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    </elf-function-symbols>
+    <abi-instr version='1.0' address-size='64' path='file.h' comp-dir-path='common' language='LANG_C89'>
+      <function-decl name='func' filepath='file.h' line='22' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='func'>
+      </function-decl>
+    </abi-instr>
+  </abi-corpus>
+</abi-corpus-group>

diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc
index c6391fe..d311833 100644 (file)
--- a/tests/test-abidiff.cc
+++ b/tests/test-abidiff.cc
@@ -101,6 +101,12 @@ static InOutSpec specs[] =
     "data/test-abidiff/test-PR18791-report0.txt",
     "output/test-abidiff/test-PR18791-report0.txt"
   },
+  {
+    "data/test-abidiff/test-PR24552-v0.abi",
+    "data/test-abidiff/test-PR24552-v1.abi",
+    "data/test-abidiff/test-PR24552-report0.txt",
+    "output/test-abidiff/test-PR24552-report0.txt"
+  },
   // This should be the last entry.
   {0, 0, 0, 0}
 };
@@ -117,10 +123,12 @@ using abigail::tools_utils::guess_file_type;
 using abigail::ir::environment;
 using abigail::ir::environment_sptr;
 using abigail::corpus_sptr;
+using abigail::corpus_group_sptr;
 using abigail::translation_unit;
 using abigail::translation_unit_sptr;
 using abigail::xml_reader::read_translation_unit_from_file;
 using abigail::xml_reader::read_corpus_from_native_xml_file;
+using abigail::xml_reader::read_corpus_group_from_native_xml_file;
 using abigail::comparison::corpus_diff_sptr;
 using abigail::comparison::translation_unit_diff_sptr;
 using abigail::comparison::compute_diff;
@@ -161,14 +169,18 @@ main(int, char*[])
       environment_sptr env(new environment);
       translation_unit_sptr tu1, tu2;
       corpus_sptr corpus1, corpus2;
+      corpus_group_sptr corpus_group1, corpus_group2;
       file_type t = guess_file_type(first_in_path);
       if (t == abigail::tools_utils::FILE_TYPE_NATIVE_BI)
        tu1 = read_translation_unit_from_file(first_in_path, env.get());
       else if (t == abigail::tools_utils::FILE_TYPE_XML_CORPUS)
        corpus1 = read_corpus_from_native_xml_file(first_in_path, env.get());
+      else if (t == abigail::tools_utils::FILE_TYPE_XML_CORPUS_GROUP)
+       corpus_group1 = read_corpus_group_from_native_xml_file(first_in_path,
+                                                              env.get());
       else
        abort();
-      if (!tu1 && !corpus1)
+      if (!tu1 && !corpus1 && !corpus_group1)
        {
          cerr << "failed to read " << first_in_path << "\n";
          is_ok = false;
@@ -180,9 +192,12 @@ main(int, char*[])
        tu2 = read_translation_unit_from_file(second_in_path, env.get());
       else if (t == abigail::tools_utils::FILE_TYPE_XML_CORPUS)
        corpus2 = read_corpus_from_native_xml_file(second_in_path, env.get());
+      else if (t == abigail::tools_utils::FILE_TYPE_XML_CORPUS_GROUP)
+       corpus_group2 = read_corpus_group_from_native_xml_file(first_in_path,
+                                                              env.get());
       else
        abort();
-      if (!tu2 && !corpus2)
+      if (!tu2 && !corpus2 && !corpus_group2)
        {
          cerr << "failed to read " << second_in_path << "\n";
          is_ok = false;
@@ -195,8 +210,10 @@ main(int, char*[])
       ctxt->show_locs(false);
       if (tu1)
        d1= compute_diff(tu1, tu2, ctxt);
-      else
+      else if (corpus1)
        d2 = compute_diff(corpus1, corpus2, ctxt);
+      else if (corpus_group1)
+       d2 = compute_diff(corpus_group1, corpus_group2, ctxt);
       ofstream of(out_path.c_str(), std::ios_base::trunc);
       if (!of.is_open())
        {