From: Dodji Seketeli Date: Wed, 7 Jan 2015 12:45:53 +0000 (+0100) Subject: Detect and report changes in ELF architecture X-Git-Tag: 1.0.rc0~373 X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=929db0a880cc637ada2928fde9326f1d18011337;p=libabigail.git 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 --- diff --git a/configure.ac b/configure.ac index 0c9d75cf..80ca8c57 100644 --- 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 5c6cd3a0..20e8ab97 100644 --- 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 0fb9f763..cd0c49d7 100644 --- 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 12b3b09a..36480740 100644 --- 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(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 e2ee9c25..f76daf2d 100644 --- a/src/abg-corpus.cc +++ b/src/abg-corpus.cc @@ -104,6 +104,7 @@ struct corpus::priv string path; vector needed; string soname; + string architecture_name; translation_units members; vector fns; vector 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 383a7b8b..daa53ee8 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -1465,6 +1466,7 @@ class read_context string_elf_symbols_map_sptr undefined_var_syms_; vector 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 246a68e5..879631da 100644 --- 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(path_str.get())); + xml::xml_char_sptr architecture_str = + XML_READER_GET_ATTRIBUTE(reader, "architecture"); + if (architecture_str) + corp.set_architecture_name(reinterpret_cast(architecture_str.get())); + xml::xml_char_sptr soname_str = XML_READER_GET_ATTRIBUTE(reader, "soname"); if (soname_str) corp.set_soname(reinterpret_cast(soname_str.get())); diff --git a/src/abg-writer.cc b/src/abg-writer.cc index 3f528d7d..d832de88 100644 --- 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 7708fc9e..7637b911 100644 --- 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 index 00000000..af815cf2 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 index 00000000..01c48453 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 index 00000000..e897d20b --- /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 index 00000000..052e129d --- /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 82cf85ea..7486b2c6 100644 --- 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 e8c12ea1..f7d84f55 100644 --- 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;