Identify a function using its symbol name and version

When a function symbol S has several versions and several different
functions with different names have those different versions of S as
underlying symbols, libabigail could mistakenly take one function for
another.  This is because there are cases where libabigail identifies
the function using its symbol name without taking the version name
into account.

This patch fixes that.

	* src/abg-default-reporter.cc (default_reporter::report): In C,
	tell the user about the underlying function symbol name only if
	said symbol name is different from the name of the function.
	* src/abg-ir.cc (function_decl::get_id): If the function has an
	underlying symbol, use the symbol name and version as the function
	ID.  But if the function symbol has an alias then use the linkage
	name as the ID.
	* tests/data/test-diff-pkg/elfutils-debuginfo-0.170-4.el7.x86_64.rpm:
	New binary test input.
	* tests/data/test-diff-pkg/elfutils-debuginfo-0.171-1.el7.x86_64.rpm: Likewise.
	* tests/data/test-diff-pkg/elfutils-devel-0.170-4.el7.x86_64.rpm: Likewise.
	* tests/data/test-diff-pkg/elfutils-devel-0.171-1.el7.x86_64.rpm: Likewise.
	* tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64.rpm: Likewise.
	* tests/data/test-diff-pkg/elfutils-libs-0.171-1.el7.x86_64.rpm: Likewise.
	* tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt:
	New reference test output.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-diff-pkg.cc (in_out_specs): Integrate the new test
	inputs above into the harness.

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

diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index c23d649995bddee2342a361cc9ddc79ecf375696..63e1e3544e1493b613e128d94777913c13c77845 100644 (file)
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -1900,14 +1900,27 @@ default_reporter::report(const corpus_diff& d, ostream& out,
                  << fn->get_pretty_representation() << "'";
              report_loc_info((*i)->second_function_decl(), *ctxt, out);
              out << " has some indirect sub-type changes:\n";
-             if ((fn->get_symbol()->has_aliases()
+             if (// The symbol of the function has aliases and the
+                 // function is not a cdtor (yeah because c++ cdtors
+                 // usually have several aliases).
+                 (fn->get_symbol()->has_aliases()
                   && !(is_member_function(fn)
                        && get_member_function_is_ctor(fn))
                   && !(is_member_function(fn)
                        && get_member_function_is_dtor(fn)))
-                 || (is_c_language(get_translation_unit(fn)->get_language())
-                     && fn->get_name() != fn->get_linkage_name()))
+                 || // We are in C and the name of the function is
+                    // different from the symbol name -- without
+                    // taking the possible symbol version into
+                    // account (this usually means the programmers
+                    // was playing tricks with symbol names and
+                    // versions).
+                 (is_c_language(get_translation_unit(fn)->get_language())
+                     && fn->get_name() != fn->get_symbol()->get_name()))
                {
+                 // As the name of the symbol of the function doesn't
+                 // seem to be obvious here, make sure to tell the
+                 // user about the name of the (function) symbol she
+                 // is looking at here.
                  int number_of_aliases =
                    fn->get_symbol()->get_number_of_aliases();
                  if (number_of_aliases == 0)

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 55fa7fcd552256e6bce1cbf2b48da233095d9800..ec9bd4ed08800492f098e7149eb807fdf343c488 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -15748,10 +15748,18 @@ function_decl::get_id() const
   if (priv_->id_.empty())
     {
       const environment* env = get_type()->get_environment();
-      if (!get_linkage_name().empty())
+      if (elf_symbol_sptr s = get_symbol())
+       {
+         if (s->has_aliases())
+           // The symbol has several aliases, so let's use the
+           // linkage name of the function as its ID.
+           priv_->id_ = env->intern(get_linkage_name());
+         else
+           // Let's use the full symbol name with its version as ID.
+           priv_->id_ = env->intern(s->get_id_string());
+       }
+      else if (!get_linkage_name().empty())
        priv_->id_= env->intern(get_linkage_name());
-      else if (elf_symbol_sptr s = get_symbol())
-       priv_->id_ = env->intern(s->get_id_string());
       else
        priv_->id_ = env->intern(get_pretty_representation());
     }

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 3bcb3d2b27d1e20213f96230ecc67a3d550fa1ca..ad296b48215dca9ff3809ef96ea5973a8ec21ca6 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -1383,6 +1383,13 @@ test-diff-pkg/libxfce4ui-devel-debuginfo-4.12.1-8.fc27.ppc64.rpm \
 test-diff-pkg/libxfce4ui-debuginfo-4.12.1-8.fc27.ppc64.rpm \
 test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-0.txt \
 test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-ok-0.txt \
+test-diff-pkg/elfutils-debuginfo-0.170-4.el7.x86_64.rpm \
+test-diff-pkg/elfutils-debuginfo-0.171-1.el7.x86_64.rpm \
+test-diff-pkg/elfutils-devel-0.170-4.el7.x86_64.rpm \
+test-diff-pkg/elfutils-devel-0.171-1.el7.x86_64.rpm \
+test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64.rpm \
+test-diff-pkg/elfutils-libs-0.171-1.el7.x86_64.rpm \
+test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt \
 \
 test-fedabipkgdiff/dbus-glib-0.104-3.fc23.x86_64.rpm \
 test-fedabipkgdiff/dbus-glib-debuginfo-0.104-3.fc23.x86_64.rpm \

diff --git a/tests/data/test-diff-pkg/elfutils-debuginfo-0.170-4.el7.x86_64.rpm b/tests/data/test-diff-pkg/elfutils-debuginfo-0.170-4.el7.x86_64.rpm
new file mode 100644 (file)
index 0000000..81d08d3
Binary files /dev/null and b/tests/data/test-diff-pkg/elfutils-debuginfo-0.170-4.el7.x86_64.rpm differ

diff --git a/tests/data/test-diff-pkg/elfutils-debuginfo-0.171-1.el7.x86_64.rpm b/tests/data/test-diff-pkg/elfutils-debuginfo-0.171-1.el7.x86_64.rpm
new file mode 100644 (file)
index 0000000..463722c
Binary files /dev/null and b/tests/data/test-diff-pkg/elfutils-debuginfo-0.171-1.el7.x86_64.rpm differ

diff --git a/tests/data/test-diff-pkg/elfutils-devel-0.170-4.el7.x86_64.rpm b/tests/data/test-diff-pkg/elfutils-devel-0.170-4.el7.x86_64.rpm
new file mode 100644 (file)
index 0000000..fc5cacd
Binary files /dev/null and b/tests/data/test-diff-pkg/elfutils-devel-0.170-4.el7.x86_64.rpm differ

diff --git a/tests/data/test-diff-pkg/elfutils-devel-0.171-1.el7.x86_64.rpm b/tests/data/test-diff-pkg/elfutils-devel-0.171-1.el7.x86_64.rpm
new file mode 100644 (file)
index 0000000..58b3a8e
Binary files /dev/null and b/tests/data/test-diff-pkg/elfutils-devel-0.171-1.el7.x86_64.rpm differ

diff --git a/tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt b/tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt
new file mode 100644 (file)
index 0000000..37e6b48
--- /dev/null
+++ b/tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt
@@ -0,0 +1,13 @@
+================ changes of 'libdw-0.170.so'===============
+  Functions changes summary: 0 Removed, 0 Changed (175 filtered out), 4 Added functions
+  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+  4 Added functions:
+
+    'function int dwarf_cu_info(Dwarf_CU*, Dwarf_Half*, uint8_t*, Dwarf_Die*, Dwarf_Die*, uint64_t*, uint8_t*, uint8_t*)'    {dwarf_cu_info@@ELFUTILS_0.171}
+    'function Dwarf_Die* dwarf_die_addr_die(Dwarf*, void*, Dwarf_Die*)'    {dwarf_die_addr_die@@ELFUTILS_0.171}
+    'function int dwarf_get_units(Dwarf*, Dwarf_CU*, Dwarf_CU**, Dwarf_Half*, uint8_t*, Dwarf_Die*, Dwarf_Die*)'    {dwarf_get_units@@ELFUTILS_0.171}
+    'function int dwarf_getabbrevattr_data(Dwarf_Abbrev*, size_t, unsigned int*, unsigned int*, Dwarf_Sword*, Dwarf_Off*)'    {dwarf_getabbrevattr_data@@ELFUTILS_0.171}
+
+================ end of changes of 'libdw-0.170.so'===============
+

diff --git a/tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64.rpm b/tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64.rpm
new file mode 100644 (file)
index 0000000..edc9346
Binary files /dev/null and b/tests/data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64.rpm differ

diff --git a/tests/data/test-diff-pkg/elfutils-libs-0.171-1.el7.x86_64.rpm b/tests/data/test-diff-pkg/elfutils-libs-0.171-1.el7.x86_64.rpm
new file mode 100644 (file)
index 0000000..835daaa
Binary files /dev/null and b/tests/data/test-diff-pkg/elfutils-libs-0.171-1.el7.x86_64.rpm differ

diff --git a/tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc
index 4eb19e4566ab09d74aaf216bd94589670e335122..77911185492b9b9dc57e1acd3b0609d0728f2d70 100644 (file)
--- a/tests/test-diff-pkg.cc
+++ b/tests/test-diff-pkg.cc
@@ -539,6 +539,18 @@ static InOutSpec in_out_specs[] =
     "data/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-ok-0.txt",
     "output/test-diff-pkg/libxfce4ui-devel-4.12.1-8.fc27.ppc64-self-report-ok-0.txt"
   },
+  {
+    "data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64.rpm",
+    "data/test-diff-pkg/elfutils-libs-0.171-1.el7.x86_64.rpm",
+    "--fail-no-dbg",
+    "",
+    "data/test-diff-pkg/elfutils-debuginfo-0.170-4.el7.x86_64.rpm",
+    "data/test-diff-pkg/elfutils-debuginfo-0.171-1.el7.x86_64.rpm",
+    "data/test-diff-pkg/elfutils-devel-0.170-4.el7.x86_64.rpm",
+    "data/test-diff-pkg/elfutils-devel-0.171-1.el7.x86_64.rpm",
+    "data/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt",
+    "output/test-diff-pkg/elfutils-libs-0.170-4.el7.x86_64-multiple-sym-vers-report-0.txt"
+  },
 #endif //WITH_RPM
 
 #ifdef WITH_DEB