Better support for inline related diffs

* include/abg-comparison.h
	(diff_category::HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY): New
	enumerator.
	(diff_category::EVERYTHING_CATEGORY): Adjust.
	* include/abg-ir.h (elf_symbol::get_aliases_id_string)
	(elf_symbol::does_alias, elf_symbols_alias)
	(compute_aliases_for_elf_symbol): Declare new functions ...
	* src/abg-ir.cc (elf_symbol::get_aliases_id_string)
	(elf_symbol::does_alias, elf_symbols_alias)
	(compute_aliases_for_elf_symbol): ... and define them.
	(function_decl::operator==): Take in account elf symbol aliases.
	* src/abg-comp-filter.cc (function_name_changed_but_not_symbol):
	Define new static functions.
	(harmless_filter::visit): Categorize function name changes that
n	doesn't impact underlying elf symbols (or the fact that two
	symbols were aliases and are not anymore) as harmless.
	* src/abg-comparison.cc (function_decl_diff::report): Properly
	report function name changes, or symbol aliases changes for that
	matter.  Also report inline-ness declaration changes.
	* src/abg-dwarf-reader.cc (die_is_declared_inline): New static
	function.
	(build_function_decl): Use the above.
	* tools/bidiff.cc (set_diff_context_from_opts): Add
	abigail::comparison::HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY into the
	harmless change camp.
	* tests/data/test-diff-dwarf/test14-inline-report.txt: New test
	input.
	* tests/data/test-diff-dwarf/test14-inline-v0.o: Likewise.
	* tests/data/test-diff-dwarf/test14-inline-v1.o: Likewise.
	* tests/data/test-diff-dwarf/test14-inline-v0.cc: Source code for
	test input.
	* tests/data/test-diff-dwarf/test14-inline-v1.cc: Source code for
	test input.
	* tests/test-diff-dwarf.cc: Run this test harness over the new
	input above.
	* tests/data/test-diff-filter/test20-inline-report-0.txt: Likewise.
	* tests/data/test-diff-filter/test20-inline-report-1.txt:
	Likewise.
	* tests/data/test-diff-filter/test20-inline-v0.o: New test input.
	* tests/data/test-diff-filter/test20-inline-v1.o: New test input.
	* tests/data/test-diff-filter/test20-inline-v0.cc: Source code for
	test input.
	* tests/data/test-diff-filter/test20-inline-v1.cc: Likewise.
	* tests/test-diff-filter.cc: Run this test harness over the new
	input above.

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

include/abg-comparison.h

@@ -266,6 +266,10 @@ enum diff_category
   /// of enumerator to an enum type.
   HARMLESS_ENUM_CHANGE_CATEGORY = 1 << 6,
 
+  /// This means that a diff node in the sub-tree carries an a symbol
+  /// alias change that is harmless.
+  HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY = 1 << 7,
+
   /// This means the diff node (or at least one of its descendant
   /// nodes) carries a change that modifies the size of a type or an
   /// offset of a type member.  Removal or changes of enumerators in a
@@ -289,6 +293,7 @@ enum diff_category
   | NON_VIRT_MEM_FUN_CHANGE_CATEGORY
   | STATIC_DATA_MEMBER_CHANGE_CATEGORY
   | HARMLESS_ENUM_CHANGE_CATEGORY
+  | HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY
   | SIZE_OR_OFFSET_CHANGE_CATEGORY
   | VIRTUAL_MEMBER_CHANGE_CATEGORY
 };

include/abg-ir.h

@@ -407,6 +407,9 @@ public:
   const string&
   get_id_string() const;
 
+  string
+  get_aliases_id_string(const string_elf_symbols_map_type& symtab) const;
+
   static bool
   get_name_and_version_from_id(const string& id,
 			       string& name,
@@ -417,6 +420,9 @@ public:
 
   bool
   operator==(const elf_symbol&) const;
+
+  bool
+  does_alias(const elf_symbol&) const;
 }; // end class elf_symbol.
 
 std::ostream&
@@ -434,6 +440,14 @@ string_to_elf_symbol_binding(const string&, elf_symbol::binding&);
 bool
 operator==(const elf_symbol_sptr lhs, const elf_symbol_sptr rhs);
 
+bool
+elf_symbols_alias(const elf_symbol& s1, const elf_symbol& s2);
+
+void
+compute_aliases_for_elf_symbol(const elf_symbol& symbol,
+			       const string_elf_symbols_map_type& symtab,
+			       vector<elf_symbol*>& alias_set);
+
 /// The abstraction of the version of an ELF symbol.
 class elf_symbol::version
 {

src/abg-comp-filter.cc

@@ -168,6 +168,59 @@ access_changed(decl_base_sptr f, decl_base_sptr s)
   return false;
 }
 
+/// Test if there was a function name change, but there there was no
+/// change in name of the underlying symbol.  IOW, if the name of a
+/// function changed, but the symbol of the new function is equal to
+/// the symbol of the old one, or is equal to an alians of the symbol
+/// of the old function.
+///
+/// @param f the first function to consider.
+///
+/// @param s the second function to consider.
+///
+/// @return true if the test is positive, false otherwise.
+static bool
+function_name_changed_but_not_symbol(const function_decl_sptr f,
+				     const function_decl_sptr s)
+{
+  if (!f || !s)
+    return false;
+  string fn = f->get_pretty_representation(),
+    sn = s->get_pretty_representation();
+
+  if (fn != sn)
+    {
+      elf_symbol_sptr fs = f->get_symbol(), ss = s->get_symbol();
+      if (fs == ss)
+	return true;
+      for (elf_symbol* s = fs->get_next_alias();
+	   s && s != fs->get_main_symbol();
+	   s = s->get_next_alias())
+	if (*s == *ss)
+	  return true;
+    }
+  return false;
+}
+
+/// Test if the current diff tree node carries a function name change,
+/// in which there there was no change in the name of the underlying
+/// symbol.  IOW, if the name of a function changed, but the symbol of
+/// the new function is equal to the symbol of the old one, or is
+/// equal to an alians of the symbol of the old function.
+///
+/// @param diff the diff tree node to consider.
+///
+/// @return true if the test is positive, false otherwise.
+static bool
+function_name_changed_but_not_symbol(const diff* diff)
+{
+  if (const function_decl_diff* d =
+      dynamic_cast<const function_decl_diff*>(diff))
+    return function_name_changed_but_not_symbol(d->first_function_decl(),
+						d->second_function_decl());
+  return false;
+}
+
 /// Tests if the offset of a given data member changed.
 ///
 /// @param f the declaration for the first version of the data member to
@@ -624,6 +677,9 @@ harmless_filter::visit(diff* d, bool pre)
 	  && !has_harmful_enum_change(d))
 	category |= HARMLESS_ENUM_CHANGE_CATEGORY;
 
+      if (function_name_changed_but_not_symbol(d))
+	category |= HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY;
+
       if (category)
 	d->add_to_category(category);
     }

src/abg-comparison.cc

@@ -5622,21 +5622,53 @@ function_decl_diff::report(ostream& out, const string& indent) const
 			       second_function_decl(),
 			       context(), out, indent);
 
-  string qn1 = first_function_decl()->get_qualified_name(),
-    qn2 = second_function_decl()->get_qualified_name(),
-    mn1 = first_function_decl()->get_linkage_name(),
-    mn2 = second_function_decl()->get_linkage_name();
+  function_decl_sptr ff = first_function_decl();
+  function_decl_sptr sf = second_function_decl();
 
-  if (qn1 != qn2 && mn1 != mn2)
+  diff_context_sptr ctxt = context();
+  corpus_sptr fc = ctxt->get_first_corpus();
+  corpus_sptr sc = ctxt->get_second_corpus();
+
+  string qn1 = ff->get_qualified_name(), qn2 = sf->get_qualified_name(),
+    linkage_names1, linkage_names2;
+  elf_symbol_sptr s1 = ff->get_symbol(), s2 = sf->get_symbol();
+
+  if (s1)
+    linkage_names1 = s1->get_id_string();
+  if (s2)
+    linkage_names2 = s2->get_id_string();
+
+  // If the symbols for ff and sf have aliases, get all the names of
+  // the aliases;
+  if (fc)
+    linkage_names1 =
+      ff->get_symbol()->get_aliases_id_string(fc->get_fun_symbol_map());
+  if (sc)
+    linkage_names2 =
+      sf->get_symbol()->get_aliases_id_string(sc->get_fun_symbol_map());
+
+  if (qn1 != qn2)
     {
       string frep1 = first_function_decl()->get_pretty_representation(),
 	frep2 = second_function_decl()->get_pretty_representation();
-      out << indent << "'" << frep1
-	  << "' is different from '"
-	  << frep2 << "'\n";
+      out << indent << "'" << frep1 << " {" << linkage_names1<< "}"
+	  << "' now becomes '"
+	  << frep2 << " {" << linkage_names2 << "}" << "'\n";
       return;
     }
 
+  // Now report about inline-ness changes
+  if (ff->is_declared_inline() != sf->is_declared_inline())
+    {
+      out << indent;
+      if (ff->is_declared_inline())
+	out << sf->get_pretty_representation()
+	    << " is not declared inline anymore\n";
+      else
+	out << sf->get_pretty_representation()
+	    << " is now declared inline\n";
+    }
+
   // Report about return type differences.
   if (priv_->return_type_diff_ && priv_->return_type_diff_->to_be_reported())
     {

src/abg-dwarf-reader.cc

@@ -3040,6 +3040,21 @@ die_is_virtual(Dwarf_Die* die)
   return v == VIRTUALITY_PURE_VIRTUAL || v == VIRTUALITY_VIRTUAL;
 }
 
+/// Test if the DIE represents an entity that was declared inlined.
+///
+/// @param die the DIE to test for.
+///
+/// @return true if the DIE represents an entity that was declared
+/// inlined.
+static bool
+die_is_declared_inline(Dwarf_Die* die)
+{
+  size_t inline_value = 0;
+  if (!die_unsigned_constant_attribute(die, DW_AT_inline, inline_value))
+    return false;
+  return inline_value == DW_INL_declared_inlined;
+}
+
 /// Get the value of a given DIE attribute, knowing that it must be a
 /// location expression.
 ///
@@ -5681,8 +5696,7 @@ build_function_decl(read_context&	ctxt,
   location floc;
   die_loc_and_name(ctxt, die, floc, fname, flinkage_name);
 
-  size_t is_inline = false;
-  die_unsigned_constant_attribute(die, DW_AT_inline, is_inline);
+  size_t is_inline = die_is_declared_inline(die);
 
   decl_base_sptr return_type_decl;
   Dwarf_Die ret_type_die;

src/abg-ir.cc

@@ -600,6 +600,27 @@ elf_symbol::get_id_string() const
   return priv_->id_string_;
 }
 
+/// Return a comma separated list of the id of the current symbol as
+/// well as the id string of its aliases.
+///
+/// @return the string.
+string
+elf_symbol::get_aliases_id_string(const string_elf_symbols_map_type& syms) const
+{
+  string result = get_id_string();
+  vector<elf_symbol*> aliases;
+  compute_aliases_for_elf_symbol(*this, syms, aliases);
+
+  for (vector<elf_symbol*>::const_iterator i = aliases.begin();
+       i != aliases.end();
+       ++i)
+    {
+      result += ", ";
+      result += (*i)->get_id_string();
+    }
+  return result;
+}
+
 /// Given the ID of a symbol, get the name and the version of said
 /// symbol.
 ///
@@ -655,6 +676,27 @@ elf_symbol::operator==(const elf_symbol& other) const
 	  && get_version() == other.get_version());
 }
 
+/// Test if the current symbol aliases another one.
+///
+/// @param o the other symbol to test against.
+///
+/// @return true iff the current symbol aliases @p o.
+bool
+elf_symbol::does_alias(const elf_symbol& o) const
+{
+  if (*this == o)
+    return true;
+
+  for (const elf_symbol* a = get_next_alias();
+       a && a != get_main_symbol();
+       a = a->get_next_alias())
+    {
+      if (o == *a)
+	return true;
+    }
+  return false;
+}
+
 bool
 operator==(const elf_symbol_sptr lhs, const elf_symbol_sptr rhs)
 {
@@ -667,6 +709,76 @@ operator==(const elf_symbol_sptr lhs, const elf_symbol_sptr rhs)
   return *lhs == *rhs;
 }
 
+/// Test if two symbols alias.
+///
+/// @param s1 the first symbol to consider.
+///
+/// @param s2 the second symbol to consider.
+///
+/// @return true if @p s1 aliases @p s2.
+bool
+elf_symbols_alias(const elf_symbol& s1, const elf_symbol& s2)
+{return s1.does_alias(s2) || s2.does_alias(s1);}
+
+void
+compute_aliases_for_elf_symbol(const elf_symbol& sym,
+			       const string_elf_symbols_map_type& symtab,
+			       vector<elf_symbol*>& aliases)
+{
+
+  if (elf_symbol* a = sym.get_next_alias())
+    for (; a != sym.get_main_symbol(); a = a->get_next_alias())
+      aliases.push_back(a);
+  else
+    for (string_elf_symbols_map_type::const_iterator i = symtab.begin();
+	 i != symtab.end();
+	 ++i)
+      for (elf_symbols::const_iterator j = i->second.begin();
+	   j != i->second.end();
+	   ++j)
+	{
+	  if (**j == sym)
+	    for (elf_symbol* s = (*j)->get_next_alias();
+		 s && s != (*j)->get_main_symbol();
+		 s = s->get_next_alias())
+	      aliases.push_back(s);
+	  else
+	    for (const elf_symbol* s = (*j)->get_next_alias();
+		 s && s != (*j)->get_main_symbol();
+		 s = s->get_next_alias())
+	      if (*s == sym)
+		aliases.push_back(j->get());
+	}
+}
+
+/// Test if two symbols alias.
+///
+/// @param s1 the first symbol to consider.
+///
+/// @param s2 the second symbol to consider.
+///
+/// @return true if @p s1 aliases @p s2.
+bool
+elf_symbols_alias(const elf_symbol* s1, const elf_symbol* s2)
+{
+  if (!!s1 != !!s2)
+    return false;
+  if (s1 == s2)
+    return true;
+  return elf_symbols_alias(*s1, *s2);
+}
+
+/// Test if two symbols alias.
+///
+/// @param s1 the first symbol to consider.
+///
+/// @param s2 the second symbol to consider.
+///
+/// @return true if @p s1 aliases @p s2.
+bool
+elf_symbols_alias(const elf_symbol_sptr s1, const elf_symbol_sptr s2)
+{return elf_symbols_alias(s1.get(), s2.get());}
+
 /// Serialize an instance of @ref symbol_type and stream it to a given
 /// output stream.
 ///
@@ -4969,7 +5081,10 @@ function_decl::operator==(const decl_base& other) const
 	return false;
 
       if (s0 && s0 != s1)
-	return false;
+	{
+	  if (!elf_symbols_alias(s0, s1))
+	    return false;
+	}
 
       if (s0)
 	{

tests/Makefile.am

@@ -201,6 +201,11 @@ data/test-diff-dwarf/test13-v0.o		\
 data/test-diff-dwarf/test13-v1.cc		\
 data/test-diff-dwarf/test13-v1.o		\
 data/test-diff-dwarf/test13-report.txt		\
+data/test-diff-dwarf/test14-inline-v0.cc	\
+data/test-diff-dwarf/test14-inline-v0.o		\
+data/test-diff-dwarf/test14-inline-v1.cc	\
+data/test-diff-dwarf/test14-inline-v1.o		\
+data/test-diff-dwarf/test14-inline-report.txt	\
 data/test-diff-dwarf/test15-enum-v0.cc		\
 data/test-diff-dwarf/test15-enum-v0.o		\
 data/test-diff-dwarf/test15-enum-v1.cc		\
@@ -331,6 +336,12 @@ data/test-diff-filter/test19-v0.o		\
 data/test-diff-filter/test19-v1.o		\
 data/test-diff-filter/test19-report-0.txt	\
 data/test-diff-filter/test19-report-1.txt	\
+data/test-diff-filter/test20-inline-v0.cc	\
+data/test-diff-filter/test20-inline-v1.cc	\
+data/test-diff-filter/test20-inline-v0.o	\
+data/test-diff-filter/test20-inline-v1.o	\
+data/test-diff-filter/test20-inline-report-0.txt \
+data/test-diff-filter/test20-inline-report-1.txt \
 \
 data/test-lookup-syms/test0.cc			\
 data/test-lookup-syms/test0.o			\

tests/data/test-diff-dwarf/test14-inline-report.txt

@@ -0,0 +1,4 @@
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+

tests/data/test-diff-dwarf/test14-inline-v0.cc

@@ -0,0 +1,15 @@
+// Compile this *without* optimization so that
+// foo is not inlined:
+//     g++ -g -O0 -c test14-inline-v0.cc
+
+int
+foo()
+{
+  int i = 0;
+  ++i;
+  return i;
+}
+
+int
+bar()
+{return foo();}

tests/data/test-diff-dwarf/test14-inline-v1.cc

@@ -0,0 +1,15 @@
+// Compile this *with* optimization so that
+// foo is inlined:
+//     g++ -g -O2 -c test14-inline-v0.cc
+
+int
+foo()
+{
+  int i = 0;
+  ++i;
+  return i;
+}
+
+int
+bar()
+{return foo();}

tests/data/test-diff-filter/test20-inline-report-0.txt

@@ -0,0 +1,4 @@
+Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+

tests/data/test-diff-filter/test20-inline-report-1.txt

@@ -0,0 +1,8 @@
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+  [C]'function int bar()' has some indirect sub-type changes:
+    'function int bar() {_ZN1S3fooEv, _Z3barv}' now becomes 'method int S::foo() {_ZN1S3fooEv}'
+

tests/data/test-diff-filter/test20-inline-v0.cc

@@ -0,0 +1,17 @@
+struct S
+{
+  int
+  foo()
+  {
+    int i = 0;
+    ++i;
+    return i;
+  }
+};
+
+int
+bar()
+{
+  S s;
+  return s.foo();
+}

tests/data/test-diff-filter/test20-inline-v1.cc

@@ -0,0 +1,19 @@
+struct S
+{
+  int
+  foo();
+};
+
+int
+S::foo()
+{
+  int i = 0;
+  return i;
+}
+
+int
+bar()
+{
+  S s;
+  return s.foo();
+}

tests/test-diff-dwarf.cc

@@ -146,6 +146,12 @@ InOutSpec in_out_specs[] =
     "data/test-diff-dwarf/test13-report.txt",
     "output/test-diff-dwarf/test13-report.txt"
   },
+  {
+    "data/test-diff-dwarf/test14-inline-v0.o",
+    "data/test-diff-dwarf/test14-inline-v1.o",
+    "data/test-diff-dwarf/test14-inline-report.txt",
+    "output/test-diff-dwarf/test14-inline-report.txt"
+  },
   {
     "data/test-diff-dwarf/test15-enum-v0.o",
     "data/test-diff-dwarf/test15-enum-v1.o",

tests/test-diff-filter.cc

@@ -229,6 +229,20 @@ InOutSpec in_out_specs[] =
     "data/test-diff-filter/test19-enum-report-1.txt",
     "output/test-diff-filter/test19-enum-report-1.txt",
   },
+  {
+    "data/test-diff-filter/test20-inline-v0.o",
+    "data/test-diff-filter/test20-inline-v1.o",
+    "",
+    "data/test-diff-filter/test20-inline-report-0.txt",
+    "output/test-diff-filter/test20-inline-report-0.txt",
+  },
+  {
+    "data/test-diff-filter/test20-inline-v0.o",
+    "data/test-diff-filter/test20-inline-v1.o",
+    "--harmless",
+    "data/test-diff-filter/test20-inline-report-1.txt",
+    "output/test-diff-filter/test20-inline-report-1.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };

tools/bidiff.cc

@@ -366,7 +366,8 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
 	 | abigail::comparison::HARMLESS_DECL_NAME_CHANGE_CATEGORY
 	 | abigail::comparison::NON_VIRT_MEM_FUN_CHANGE_CATEGORY
 	 | abigail::comparison::STATIC_DATA_MEMBER_CHANGE_CATEGORY
-	 | abigail::comparison::HARMLESS_ENUM_CHANGE_CATEGORY);
+	 | abigail::comparison::HARMLESS_ENUM_CHANGE_CATEGORY
+	 | abigail::comparison::HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY);
   if (!opts.show_harmful_changes)
     ctxt->switch_categories_off
       (abigail::comparison::SIZE_OR_OFFSET_CHANGE_CATEGORY