Consider default symbol versions when computing added/removed fns/vars

When computing the set of added function or variable symbols, if a
symbol S with no version symbol was present in a given corpus and that
symbol gained a *DEFAULT* version V in the second corpus, we should
not consider that a new symbol S was added (and that the former S was
removed) because:

  1/ S was already present in the first corpus
  2/ applications linked to the first corpus and that were using S
  (with no version) there, will automatically use the S with version V
  in the second corpus, without needing any re-linking; the
  power of symbol versioning!

Rather, it's just that S gained a default symbol version.

This patch implements that.

	* include/abg-corpus.h (corpus::{lookup_function_symbol,
	lookup_variable_symbol}): Take a elf_symbol::version object,
	rather than a string representing the version.  Add an overload
	that takes an elf_symbol.
	* src/abg-corpus.cc (find_symbol_by_version): New static function.
	(corpus::{lookup_function_symbol, lookup_variable_symbol}): Take a
	elf_symbol::version object, rather than a string representing the
	version.  Add an overload that takes an elf_symbol.  If the looked
	up symbol has no version and if the corpus contains a symbol with
	the same name and with a default version, then return that latter
	symbol if the corpus doesn't contain a symbol with the same name
	and empty version.
	* src/abg-comparison.cc
	(class_diff::ensure_lookup_tables_populated): Adjust.
	(corpus_diff::priv::ensure_lookup_tables_populated): Before
	deciding that a symbol has been added, if the symbol has a default
	version, make sure no symbol with the same name and without
	version was present in the former corpus.  Similarly, before
	deciding that a symbol has been removed, if the symbol has no
	version, make sure the latter corpus has no symbol with the same
	name and with a default version.
	* tests/data/test-diff-dwarf/test12-report.txt: Adjust.  The
	function should not be considered as added, because its symbol
	(and version) was already present in the former DSO.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This commit is contained in:
Dodji Seketeli 2015-07-24 14:04:11 +02:00
parent bf6b1924c4
commit e5cf9d1f60
4 changed files with 161 additions and 53 deletions

View File

@ -178,14 +178,20 @@ public:
const elf_symbol_sptr
lookup_function_symbol(const string& symbol_name,
const string& symbol_version) const;
const elf_symbol::version& version) const;
const elf_symbol_sptr
lookup_function_symbol(const elf_symbol& symbol) const;
const elf_symbol_sptr
lookup_variable_symbol(const string& n) const;
const elf_symbol_sptr
lookup_variable_symbol(const string& symbol_name,
const string& symbol_version) const;
const elf_symbol::version& version) const;
const elf_symbol_sptr
lookup_variable_symbol(const elf_symbol& symbol) const;
const functions&
get_functions() const;

View File

@ -8283,8 +8283,7 @@ class_diff::ensure_lookup_tables_populated(void) const
// symbols.
if (!i->second->get_symbol()
|| (i->second->get_symbol()
&& s->lookup_function_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str())))
&& s->lookup_function_symbol(*i->second->get_symbol())))
to_delete.push_back(i->first);
@ -8301,8 +8300,7 @@ class_diff::ensure_lookup_tables_populated(void) const
i != inserted_member_fns().end();
++i)
if (!i->second->get_symbol()
|| f->lookup_function_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str()))
|| f->lookup_function_symbol(*i->second->get_symbol()))
to_delete.push_back(i->first);
for (vector<string>::const_iterator i = to_delete.begin();
@ -12779,8 +12777,7 @@ corpus_diff::priv::ensure_lookup_tables_populated()
for (string_function_ptr_map::const_iterator i = deleted_fns_.begin();
i != deleted_fns_.end();
++i)
if (second_->lookup_function_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str()))
if (second_->lookup_function_symbol(*i->second->get_symbol()))
to_delete.push_back(i->first);
for (vector<string>::const_iterator i = to_delete.begin();
@ -12794,9 +12791,23 @@ corpus_diff::priv::ensure_lookup_tables_populated()
for (string_function_ptr_map::const_iterator i = added_fns_.begin();
i != added_fns_.end();
++i)
if (first_->lookup_function_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str()))
to_delete.push_back(i->first);
{
if (first_->lookup_function_symbol(*i->second->get_symbol()))
to_delete.push_back(i->first);
else if (! i->second->get_symbol()->get_version().is_empty()
&& i->second->get_symbol()->get_version().is_default())
// We are looking for a symbol that has a default version,
// and which seems to be newly added. Let's see if the same
// symbol with *no* version was already present in the
// former corpus. If yes, then the symbol shouldn't be
// considered as 'added'.
{
elf_symbol::version empty_version;
if (first_->lookup_function_symbol(i->second->get_symbol()->get_name(),
empty_version))
to_delete.push_back(i->first);
}
}
for (vector<string>::const_iterator i = to_delete.begin();
i != to_delete.end();
@ -12870,8 +12881,7 @@ corpus_diff::priv::ensure_lookup_tables_populated()
for (string_var_ptr_map::const_iterator i = deleted_vars_.begin();
i != deleted_vars_.end();
++i)
if (second_->lookup_variable_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str()))
if (second_->lookup_variable_symbol(*i->second->get_symbol()))
to_delete.push_back(i->first);
for (vector<string>::const_iterator i = to_delete.begin();
@ -12885,9 +12895,21 @@ corpus_diff::priv::ensure_lookup_tables_populated()
for (string_var_ptr_map::const_iterator i = added_vars_.begin();
i != added_vars_.end();
++i)
if (first_->lookup_variable_symbol(i->second->get_symbol()->get_name(),
i->second->get_symbol()->get_version().str()))
if (first_->lookup_variable_symbol(*i->second->get_symbol()))
to_delete.push_back(i->first);
else if (! i->second->get_symbol()->get_version().is_empty()
&& i->second->get_symbol()->get_version().is_default())
// We are looking for a symbol that has a default version,
// and which seems to be newly added. Let's see if the same
// symbol with *no* version was already present in the
// former corpus. If yes, then the symbol shouldn't be
// considered as 'added'.
{
elf_symbol::version empty_version;
if (first_->lookup_variable_symbol(i->second->get_symbol()->get_name(),
empty_version))
to_delete.push_back(i->first);
}
for (vector<string>::const_iterator i = to_delete.begin();
i != to_delete.end();
@ -12908,8 +12930,7 @@ corpus_diff::priv::ensure_lookup_tables_populated()
assert(i < first_->get_unreferenced_function_symbols().size());
elf_symbol_sptr deleted_sym =
first_->get_unreferenced_function_symbols()[i];
if (!second_->lookup_function_symbol(deleted_sym->get_name(),
deleted_sym->get_version()))
if (!second_->lookup_function_symbol(*deleted_sym))
deleted_unrefed_fn_syms_[deleted_sym->get_id_string()] = deleted_sym;
}
@ -12929,10 +12950,27 @@ corpus_diff::priv::ensure_lookup_tables_populated()
if ((deleted_unrefed_fn_syms_.find(added_sym->get_id_string())
== deleted_unrefed_fn_syms_.end()))
{
if (!first_->lookup_function_symbol(added_sym->get_name(),
added_sym->get_version()))
added_unrefed_fn_syms_[added_sym->get_id_string()] =
added_sym;
if (!first_->lookup_function_symbol(*added_sym))
{
bool do_add = true;
if (! added_sym->get_version().is_empty()
&& added_sym->get_version().is_default())
{
// So added_seem has a default version. If
// the former corpus had a symbol with the
// same name as added_sym but with *no*
// version, then added_sym shouldn't be
// considered as a newly added symbol.
elf_symbol::version empty_version;
if(first_->lookup_function_symbol(added_sym->get_name(),
empty_version))
do_add = false;
}
if (do_add)
added_unrefed_fn_syms_[added_sym->get_id_string()] =
added_sym;
}
}
else
deleted_unrefed_fn_syms_.erase(added_sym->get_id_string());
@ -12953,8 +12991,7 @@ corpus_diff::priv::ensure_lookup_tables_populated()
assert(i < first_->get_unreferenced_variable_symbols().size());
elf_symbol_sptr deleted_sym =
first_->get_unreferenced_variable_symbols()[i];
if (!second_->lookup_variable_symbol(deleted_sym->get_name(),
deleted_sym->get_version()))
if (!second_->lookup_variable_symbol(*deleted_sym))
deleted_unrefed_var_syms_[deleted_sym->get_id_string()] = deleted_sym;
}
@ -12974,10 +13011,27 @@ corpus_diff::priv::ensure_lookup_tables_populated()
if (deleted_unrefed_var_syms_.find(added_sym->get_id_string())
== deleted_unrefed_var_syms_.end())
{
if (!first_->lookup_variable_symbol(added_sym->get_name(),
added_sym->get_version()))
added_unrefed_var_syms_[added_sym->get_id_string()] =
added_sym;
if (!first_->lookup_variable_symbol(*added_sym))
{
bool do_add = true;
if (! added_sym->get_version().is_empty()
&& added_sym->get_version().is_default())
{
// So added_seem has a default version. If
// the former corpus had a symbol with the
// same name as added_sym but with *no*
// version, then added_sym shouldn't be
// considered as a newly added symbol.
elf_symbol::version empty_version;
if(first_->lookup_variable_symbol(added_sym->get_name(),
empty_version))
do_add = false;
}
if (do_add)
added_unrefed_var_syms_[added_sym->get_id_string()] =
added_sym;
}
}
else
deleted_unrefed_var_syms_.erase(added_sym->get_id_string());

View File

@ -1485,18 +1485,62 @@ corpus::lookup_function_symbol(const string& n) const
return it->second[0];
}
/// Look into a set of symbols and look for a symbol that has a given
/// version.
///
/// This is a sub-routine for corpus::lookup_function_symbol() and
/// corpus::lookup_variable_symbol().
///
/// @param version the version of the symbol to look for.
///
/// @param symbols the set of symbols to consider.
///
/// @return the symbol found, or nil if none was found.
static const elf_symbol_sptr
find_symbol_by_version(const elf_symbol::version& version,
const vector<elf_symbol_sptr>& symbols)
{
if (version.is_empty())
{
// We are looing for a symbol with no version.
// So first look for possible aliases with no version
for (elf_symbols::const_iterator s = symbols.begin();
s != symbols.end();
++s)
if ((*s)->get_version().is_empty())
return *s;
// Or, look for a version that is a default one!
for (elf_symbols::const_iterator s = symbols.begin();
s != symbols.end();
++s)
if ((*s)->get_version().is_default())
return *s;
}
else
// We are looking for a symbol with a particular defined version.
for (elf_symbols::const_iterator s = symbols.begin();
s != symbols.end();
++s)
if ((*s)->get_version().str() == version.str())
return *s;
return elf_symbol_sptr();
}
/// Look in the function symbols map for a symbol with a given name.
///
/// @param symbol_name the name of the symbol to look for.
///
/// @param symbol_version the version of the symbol to look for.
/// @param version the version of the symbol to look for.
///
/// return the symbol with the name @p symbol_name and with the name
/// @p symbol_version, or nil if no symbol has been found with that
/// name and version.
/// return the symbol with name @p symbol_name and with version @p
/// version, or nil if no symbol has been found with that name and
/// version.
const elf_symbol_sptr
corpus::lookup_function_symbol(const string& symbol_name,
const string& symbol_version) const
const elf_symbol::version& version) const
{
if (!get_fun_symbol_map_sptr())
return elf_symbol_sptr();
@ -1506,14 +1550,19 @@ corpus::lookup_function_symbol(const string& symbol_name,
if ( it == get_fun_symbol_map_sptr()->end())
return elf_symbol_sptr();
for (elf_symbols::const_iterator s = it->second.begin();
s != it->second.end();
++s)
if ((*s)->get_version().str() == symbol_version)
return *s;
return elf_symbol_sptr();
return find_symbol_by_version(version, it->second);
}
/// Look in the function symbols map for a symbol with the same name
/// and version as a given symbol.
///
/// @param symbol the symbol to look for.
///
/// return the symbol with the same name and version as @p symbol.
const elf_symbol_sptr
corpus::lookup_function_symbol(const elf_symbol& symbol) const
{return lookup_function_symbol(symbol.get_name(), symbol.get_version());}
/// Look in the variable symbols map for a symbol with a given name.
///
/// @param n the name of the symbol to look for.
@ -1538,10 +1587,11 @@ corpus::lookup_variable_symbol(const string& n) const
///
/// @param symbol_version the version of the symbol to look for.
///
/// return the first symbol with the name @p n.
/// return the first symbol with the name @p symbol_name and with
/// version @p version.
const elf_symbol_sptr
corpus::lookup_variable_symbol(const string& symbol_name,
const string& symbol_version) const
const elf_symbol::version& version) const
{
if (!get_var_symbol_map_sptr())
return elf_symbol_sptr();
@ -1551,14 +1601,19 @@ corpus::lookup_variable_symbol(const string& symbol_name,
if ( it == get_var_symbol_map_sptr()->end())
return elf_symbol_sptr();
for (elf_symbols::const_iterator s = it->second.begin();
s != it->second.end();
++s)
if ((*s)->get_version().str() == symbol_version)
return *s;
return elf_symbol_sptr();
return find_symbol_by_version(version, it->second);
}
/// Look in the variable symbols map for a symbol with the same name
/// and version as a given symbol.
///
/// @param symbol the symbol to look for.
///
/// return the symbol with the same name and version as @p symbol.
const elf_symbol_sptr
corpus::lookup_variable_symbol(const elf_symbol& symbol) const
{return lookup_variable_symbol(symbol.get_name(), symbol.get_version());}
/// Return the functions public decl table of the current corpus.
///
/// The function public decl tables is a vector of all the functions

View File

@ -1,7 +0,0 @@
Functions changes summary: 0 Removed, 0 Changed, 1 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
1 Added function:
'function int _foo3(int, int)'