Fix aborting when reading .foo symbols from a ppc64 binary

On ppc64, the value of a function symbol is *usually* the pointer to
the function.  That function pointer value refers to an index inside
the ".opd" (Official Procedure Descriptor) section.  That section is a
record of tripplets values.  One of these values is the address of the
entry point of the function.  A debug information entry for a
function, in DWARF, refers to the entry point of the function; not to
the function pointer address.

So libabigail builds a table that associates a function entry point
address to its function pointer address.

So, if a function is named "foo", it has an entry in the symbol table
for the symbol name "foo"; the value of that symbol is the function
pointer address of foo, which refers to an offset in the ".opd"
section.

But then it turns out that there is also going to be an entry in the
symbol table for an artificial symbol named ".foo".  Pleast note the
dot before the suffix "foo".  The value of the ".foo" symbol is the
address of the entry point of the function "foo"; it's an address in
the ".text" section this time.

Libabigail's ELF symbols reading code was (wrongly) expecting all
entries in the symbol table that refer to function to have values that
are offset in the ".opd" section.  It wasn't expecting the ".foo"
symbols whose value are an address in the ".text" section.

This patch fixes the ELF symbol reading code to make it be aware of
the ".foo" symbols too.

	* abg-dwarf-reader.cc (read_context::find_opd_section): Fix
	comment.
	(read_context::load_symbol_maps): If for a given function entry
	point (that we got by looking at the ".opd" section for a given
	function pointer value) we already had an entry in the
	function_entry_address -> symbol, maybe it means that the previous
	entry that we had was from an entry in the symbol table which
	value was directly the entry point address of a function.  In that
	case, if the name of the symbol is "foo", the name of the symbol
	which value is directly the entry point address is ".foo".  What
	we do in this case is that we just keep the reference to the "foo"
	symbol in the function_entry_address -> symbol map.
	(read_context::address_is_in_opd_section): Define new member
	function.
	* tests/data/test-diff-pkg/gmp-4.3.1-10.el6.ppc64.rpm: New test input.
	* tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64.rpm: Likewise.
	* tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-10.el6.ppc64.rpm: Likewise.
	* tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-7.el6_2.2.ppc64.rpm: Likewise.
	* tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64--gmp-4.3.1-10.el6.ppc64-report-0.txt:
	New test reference output.
	* tests/data/Makefile.am: Add the new test input and reference
	output to source distribution.
	* tests/test-diff-pkg.cc (in_out_specs): Add the new test inputs
	and reference output to the set of inputs that are compared.

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

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index c6bc4e5f83239dc87518baf1bd64abe648d5a6dd..1bfcc2c880b6b58ce8438a5931daca034c998098 100644 (file)
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -4102,8 +4102,8 @@ public:
   }
 
   /// Return the "Official Procedure descriptors section."  This
-  /// section is named .opd, and usually present only on PPC64 ELFv1
-  /// binaries.
+  /// section is named .opd, and is usually present only on PPC64
+  /// ELFv1 binaries.
   ///
   /// @return the .opd section, if found.  Return nil otherwise.
   Elf_Scn*
@@ -4944,6 +4944,19 @@ public:
                      // of that function, to be able to get the
                      // function symbol that corresponds to a given
                      // function DIE, on ppc64.
+                     //
+                     // The value of the function pointer (the value
+                     // of the symbol) usually refers to the offset
+                     // of a table in the .opd section.  But
+                     // sometimes, for a symbol named "foo", the
+                     // corresponding symbol named ".foo" (note the
+                     // dot before foo) which value is the entry
+                     // point address of the function; that entry
+                     // point address refers to a region in the .text
+                     // section.
+                     //
+                     // So we are only interested in values of the
+                     // symbol that are in the .opd section.
                      GElf_Addr fn_desc_addr = sym->st_value;
                      GElf_Addr fn_entry_point_addr =
                        lookup_ppc64_elf_fn_entry_pointer_address(fn_desc_addr);
@@ -4952,12 +4965,45 @@ public:
 
                      if (it2 == fun_entry_addr_sym_map().end())
                        fun_entry_addr_sym_map()[fn_entry_point_addr] = symbol;
-                     else
-                       // 'symbol' must have been registered as an
-                       // alias for it2->second->get_main_symbol(),
-                       // right before the "if (ppc64)" statement.
-                       assert(it2->second->get_main_symbol()->
-                              does_alias(*symbol));
+                     else if (address_is_in_opd_section(fn_desc_addr))
+                       {
+                         // Either
+                         //
+                         // 'symbol' must have been registered as an
+                         // alias for it2->second->get_main_symbol(),
+                         // right before the "if (ppc64)" statement.
+                         //
+                         // Or
+                         //
+                         // if the name of 'symbol' is foo, then the
+                         // name of it2->second is ".foo".  That is,
+                         // foo is the name of the symbol when it
+                         // refers to the function descriptor in the
+                         // .opd section and ".foo" is an internal
+                         // name for the address of the entry point
+                         // of foo.
+                         //
+                         // In the latter case, we just want to keep
+                         // a refernce to "foo" as .foo is an
+                         // internal name.
+
+                         bool two_symbols_alias =
+                           it2->second->get_main_symbol()->does_alias(*symbol);
+                         bool symbol_is_foo_and_prev_symbol_is_dot_foo =
+                           (it2->second->get_name()
+                            == string(".") + symbol->get_name());
+
+                         assert(two_symbols_alias
+                                || symbol_is_foo_and_prev_symbol_is_dot_foo);
+
+                         if (symbol_is_foo_and_prev_symbol_is_dot_foo)
+                           // Let's just keep a reference of the
+                           // symbol that the user sees in the source
+                           // code (the one named foo).  The symbol
+                           // which name is prefixed with a "dot" is
+                           // an artificial one.
+                           fun_entry_addr_sym_map()[fn_entry_point_addr] = symbol;
+                       }
                   }
                }
              }
@@ -5207,6 +5253,24 @@ public:
     return false;
   }
 
+  /// Return true if an address is in the ".opd" section that is
+  /// present on the ppc64 platform.
+  ///
+  /// @param addr the address to consider.
+  ///
+  /// @return true iff @p addr is designates a word that is in the
+  /// ".opd" section.
+  bool
+  address_is_in_opd_section(Dwarf_Addr addr)
+  {
+    Elf_Scn * opd_section = find_opd_section();
+    if (!opd_section)
+      return false;
+    if (address_is_in_section(addr, opd_section))
+      return true;
+    return false;
+  }
+
   /// Get the section which a global variable address comes from.
   ///
   /// @param elf the elf handle to consider.

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index f121a7045d3c0d3828c7e1cb1f02a37f454722f5..99575345a3f0f7495c06e5b58aa5ffa592188b30 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -1207,6 +1207,11 @@ test-diff-pkg/qemu-img-rhev-2.3.0-7.el7.ppc64--qemu-img-rhev-2.3.0-20.el7.ppc64-
 test-diff-pkg/empty-pkg-libvirt-0.9.11.3-1.el7.ppc64.rpm \
 test-diff-pkg/empty-pkg-libvirt-1.2.17-13.el7_2.2.ppc64.rpm \
 test-diff-pkg/empty-pkg-report-0.txt \
+test-diff-pkg/gmp-4.3.1-10.el6.ppc64.rpm \
+test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64--gmp-4.3.1-10.el6.ppc64-report-0.txt \
+test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64.rpm \
+test-diff-pkg/gmp-debuginfo-4.3.1-10.el6.ppc64.rpm \
+test-diff-pkg/gmp-debuginfo-4.3.1-7.el6_2.2.ppc64.rpm \
 test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64.rpm \
 test-diff-pkg/tbb-4.3-3.20141204.fc23.x86_64.rpm \
 test-diff-pkg/tbb-debuginfo-4.1-9.20130314.fc22.x86_64.rpm \

diff --git a/tests/data/test-diff-pkg/gmp-4.3.1-10.el6.ppc64.rpm b/tests/data/test-diff-pkg/gmp-4.3.1-10.el6.ppc64.rpm
new file mode 100644 (file)
index 0000000..e10aaee
Binary files /dev/null and b/tests/data/test-diff-pkg/gmp-4.3.1-10.el6.ppc64.rpm differ

diff --git a/tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64--gmp-4.3.1-10.el6.ppc64-report-0.txt b/tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64--gmp-4.3.1-10.el6.ppc64-report-0.txt
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64.rpm b/tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64.rpm
new file mode 100644 (file)
index 0000000..1ee7006
Binary files /dev/null and b/tests/data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64.rpm differ

diff --git a/tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-10.el6.ppc64.rpm b/tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-10.el6.ppc64.rpm
new file mode 100644 (file)
index 0000000..d6d7086
Binary files /dev/null and b/tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-10.el6.ppc64.rpm differ

diff --git a/tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-7.el6_2.2.ppc64.rpm b/tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-7.el6_2.2.ppc64.rpm
new file mode 100644 (file)
index 0000000..7122aee
Binary files /dev/null and b/tests/data/test-diff-pkg/gmp-debuginfo-4.3.1-7.el6_2.2.ppc64.rpm differ

diff --git a/tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc
index 902a5cca2751d8a356c98d8f1d5d4d45c801aa92..0474c806e55634bae4fb4826761aa04fa63e1036 100644 (file)
--- a/tests/test-diff-pkg.cc
+++ b/tests/test-diff-pkg.cc
@@ -342,6 +342,18 @@ static InOutSpec in_out_specs[] =
     "data/test-diff-pkg/empty-pkg-report-0.txt",
     "output/test-diff-pkg/empty-pkg-report-0.txt"
   },
+  {
+    "data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64.rpm",
+    "data/test-diff-pkg/gmp-4.3.1-10.el6.ppc64.rpm",
+    "",
+    "",
+    "data/test-diff-pkg/gmp-debuginfo-4.3.1-7.el6_2.2.ppc64.rpm",
+    "data/test-diff-pkg/gmp-debuginfo-4.3.1-10.el6.ppc64.rpm",
+    "",
+    "",
+    "data/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64--gmp-4.3.1-10.el6.ppc64-report-0.txt",
+    "output/test-diff-pkg/gmp-4.3.1-7.el6_2.2.ppc64--gmp-4.3.1-10.el6.ppc64-report-0.txt"
+  },
   {
     "data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64.rpm",
     "data/test-diff-pkg/tbb-4.3-3.20141204.fc23.x86_64.rpm",