Match symbols by object path and name
In the case of corpus groups, we should consider symbols in different
modules to be independent (and not duplicates if they happen to have
the same name).
For the moment, we won't model corpus groups directly, but we can make
sure to scope symbols name by corpus path and mention the path in
symbol diffs (if it is not "vmlinux").
PiperOrigin-RevId: 459449265
Change-Id: I1a5fd1571c8bfb4262e7c23d14b2c7c708562040
diff --git a/abigail_reader.cc b/abigail_reader.cc
index bc4bd3b..ceed261 100644
--- a/abigail_reader.cc
+++ b/abigail_reader.cc
@@ -766,16 +766,25 @@
Id Abigail::ProcessRoot(xmlNodePtr root) {
Typing typing(graph_, verbose_);
- std::map<std::string, Id> symbols;
+ std::map<SymbolKey, Id> symbols;
+ auto merge = [&](const std::string& path,
+ const std::map<std::string, Id>& corpus_symbols) {
+ for (const auto& [name, id] : corpus_symbols) {
+ const SymbolKey key{path, name};
+ Check(symbols.insert({key, id}).second)
+ << "found duplicate symbol '" << key << "'";
+ }
+ };
const auto name = GetElementName(root);
if (name == "abi-corpus-group") {
for (auto child = xmlFirstElementChild(root); child;
child = xmlNextElementSibling(child)) {
CheckElementName("abi-corpus", child);
- symbols.merge(Corpus(graph_, verbose_, typing).ProcessCorpus(child));
+ const auto path = ReadAttribute(child, "path", std::string());
+ merge(path, Corpus(graph_, verbose_, typing).ProcessCorpus(child));
}
} else if (name == "abi-corpus") {
- symbols.merge(Corpus(graph_, verbose_, typing).ProcessCorpus(root));
+ merge(std::string(), Corpus(graph_, verbose_, typing).ProcessCorpus(root));
} else {
Die() << "unrecognised root element '" << name << "'";
}
diff --git a/btf_reader.cc b/btf_reader.cc
index f7aceeb..9c7b4fb 100644
--- a/btf_reader.cc
+++ b/btf_reader.cc
@@ -394,7 +394,8 @@
std::nullopt,
GetId(t->type),
std::nullopt));
- bool inserted = btf_symbols_.insert({name, GetIdRaw(btf_index)}).second;
+ bool inserted = btf_symbols_.insert(
+ {{std::string(), name}, GetIdRaw(btf_index)}).second;
Check(inserted) << "duplicate symbol " << name;
break;
}
@@ -429,7 +430,8 @@
std::nullopt,
GetId(t->type),
std::nullopt));
- bool inserted = btf_symbols_.insert({name, GetIdRaw(btf_index)}).second;
+ bool inserted = btf_symbols_.insert(
+ {{std::string(), name}, GetIdRaw(btf_index)}).second;
Check(inserted) << "duplicate symbol " << name;
break;
}
diff --git a/btf_reader.h b/btf_reader.h
index 0cf12c5..5b6ff7b 100644
--- a/btf_reader.h
+++ b/btf_reader.h
@@ -59,7 +59,7 @@
std::optional<Id> void_;
std::optional<Id> variadic_;
std::unordered_map<uint32_t, Id> btf_type_ids_;
- std::map<std::string, Id> btf_symbols_;
+ std::map<SymbolKey, Id> btf_symbols_;
Id GetVoid();
Id GetVariadic();
diff --git a/elf_reader.cc b/elf_reader.cc
index 1d93503..7cf6b0d 100644
--- a/elf_reader.cc
+++ b/elf_reader.cc
@@ -31,7 +31,7 @@
Id Read(Graph& graph, const std::string& path, bool verbose) {
if (verbose)
std::cerr << "Parsing ELF: " << path << "\n";
- std::map<std::string, Id> symbols;
+ std::map<SymbolKey, Id> symbols;
return graph.Add(Make<Symbols>(symbols));
}
diff --git a/stg.cc b/stg.cc
index 55cee40..e0e26fc 100644
--- a/stg.cc
+++ b/stg.cc
@@ -823,9 +823,9 @@
result.diff_.holds_changes = true;
// Group diffs into removed, added and changed symbols for readability.
- std::vector<Id> removed;
- std::vector<Id> added;
- std::vector<std::pair<Id, Id>> in_both;
+ std::vector<std::pair<std::string, Id>> removed;
+ std::vector<std::pair<std::string, Id>> added;
+ std::vector<std::tuple<std::string, Id, Id>> in_both;
const auto& symbols1 = symbols;
const auto& symbols2 = o.symbols;
@@ -836,26 +836,30 @@
while (it1 != end1 || it2 != end2) {
if (it2 == end2 || (it1 != end1 && it1->first < it2->first)) {
// removed
- removed.push_back(it1->second);
+ removed.push_back({it1->first.path, it1->second});
++it1;
} else if (it1 == end1 || (it2 != end2 && it1->first > it2->first)) {
// added
- added.push_back(it2->second);
+ added.push_back({it2->first.path, it2->second});
++it2;
} else {
// in both
- in_both.push_back({it1->second, it2->second});
+ in_both.push_back({it1->first.path, it1->second, it2->second});
++it1;
++it2;
}
}
- for (const auto symbol1 : removed)
- result.AddEdgeDiff("", Removed(state, symbol1));
- for (const auto symbol2 : added)
- result.AddEdgeDiff("", Added(state, symbol2));
- for (const auto& [symbol1, symbol2] : in_both)
- result.MaybeAddEdgeDiff("", Compare(state, symbol1, symbol2));
+ auto quote = [](const std::string& path) {
+ return path.empty() || path == "vmlinux"
+ ? std::string() : '\'' + path + '\'';
+ };
+ for (const auto& [path, symbol1] : removed)
+ result.AddEdgeDiff(quote(path), Removed(state, symbol1));
+ for (const auto& [path, symbol2] : added)
+ result.AddEdgeDiff(quote(path), Added(state, symbol2));
+ for (const auto& [path, symbol1, symbol2] : in_both)
+ result.MaybeAddEdgeDiff(quote(path), Compare(state, symbol1, symbol2));
return result;
}
@@ -1047,6 +1051,13 @@
return os;
}
+std::ostream& operator<<(std::ostream& os, const SymbolKey& key) {
+ if (!key.path.empty())
+ os << key << ':';
+ os << key.name;
+ return os;
+}
+
std::ostream& operator<<(std::ostream& os, Integer::Encoding encoding) {
auto ix = static_cast<size_t>(encoding);
return os << (ix < kIntEncoding.size() ? kIntEncoding[ix] : "(unknown)");
diff --git a/stg.h b/stg.h
index c6c57aa..80dccc7 100644
--- a/stg.h
+++ b/stg.h
@@ -504,13 +504,45 @@
std::ostream& operator<<(std::ostream& os, ElfSymbol::Binding);
std::ostream& operator<<(std::ostream& os, ElfSymbol::Visibility);
+struct SymbolKey {
+ const std::string path;
+ const std::string name;
+ // auto operator<=>(const SymbolKey&) const = default
+ int compare(const SymbolKey& other) const {
+ int result = path.compare(other.path);
+ if (result)
+ return result;
+ return name.compare(other.name);
+ }
+ bool operator==(const SymbolKey& other) const {
+ return compare(other) == 0;
+ }
+ bool operator!=(const SymbolKey& other) const {
+ return compare(other) != 0;
+ }
+ bool operator<(const SymbolKey& other) const {
+ return compare(other) < 0;
+ }
+ bool operator>(const SymbolKey& other) const {
+ return compare(other) > 0;
+ }
+ bool operator<=(const SymbolKey& other) const {
+ return compare(other) <= 0;
+ }
+ bool operator>=(const SymbolKey& other) const {
+ return compare(other) >= 0;
+ }
+};
+
+std::ostream& operator<<(std::ostream& os, const SymbolKey& key);
+
struct Symbols : Node {
- Symbols(const std::map<std::string, Id>& symbols) : symbols(symbols) {}
+ Symbols(const std::map<SymbolKey, Id>& symbols) : symbols(symbols) {}
std::string GetKindDescription() const final;
Name MakeDescription(const Graph& graph, NameCache& names) const final;
Result Equals(State& state, const Node& other) const final;
- const std::map<std::string, Id> symbols;
+ const std::map<SymbolKey, Id> symbols;
};
std::ostream& operator<<(std::ostream& os, Integer::Encoding encoding);
