From f754d8111656b2ebb480952035ecd0746d3c2287 Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Thu, 16 May 2019 17:23:40 +0200 Subject: [PATCH] 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 --- include/abg-corpus.h | 8 +++---- src/abg-corpus.cc | 24 +++++++++---------- tests/data/Makefile.am | 3 +++ .../test-abidiff/test-PR24552-report0.txt | 3 +++ tests/data/test-abidiff/test-PR24552-v0.abi | 11 +++++++++ tests/data/test-abidiff/test-PR24552-v1.abi | 11 +++++++++ tests/test-abidiff.cc | 23 +++++++++++++++--- 7 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 tests/data/test-abidiff/test-PR24552-report0.txt create mode 100644 tests/data/test-abidiff/test-PR24552-v0.abi create mode 100644 tests/data/test-abidiff/test-PR24552-v1.abi diff --git a/include/abg-corpus.h b/include/abg-corpus.h index 1a25b4d1..b336b93c 100644 --- 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 9c7657c4..7d164bdc 100644 --- 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 49d1d21a..3ad2bc7c 100644 --- 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 index 00000000..a9d032e7 --- /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 index 00000000..85774163 --- /dev/null +++ b/tests/data/test-abidiff/test-PR24552-v0.abi @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/tests/data/test-abidiff/test-PR24552-v1.abi b/tests/data/test-abidiff/test-PR24552-v1.abi new file mode 100644 index 00000000..95e007a8 --- /dev/null +++ b/tests/data/test-abidiff/test-PR24552-v1.abi @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc index c6391fe2..d3118338 100644 --- 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()) { -- 2.43.5