Detect and report changes in ELF architecture

Libabigail does not take in account the architecture of the ELF file
it reads.  This patch changes that to represent the ELF architecture
as a string, detect when that architecture changes accross two corpora
being compared and emit a report about that change.

	* configure.ac: Detect the presence of libebl.a and add it to the
	list of library we depend on to build libabigail.  Report when
	libelf.so is not found.
	* include/abg-comparison.h:
	(diff_context::show_architecture_change): Declare new accessors.
	(corpus_diff::architecture_changed): Declare new method.
	* include/abg-corpus.h (corpus::{get,set}_architecture_name):
	Declare new accessors.
	* src/abg-comparison.cc
	(diff_context::priv::show_architecture_change_): New data member.
	(diff_context::priv::priv): Initialize it.
	(diff_context::show_architecture_change): Define new accessors.
	(function_decl_diff::report): Report when the size/alignment of
	the function address changes.
	(corpus_diff::priv::architectures_equal_): New data member.
	(corpus_diff::priv::priv): Initialize it.
	(corpus_diff::priv::emit_diff_stats): Take in account changes of
	architecture.
	(corpus_diff::architecture_changed): Define new method.
	(corpus_diff::length): Take in account changes of architecture.
	(corpus_diff::report): Report about changes of architecture.
	(compute_diff): In the overload for corpus_diff_sptr, detect
	changes fo architecture.
	* src/abg-corpus.cc (corpus_priv::architecture_name): Define new
	data member.
	(corpus::{get,set}_architecture_name): Define new method.
	* src/abg-dwarf-reader.cc: Include elfutils/libebl.h to use
	ebl_openbackend() and ebl_backend_name()
	(read_context::elf_architecture_): Define new data member.
	(read_context::elf_architecture): Define new accessor.
	(read_context::{load_elf_architecture, load_remaining_elf_data}):
	Define new methods.
	(read_corpus_from_elf): Use ctxt.load_remaining_elf_data() in lieu
	of ctxt.load_dt_soname_and_needed.  Stick the architecture into
	the corpus.
	* src/abg-reader.cc (read_corpus_from_input): Read the
	'architecture' XML property.
	* src/abg-writer.cc (write_corpus_to_native_xml): Write the
	'architecture' XML property.
	* tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-32.so: New
	test input file.
	* tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-64.so:
	Likewise.
	* tests/data/test-diff-dwarf/test-23-diff-arch-report-0.txt:
	Likewise.
	* tests/data/test-diff-dwarf/test-23-diff-arch-v0.cc: Source code
	for the binary test input files above.
	* tests/data/Makefile.am: Add the new test input files to the
	source distribution.
	* tests/test-diff-dwarf.cc (in_out_specs): Add the new test input
	data to the set of input data to run this test harness over.
	* tests/test-read-dwarf.cc (main): Do not take the architecture in
	account during comparisons.

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

diff --git a/configure.ac b/configure.ac
index 0c9d75cfdd2b932c4dc22ad8723d1b32a5040ca7..80ca8c575da5e40bb3129670b2e8eff2e8d330a5 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -66,22 +66,39 @@ LT_INIT
 AC_LANG([C++])
 AC_LANG_COMPILER_REQUIRE
 
-dnl Check for dependency: libdw (elfutils)
+dnl Check for dependency: libelf, libdw, libebl (elfutils)
+ELF_LIBS=
+AC_CHECK_LIB([elf], [elf_end], [ELF_LIBS="-lelf"])
+AC_CHECK_HEADER([libelf.h],
+               [],
+               [AC_MSG_ERROR([could not find libelf.h])])
+
 DW_LIBS=
 AC_CHECK_LIB(dw, dwfl_begin, [DW_LIBS=-ldw])
 AC_CHECK_HEADER(elfutils/libdwfl.h,
                [],
                [AC_MSG_ERROR([could not find elfutils/libdwfl.h installed])])
-AC_CHECK_LIB([elf], [elf_end], [ELF_LIBS="-lelf"])
-AC_CHECK_HEADER([libelf.h],
+
+EBL_LIBS=
+AC_CHECK_LIB(ebl, ebl_openbackend, [EBL_LIBS=-lebl], [], [-ldl -lelf])
+AC_CHECK_HEADER(elfutils/libebl.h,
                [],
-               [AC_MSG_ERROR([could not find libelf.h])])
+               [AC_MSG_ERROR([could not find elfutils/libebl.h installed])])
+
+if test x$ELF_LIBS = x; then
+   AC_MSG_ERROR([could not find elfutils elf library installed])
+fi
 
 if test x$DW_LIBS = x; then
    AC_MSG_ERROR([could not find elfutils dwarf library installed])
 fi
 
+if test x$EBL_LIBS = x; then
+   AC_MSG_ERROR([could not find elfutils backend library installed])
+fi
+
 AC_SUBST(DW_LIBS)
+AC_SUBST(EBL_LIBS)
 AC_SUBST([ELF_LIBS])
 
 dnl Check for dependency: libxml
@@ -174,7 +191,7 @@ AM_CONDITIONAL(ENABLE_MANUAL, test x$ENABLE_MANUAL = xyes)
 
 dnl Set the list of libraries libabigail depends on
 
-DEPS_LIBS="$XML_LIBS $LIBZIP_LIBS $DW_LIBS $ELF_LIBS"
+DEPS_LIBS="$XML_LIBS $LIBZIP_LIBS $ELF_LIBS $EBL_LIBS $DW_LIBS"
 AC_SUBST(DEPS_LIBS)
 
 if test x$ABIGAIL_DEVEL != x; then

diff --git a/include/abg-comparison.h b/include/abg-comparison.h
index 5c6cd3a05fade6bba2cc3e9a211bcd14821f3751..20e8ab97bfc467a82b23d0e57e1a94a288c0012d 100644 (file)
--- a/include/abg-comparison.h
+++ b/include/abg-comparison.h
@@ -850,6 +850,12 @@ public:
   bool
   show_soname_change() const;
 
+  void
+  show_architecture_change(bool f);
+
+  bool
+  show_architecture_change() const;
+
   void
   show_deleted_fns(bool f);
 
@@ -2158,6 +2164,9 @@ public:
   bool
   soname_changed() const;
 
+  bool
+  architecture_changed() const;
+
   const string_function_ptr_map&
   deleted_functions() const;
 

diff --git a/include/abg-corpus.h b/include/abg-corpus.h
index 0fb9f763d02f8a2ed17f78168d9b6a787c8358c9..cd0c49d7a248148ff48d8ca289c2a562a5d16d40 100644 (file)
--- a/include/abg-corpus.h
+++ b/include/abg-corpus.h
@@ -101,6 +101,12 @@ public:
   void
   set_soname(const string&);
 
+  const string&
+  get_architecture_name();
+
+  void
+  set_architecture_name(const string&);
+
   bool
   is_empty() const;
 

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 12b3b09a7d6dfffe765674d185cc8635bc61696c..36480740606fe144225b53f7f694257eefc68bcd 100644 (file)
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -2272,6 +2272,7 @@ struct diff_context::priv
   bool                                 forbid_traversing_a_node_twice_;
   bool                                 show_stats_only_;
   bool                                 show_soname_change_;
+  bool                                 show_architecture_change_;
   bool                                 show_deleted_fns_;
   bool                                 show_changed_fns_;
   bool                                 show_added_fns_;
@@ -2291,6 +2292,7 @@ struct diff_context::priv
       forbid_traversing_a_node_twice_(true),
       show_stats_only_(false),
       show_soname_change_(true),
+      show_architecture_change_(true),
       show_deleted_fns_(true),
       show_changed_fns_(true),
       show_added_fns_(true),
@@ -2777,6 +2779,22 @@ bool
 diff_context::show_soname_change() const
 {return priv_->show_soname_change_;}
 
+/// Setter for the property that says if the comparison module should
+/// show the architecture changes in its report.
+///
+/// @param f the new value of the property.
+void
+diff_context::show_architecture_change(bool f)
+{priv_->show_architecture_change_ = f;}
+
+/// Getter for the property that says if the comparison module should
+/// show the architecture changes in its report.
+///
+/// @return the value of the property.
+bool
+diff_context::show_architecture_change() const
+{return priv_->show_architecture_change_;}
+
 /// Set a flag saying to show the deleted functions.
 ///
 /// @param f true to show deleted functions.
@@ -9038,6 +9056,27 @@ function_decl_diff::report(ostream& out, const string& indent) const
            << " is now declared inline\n";
     }
 
+  // Report about the size of the function address
+  if (ff->get_type()->get_size_in_bits() != sf->get_type()->get_size_in_bits())
+    {
+      out << indent << "address size of function changed from "
+         << ff->get_type()->get_size_in_bits()
+         << " bits to "
+         << sf->get_type()->get_size_in_bits()
+         << " bits\n";
+    }
+
+  // Report about the alignment of the function address
+  if (ff->get_type()->get_alignment_in_bits()
+      != sf->get_type()->get_alignment_in_bits())
+    {
+      out << indent << "address alignment of function changed from "
+         << ff->get_type()->get_alignment_in_bits()
+         << " bits to "
+         << sf->get_type()->get_alignment_in_bits()
+         << " bits\n";
+    }
+
   // Report about return type differences.
   if (priv_->return_type_diff_ && priv_->return_type_diff_->to_be_reported())
     {
@@ -9862,6 +9901,7 @@ struct corpus_diff::priv
   corpus_diff::diff_stats              diff_stats_;
   bool                                 filters_and_suppr_applied_;
   bool                                 sonames_equal_;
+  bool                                 architectures_equal_;
   edit_script                          fns_edit_script_;
   edit_script                          vars_edit_script_;
   edit_script                          unrefed_fn_syms_edit_script_;
@@ -9880,7 +9920,8 @@ struct corpus_diff::priv
   priv()
     : finished_(false),
       filters_and_suppr_applied_(false),
-      sonames_equal_(false)
+      sonames_equal_(false),
+      architectures_equal_(false)
   {}
 
   bool
@@ -10297,6 +10338,9 @@ corpus_diff::priv::emit_diff_stats(const diff_stats&    s,
   if (!sonames_equal_)
     out << indent << "ELF SONAME changed\n";
 
+  if (!architectures_equal_)
+    out << indent << "ELF architecture changed\n";
+
   // function changes summary
   out << indent << "Functions changes summary: ";
   out << s.num_func_removed() << " Removed, ";
@@ -10543,13 +10587,20 @@ edit_script&
 corpus_diff::variable_changes() const
 {return priv_->vars_edit_script_;}
 
-/// Test if the soname of the underying corpus has changed.
+/// Test if the soname of the underlying corpus has changed.
 ///
 /// @return true iff the soname has changed.
 bool
 corpus_diff::soname_changed() const
 {return !priv_->sonames_equal_;}
 
+/// Test if the architecture of the underlying corpus has changed.
+///
+/// @return true iff the architecture has changed.
+bool
+corpus_diff::architecture_changed() const
+{return !priv_->architectures_equal_;}
+
 /// Getter for the deleted functions of the diff.
 ///
 /// @return the the deleted functions of the diff.
@@ -10661,6 +10712,7 @@ unsigned
 corpus_diff::length() const
 {
   return (soname_changed()
+         + architecture_changed()
          + priv_->deleted_fns_.size()
          + priv_->added_fns_.size()
          + priv_->changed_fns_.size()
@@ -10968,6 +11020,12 @@ corpus_diff::report(ostream& out, const string& indent) const
        << first_corpus()->get_soname() << "' to '"
        << second_corpus()->get_soname() << "'\n\n";
 
+  if (context()->show_architecture_change()
+      && !priv_->architectures_equal_)
+    out << indent << "architecture changed from '"
+       << first_corpus()->get_architecture_name() << "' to '"
+       << second_corpus()->get_architecture_name() << "'\n\n";
+
   if (context()->show_deleted_fns())
     {
       if (s.num_func_removed() == 1)
@@ -11392,6 +11450,9 @@ compute_diff(const corpus_sptr  f,
 
   r->priv_->sonames_equal_ = f->get_soname() == s->get_soname();
 
+  r->priv_->architectures_equal_ =
+    f->get_architecture_name() == s->get_architecture_name();
+
   diff_utils::compute_diff<fns_it_type, eq_type>(f->get_functions().begin(),
                                                 f->get_functions().end(),
                                                 s->get_functions().begin(),

diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index e2ee9c254889c44a3b81fcf80140dbd5e1249c99..f76daf2dd9936fd4b4fc3c0273b7fb9e80b99c1a 100644 (file)
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -104,6 +104,7 @@ struct corpus::priv
   string                       path;
   vector<string>               needed;
   string                       soname;
+  string                       architecture_name;
   translation_units            members;
   vector<function_decl*>       fns;
   vector<var_decl*>            vars;
@@ -1026,6 +1027,28 @@ void
 corpus::set_soname(const string& soname)
 {priv_->soname = soname;}
 
+/// Getter for the architecture name of the corpus.
+///
+/// This property is meaningful for e.g, corpora built from ELF shared
+/// library files.  In that case, this is a string representation of
+/// the Elf{32,64}_Ehdr::e_machine field.
+///
+/// @return the architecture name string.
+const string&
+corpus::get_architecture_name()
+{return priv_->architecture_name;}
+
+/// Setter for the architecture name of the corpus.
+///
+/// This property is meaningful for e.g, corpora built from ELF shared
+/// library files.  In that case, this is a string representation of
+/// the Elf{32,64}_Ehdr::e_machine field.
+///
+/// @param arch the architecture name string.
+void
+corpus::set_architecture_name(const string& arch)
+{priv_->architecture_name = arch;}
+
 /// Tests if the corpus contains no translation unit.
 ///
 /// @return true if the corpus contains no translation unit.

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 383a7b8b0403eeb57c817180e3b4a585ffc22017..daa53ee84d4f1aeb6b1f14c9e65fec33f852fa18 100644 (file)
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -35,6 +35,7 @@
 #include <cstring>
 #include <cmath>
 #include <elfutils/libdwfl.h>
+#include <elfutils/libebl.h>
 #include <dwarf.h>
 #include <tr1/unordered_map>
 #include <stack>
@@ -1465,6 +1466,7 @@ class read_context
   string_elf_symbols_map_sptr  undefined_var_syms_;
   vector<string>               dt_needed_;
   string                       dt_soname_;
+  string                       elf_architecture_;
 
   read_context();
 
@@ -2311,6 +2313,11 @@ public:
   dt_soname() const
   {return dt_soname_;}
 
+  /// Getter for the ELF architecture of the current file.
+  const string&
+  elf_architecture() const
+  {return elf_architecture_;}
+
   /// Getter for the map of global variables symbol address -> global
   /// variable symbol index.
   ///
@@ -2587,6 +2594,29 @@ public:
       }
   }
 
+  /// Read the string representing the architecture of the current ELF
+  /// file.
+  void
+  load_elf_architecture()
+  {
+    if (!elf_handle())
+      return;
+
+    elf_architecture_ = ebl_backend_name(ebl_openbackend(elf_handle()));
+  }
+
+  /// Load various ELF data.
+  ///
+  /// This function loads ELF data that are not symbol maps or debug
+  /// info.  That is, things like various tags, elf architecture and
+  /// so on.
+  void
+  load_remaining_elf_data()
+  {
+    load_dt_soname_and_needed();
+    load_elf_architecture();
+  }
+
   /// This is a sub-routine of maybe_adjust_fn_sym_address and
   /// maybe_adjust_var_sym_address.
   ///
@@ -6820,7 +6850,7 @@ read_corpus_from_elf(read_context& ctxt, corpus_sptr& resulting_corp)
   if (!ctxt.load_symbol_maps())
     status |= STATUS_NO_SYMBOLS_FOUND;
 
-  ctxt.load_dt_soname_and_needed();
+  ctxt.load_remaining_elf_data();
 
   if (status & STATUS_NO_SYMBOLS_FOUND)
     return status;
@@ -6832,6 +6862,7 @@ read_corpus_from_elf(read_context& ctxt, corpus_sptr& resulting_corp)
   corp->set_origin(corpus::DWARF_ORIGIN);
   corp->set_soname(ctxt.dt_soname());
   corp->set_needed(ctxt.dt_needed());
+  corp->set_architecture_name(ctxt.elf_architecture());
 
   corp->set_fun_symbol_map(ctxt.fun_syms_sptr());
   corp->set_undefined_fun_symbol_map(ctxt.undefined_fun_syms_sptr());

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 246a68e5635f0d54ad31dee49386d427d678b415..879631da858b0eb312f7451ea05de85ecda10c53 100644 (file)
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -1017,6 +1017,11 @@ read_corpus_from_input(read_context& ctxt)
   if (path_str)
     corp.set_path(reinterpret_cast<char*>(path_str.get()));
 
+  xml::xml_char_sptr architecture_str =
+    XML_READER_GET_ATTRIBUTE(reader, "architecture");
+  if (architecture_str)
+    corp.set_architecture_name(reinterpret_cast<char*>(architecture_str.get()));
+
   xml::xml_char_sptr soname_str = XML_READER_GET_ATTRIBUTE(reader, "soname");
   if (soname_str)
     corp.set_soname(reinterpret_cast<char*>(soname_str.get()));

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 3f528d7d398b178ae40665aeb8d465ba55e76f30..d832de889dade5a3fc2da2a45a0451c9e6a659f7 100644 (file)
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -2507,6 +2507,9 @@ write_corpus_to_native_xml(const corpus_sptr      corpus,
   if (!corpus->get_path().empty())
     out << " path='" << corpus->get_path() << "'";
 
+  if (!corpus->get_architecture_name().empty())
+    out << " architecture='" << corpus->get_architecture_name()<< "'";
+
   if (!corpus->get_soname().empty())
     out << " soname='" << corpus->get_soname()<< "'";
 

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 7708fc9e0bcfbcab5f95f3f6cf1333c84012c83a..7637b9115d9fcfbbb6bf71d3fdd9fb041619e9a6 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -185,6 +185,10 @@ test-diff-dwarf/libtest22-changed-parm-c-v1.so \
 test-diff-dwarf/test22-changed-parm-c-report-0.txt \
 test-diff-dwarf/test22-changed-parm-c-v0.c \
 test-diff-dwarf/test22-changed-parm-c-v1.c \
+test-diff-dwarf/libtest-23-diff-arch-v0-32.so \
+test-diff-dwarf/libtest-23-diff-arch-v0-64.so \
+test-diff-dwarf/test-23-diff-arch-report-0.txt \
+test-diff-dwarf/test-23-diff-arch-v0.cc \
 \
 test-read-dwarf/test0                  \
 test-read-dwarf/test0.abi                      \

diff --git a/tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-32.so b/tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-32.so
new file mode 100755 (executable)
index 0000000..af815cf
Binary files /dev/null and b/tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-32.so differ

diff --git a/tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-64.so b/tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-64.so
new file mode 100755 (executable)
index 0000000..01c4845
Binary files /dev/null and b/tests/data/test-diff-dwarf/libtest-23-diff-arch-v0-64.so differ

diff --git a/tests/data/test-diff-dwarf/test-23-diff-arch-report-0.txt b/tests/data/test-diff-dwarf/test-23-diff-arch-report-0.txt
new file mode 100644 (file)
index 0000000..e897d20
--- /dev/null
+++ b/tests/data/test-diff-dwarf/test-23-diff-arch-report-0.txt
@@ -0,0 +1,12 @@
+ELF architecture changed
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+architecture changed from 'elf_i386' to 'elf_x86_64'
+
+1 function with some indirect sub-type change:
+
+  [C]'function int foo()' has some indirect sub-type changes:
+    address size of function changed from 32 bits to 64 bits
+    address alignment of function changed from 32 bits to 64 bits
+

diff --git a/tests/data/test-diff-dwarf/test-23-diff-arch-v0.cc b/tests/data/test-diff-dwarf/test-23-diff-arch-v0.cc
new file mode 100644 (file)
index 0000000..052e129
--- /dev/null
+++ b/tests/data/test-diff-dwarf/test-23-diff-arch-v0.cc
@@ -0,0 +1,8 @@
+// Compile this with:
+// gcc -g -Wall -shared -m32 -o libtest-23-diff-arch-v0-32.so test-23-diff-arch-v0.cc
+
+// And then, compile this again with:
+// gcc -g -Wall -shared -o libtest-23-diff-arch-v0-64.so test-23-diff-arch-v0.cc
+int
+foo()
+{return 0;}

diff --git a/tests/test-diff-dwarf.cc b/tests/test-diff-dwarf.cc
index 82cf85ea1e9f5135e10469aa1d5aeb8ec256ba4b..7486b2c6efe1a8ead611fa8f44495a611a010f22 100644 (file)
--- a/tests/test-diff-dwarf.cc
+++ b/tests/test-diff-dwarf.cc
@@ -200,6 +200,12 @@ InOutSpec in_out_specs[] =
     "data/test-diff-dwarf/test22-changed-parm-c-report-0.txt",
     "output/test-diff-dwarf/test22-changed-parm-c-report-0.txt"
   },
+  {
+    "data/test-diff-dwarf/libtest-23-diff-arch-v0-32.so",
+    "data/test-diff-dwarf/libtest-23-diff-arch-v0-64.so",
+    "data/test-diff-dwarf/test-23-diff-arch-report-0.txt",
+    "output/test-diff-dwarf/test-23-diff-arch-report-0.txt"
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL}
 };

diff --git a/tests/test-read-dwarf.cc b/tests/test-read-dwarf.cc
index e8c12ea13f096d2dcced7e1c167c30ac822132b8..f7d84f55bd28ebac48def5f3a88dfdb4f675de03 100644 (file)
--- a/tests/test-read-dwarf.cc
+++ b/tests/test-read-dwarf.cc
@@ -121,6 +121,10 @@ main()
          continue;
        }
       corp->set_path(s->in_elf_path);
+      // Do not take architecture names in comparison so that these
+      // test input binaries can come from whatever arch the
+      // programmer likes.
+      corp->set_architecture_name("");
 
       out_abi_path =
        abigail::tests::get_build_dir() + "/tests/" + s->out_abi_path;