Add option to ignore some specific LinkerSetKeys in header-abi-diff

Sometimes developers may want to ignore some incompatible changes in ABI
because it is safe for some reason. An option should be added to
header-abi-diff so it ignores these LinkerSetKeys passed from cli or
config file.

Test: perform abi diff on libz with abi-dump version 33 with the LinkerSetKey _ZTI14internal_stat is specified.
Bug: 243903630
Change-Id: Iccc79d648ae17634a987faac32a2a7cbaa784e5c
This commit is contained in:
Mu-Le Lee
2022-08-26 09:31:54 +00:00
parent 82c4dd2bc7
commit d93d4de71f
10 changed files with 62 additions and 7 deletions

View File

@@ -387,7 +387,7 @@ bool HeaderAbiDiff::DumpDiffElements(
DiffWrapper<T> diff_wrapper(
old_element, new_element, ir_diff_dumper, old_types, new_types,
diff_policy_options_, &type_cache_);
diff_policy_options_, &type_cache_, ignored_linker_set_keys_);
if (!diff_wrapper.DumpDiff(diff_kind)) {
llvm::errs() << "Failed to diff elements\n";
return false;

View File

@@ -36,6 +36,7 @@ class HeaderAbiDiff {
const std::string &old_dump, const std::string &new_dump,
const std::string &compatibility_report,
const std::set<std::string> &ignored_symbols,
const std::set<std::string> &ignored_linker_set_keys,
bool allow_adding_removing_weak_symbols,
const DiffPolicyOptions &diff_policy_options,
bool check_all_apis, repr::TextFormatIR text_format_old,
@@ -44,6 +45,7 @@ class HeaderAbiDiff {
: lib_name_(lib_name), arch_(arch), old_dump_(old_dump),
new_dump_(new_dump), cr_(compatibility_report),
ignored_symbols_(ignored_symbols),
ignored_linker_set_keys_(ignored_linker_set_keys),
diff_policy_options_(diff_policy_options),
allow_adding_removing_weak_symbols_(allow_adding_removing_weak_symbols),
check_all_apis_(check_all_apis),
@@ -150,6 +152,7 @@ class HeaderAbiDiff {
const std::string &new_dump_;
const std::string &cr_;
const std::set<std::string> &ignored_symbols_;
const std::set<std::string> &ignored_linker_set_keys_;
const DiffPolicyOptions &diff_policy_options_;
bool allow_adding_removing_weak_symbols_;
bool check_all_apis_;

View File

@@ -43,9 +43,10 @@ class DiffWrapper : public AbiDiffHelper {
const AbiElementMap<const repr::TypeIR *> &old_types,
const AbiElementMap<const repr::TypeIR *> &new_types,
const repr::DiffPolicyOptions &diff_policy_options,
std::set<std::string> *type_cache)
std::set<std::string> *type_cache,
const std::set<std::string> &ignored_linker_set_keys)
: AbiDiffHelper(old_types, new_types, diff_policy_options, type_cache,
ir_diff_dumper),
ignored_linker_set_keys, ir_diff_dumper),
oldp_(oldp), newp_(newp) {}
bool DumpDiff(repr::IRDiffDumper::DiffKind diff_kind);

View File

@@ -139,6 +139,11 @@ static llvm::cl::opt<std::string> target_version(
llvm::cl::init("current"), llvm::cl::Optional,
llvm::cl::cat(header_checker_category));
static llvm::cl::list<std::string> ignore_linker_set_keys(
"ignore-linker-set-key",
llvm::cl::desc("Ignore a specific type or function in the comparison."),
llvm::cl::ZeroOrMore, llvm::cl::cat(header_checker_category));
static std::set<std::string> LoadIgnoredSymbols(std::string &symbol_list_path) {
std::ifstream symbol_ifstream(symbol_list_path);
std::set<std::string> ignored_symbols;
@@ -161,6 +166,9 @@ static std::string GetConfigFilePath(const std::string &dump_file_path) {
}
static void UpdateFlags(const ConfigSection &section) {
for (auto &&i : section.GetIgnoredLinkerSetKeys()) {
ignore_linker_set_keys.push_back(i);
}
for (auto &&p : section) {
auto &&key = p.first;
bool value_bool = p.second;
@@ -223,12 +231,16 @@ int main(int argc, const char **argv) {
ignored_symbols = LoadIgnoredSymbols(ignore_symbol_list);
}
std::set<std::string> ignored_linker_set_keys_set(
ignore_linker_set_keys.begin(), ignore_linker_set_keys.end());
DiffPolicyOptions diff_policy_options(consider_opaque_types_different);
HeaderAbiDiff judge(lib_name, arch, old_dump, new_dump, compatibility_report,
ignored_symbols, allow_adding_removing_weak_symbols,
diff_policy_options, check_all_apis, text_format_old,
text_format_new, text_format_diff);
ignored_symbols, ignored_linker_set_keys_set,
allow_adding_removing_weak_symbols, diff_policy_options,
check_all_apis, text_format_old, text_format_new,
text_format_diff);
CompatibilityStatusIR status = judge.GenerateCompatibilityReport();

View File

@@ -63,7 +63,7 @@ MergeStatus ModuleMerger::LookupUserDefinedType(
std::set<std::string> type_cache;
repr::DiffPolicyOptions diff_policy_options(false);
repr::AbiDiffHelper diff_helper(module_->type_graph_, addend.type_graph_,
diff_policy_options, &type_cache, nullptr);
diff_policy_options, &type_cache, {}, nullptr);
// Compare each user-defined type with the latest input user-defined type.
// If there is a match, re-use the existing user-defined type.

View File

@@ -732,6 +732,11 @@ DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
const TypeIR *old_type, const TypeIR *new_type,
LinkableMessageKind kind, std::deque<std::string> *type_queue,
DiffMessageIR::DiffKind diff_kind) {
if (ignored_linker_set_keys_.find(new_type->GetLinkerSetKey()) !=
ignored_linker_set_keys_.end()) {
return DiffStatus::no_diff;
}
if (kind == LinkableMessageKind::BuiltinTypeKind) {
return CompareBuiltinTypes(
static_cast<const BuiltinTypeIR *>(old_type),

View File

@@ -20,6 +20,7 @@
#include "repr/ir_representation.h"
#include <deque>
#include <set>
namespace header_checker {
@@ -80,9 +81,11 @@ class AbiDiffHelper {
const AbiElementMap<const TypeIR *> &new_types,
const DiffPolicyOptions &diff_policy_options,
std::set<std::string> *type_cache,
const std::set<std::string> &ignored_linker_set_keys,
IRDiffDumper *ir_diff_dumper = nullptr)
: old_types_(old_types), new_types_(new_types),
diff_policy_options_(diff_policy_options), type_cache_(type_cache),
ignored_linker_set_keys_(ignored_linker_set_keys),
ir_diff_dumper_(ir_diff_dumper) {}
DiffStatus CompareAndDumpTypeDiff(
@@ -208,6 +211,7 @@ class AbiDiffHelper {
const AbiElementMap<const TypeIR *> &new_types_;
const DiffPolicyOptions &diff_policy_options_;
std::set<std::string> *type_cache_;
const std::set<std::string> &ignored_linker_set_keys_;
IRDiffDumper *ir_diff_dumper_;
};

View File

@@ -39,6 +39,15 @@ static std::map<std::string, bool> LoadFlags(const Json::Value &section) {
return map;
}
static std::vector<std::string>
LoadIgnoreLinkerSetKeys(const Json::Value &section) {
std::vector<std::string> vec;
for (auto &key : section.get("ignore_linker_set_keys", {})) {
vec.emplace_back(key.asString());
}
return vec;
}
bool ConfigFile::HasGlobalSection() {
return HasSection(GLOBAL_SECTION_NAME, "");
}
@@ -59,12 +68,15 @@ bool ConfigFile::Load(std::istream &istream) {
if (key == GLOBAL_SECTION_NAME) {
ConfigSection &config_section = map_[{GLOBAL_SECTION_NAME, ""}];
config_section.map_ = LoadFlags(root[GLOBAL_SECTION_NAME]);
config_section.ignored_linker_set_keys_ =
LoadIgnoreLinkerSetKeys(root[GLOBAL_SECTION_NAME]);
continue;
}
for (auto &section : root[key]) {
ConfigSection &config_section =
map_[{key, section["target_version"].asString()}];
config_section.map_ = LoadFlags(section);
config_section.ignored_linker_set_keys_ = LoadIgnoreLinkerSetKeys(section);
}
}
return true;

View File

@@ -19,6 +19,7 @@
#include <iosfwd>
#include <map>
#include <string>
#include <vector>
namespace header_checker {
@@ -48,6 +49,10 @@ class ConfigSection {
return it->second;
}
const std::vector<std::string> &GetIgnoredLinkerSetKeys() const {
return ignored_linker_set_keys_;
}
bool operator[](const std::string &name) const { return GetProperty(name); }
const_iterator begin() const { return map_.begin(); }
@@ -61,6 +66,7 @@ class ConfigSection {
private:
MapType map_;
std::vector<std::string> ignored_linker_set_keys_;
friend class ConfigFile;
};

View File

@@ -15,6 +15,8 @@
#include "utils/config_file.h"
#include <sstream>
#include <string>
#include <vector>
#include <gtest/gtest.h>
@@ -29,6 +31,10 @@ TEST(ConfigFileTest, Parse) {
/* embedded comment */
{
"global": {
"ignore_linker_set_keys": [
"set_key1",
"set_key3",
],
"flags": {
"key1": true,
"key2": false,
@@ -37,6 +43,10 @@ TEST(ConfigFileTest, Parse) {
"library1": [
{
"target_version": "current",
"ignore_linker_set_keys": [
"set_key1",
"set_key2",
],
"flags": {
"key1": true,
"key2": false,
@@ -68,12 +78,14 @@ TEST(ConfigFileTest, Parse) {
EXPECT_FALSE(section1.HasProperty("key3"));
EXPECT_FALSE(section1.GetProperty("key3"));
EXPECT_EQ(std::vector<std::string>({"set_key1", "set_key3"}), section1.GetIgnoredLinkerSetKeys());
auto &&section2 = cfg.GetSection("library1", "current");
EXPECT_TRUE(section2.HasProperty("key1"));
EXPECT_TRUE(section2.GetProperty("key1"));
EXPECT_TRUE(section2.HasProperty("key2"));
EXPECT_FALSE(section2.GetProperty("key2"));
EXPECT_EQ(std::vector<std::string>({"set_key1", "set_key2"}), section2.GetIgnoredLinkerSetKeys());
EXPECT_TRUE(cfg.GetProperty("global", "", "key1"));
EXPECT_TRUE(cfg.GetProperty("library1", "34", "key2"));