From 2ce72d8673d96010300a99038d74fa17104e2bba Mon Sep 17 00:00:00 2001 From: Mu-Le Lee Date: Wed, 10 Aug 2022 15:11:17 +0000 Subject: [PATCH] Implement the Cross-Version ABI diff configuration Enable the Cross-Version ABI diff configuration so developers can specify different flags for current and previous version ABI Check. The -target-version flag were added to determine the config section to be selected. The details of this configuration logic could be found in go/cross-version-abi-diff-configuration. Test: preform abi diff with config.json Bug: 239792343 Change-Id: I8fdad2d18096cfa7866183ecc2d7826682c85eb6 --- .../src/diff/header_abi_diff.cpp | 61 ++++++++++++------- .../header-checker/src/utils/config_file.cpp | 42 ++++++++++--- .../header-checker/src/utils/config_file.h | 50 ++++++--------- .../src/utils/config_file_test.cpp | 41 ++++++++----- 4 files changed, 120 insertions(+), 74 deletions(-) diff --git a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp index c14f69b35..0ce370362 100644 --- a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp +++ b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp @@ -30,6 +30,7 @@ using header_checker::repr::CompatibilityStatusIR; using header_checker::repr::DiffPolicyOptions; using header_checker::repr::TextFormatIR; using header_checker::utils::ConfigFile; +using header_checker::utils::ConfigSection; static llvm::cl::OptionCategory header_checker_category( @@ -129,6 +130,15 @@ static llvm::cl::opt allow_adding_removing_weak_symbols( llvm::cl::init(false), llvm::cl::Optional, llvm::cl::cat(header_checker_category)); +static llvm::cl::opt target_version( + "target-version", + llvm::cl::desc( + "Load the flags for and from config.json in " + "the old dump's parent directory." + ), + llvm::cl::init("current"), llvm::cl::Optional, + llvm::cl::cat(header_checker_category)); + static std::set LoadIgnoredSymbols(std::string &symbol_list_path) { std::ifstream symbol_ifstream(symbol_list_path); std::set ignored_symbols; @@ -150,33 +160,40 @@ static std::string GetConfigFilePath(const std::string &dump_file_path) { return std::string(config_file_path); } +static void UpdateFlags(const ConfigSection §ion) { + for (auto &&p : section) { + auto &&key = p.first; + bool value_bool = p.second; + if (key == "allow_adding_removing_weak_symbols") { + allow_adding_removing_weak_symbols = value_bool; + } else if (key == "advice_only") { + advice_only = value_bool; + } else if (key == "elf_unreferenced_symbol_errors") { + elf_unreferenced_symbol_errors = value_bool; + } else if (key == "check_all_apis") { + check_all_apis = value_bool; + } else if (key == "allow_extensions") { + allow_extensions = value_bool; + } else if (key == "allow_unreferenced_elf_symbol_changes") { + allow_unreferenced_elf_symbol_changes = value_bool; + } else if (key == "allow_unreferenced_changes") { + allow_unreferenced_changes = value_bool; + } else if (key == "consider_opaque_types_different") { + consider_opaque_types_different = value_bool; + } + } +} + static void ReadConfigFile(const std::string &config_file_path) { ConfigFile cfg; if (!cfg.Load(config_file_path)) { ::exit(1); } - if (cfg.HasSection("global")) { - for (auto &&p : cfg.GetSection("global")) { - auto &&key = p.first; - bool value_bool = p.second; - if (key == "allow_adding_removing_weak_symbols") { - allow_adding_removing_weak_symbols = value_bool; - } else if (key == "advice_only") { - advice_only = value_bool; - } else if (key == "elf_unreferenced_symbol_errors") { - elf_unreferenced_symbol_errors = value_bool; - } else if (key == "check_all_apis") { - check_all_apis = value_bool; - } else if (key == "allow_extensions") { - allow_extensions = value_bool; - } else if (key == "allow_unreferenced_elf_symbol_changes") { - allow_unreferenced_elf_symbol_changes = value_bool; - } else if (key == "allow_unreferenced_changes") { - allow_unreferenced_changes = value_bool; - } else if (key == "consider_opaque_types_different") { - consider_opaque_types_different = value_bool; - } - } + if (cfg.HasGlobalSection()) { + UpdateFlags(cfg.GetGlobalSection()); + } + if (cfg.HasSection(lib_name, target_version)) { + UpdateFlags(cfg.GetSection(lib_name, target_version)); } } diff --git a/vndk/tools/header-checker/src/utils/config_file.cpp b/vndk/tools/header-checker/src/utils/config_file.cpp index 9a49b08ad..c6a8eb2d2 100644 --- a/vndk/tools/header-checker/src/utils/config_file.cpp +++ b/vndk/tools/header-checker/src/utils/config_file.cpp @@ -18,13 +18,35 @@ #include #include +#include #include +static const std::string GLOBAL_SECTION_NAME = "global"; + + namespace header_checker { namespace utils { +static std::map LoadFlags(const Json::Value §ion) { + std::map map; + if (section.isMember("flags")) { + for (auto &flag_keys : section["flags"].getMemberNames()) { + map[flag_keys] = section["flags"][flag_keys].asBool(); + } + } + return map; +} + +bool ConfigFile::HasGlobalSection() { + return HasSection(GLOBAL_SECTION_NAME, ""); +} + +const ConfigSection &ConfigFile::GetGlobalSection() { + return GetSection(GLOBAL_SECTION_NAME, ""); +} + bool ConfigFile::Load(std::istream &istream) { Json::Value root; Json::CharReaderBuilder builder; @@ -34,11 +56,15 @@ bool ConfigFile::Load(std::istream &istream) { return false; } for (auto &key : root.getMemberNames()) { - map_[key] = ConfigSection(); - if (root[key].isMember("flags")) { - for (auto &flag_keys : root[key]["flags"].getMemberNames()) { - map_[key].map_[flag_keys] = root[key]["flags"][flag_keys].asBool(); - } + if (key == GLOBAL_SECTION_NAME) { + ConfigSection &config_section = map_[{GLOBAL_SECTION_NAME, ""}]; + config_section.map_ = LoadFlags(root[GLOBAL_SECTION_NAME]); + continue; + } + for (auto §ion : root[key]) { + ConfigSection &config_section = + map_[{key, section["target_version"].asString()}]; + config_section.map_ = LoadFlags(section); } } return true; @@ -46,9 +72,11 @@ bool ConfigFile::Load(std::istream &istream) { bool ConfigFile::Load(const std::string &path) { std::ifstream stream(path); - return Load(stream); + if (stream.is_open()) { + return Load(stream); + } + return false; } - } // namespace utils } // namespace header_checker diff --git a/vndk/tools/header-checker/src/utils/config_file.h b/vndk/tools/header-checker/src/utils/config_file.h index 2dfe7eab0..215215dfe 100644 --- a/vndk/tools/header-checker/src/utils/config_file.h +++ b/vndk/tools/header-checker/src/utils/config_file.h @@ -48,18 +48,11 @@ class ConfigSection { return it->second; } - bool operator[](const std::string &name) const { - return GetProperty(name); - } + bool operator[](const std::string &name) const { return GetProperty(name); } - const_iterator begin() const { - return map_.begin(); - } - - const_iterator end() const { - return map_.end(); - } + const_iterator begin() const { return map_.begin(); } + const_iterator end() const { return map_.end(); } private: ConfigSection(const ConfigSection &) = delete; @@ -75,7 +68,7 @@ class ConfigSection { class ConfigFile { public: - using MapType = std::map; + using MapType = std::map, ConfigSection>; using const_iterator = MapType::const_iterator; @@ -87,46 +80,43 @@ class ConfigFile { bool Load(const std::string &path); bool Load(std::istream &istream); - bool HasSection(const std::string §ion_name) const { - return map_.find(section_name) != map_.end(); + bool HasSection(const std::string §ion_name, + const std::string &version) const { + return map_.find({section_name, version}) != map_.end(); } - const ConfigSection &GetSection(const std::string §ion_name) const { - auto &&it = map_.find(section_name); + const ConfigSection &GetSection(const std::string §ion_name, + const std::string &version) const { + auto &&it = map_.find({section_name, version}); assert(it != map_.end()); return it->second; } - const ConfigSection &operator[](const std::string §ion_name) const { - return GetSection(section_name); - } + bool HasGlobalSection(); - bool HasProperty(const std::string §ion_name, + const ConfigSection &GetGlobalSection(); + + bool HasProperty(const std::string §ion_name, const std::string &version, const std::string &property_name) const { - auto &&it = map_.find(section_name); + auto &&it = map_.find({section_name, version}); if (it == map_.end()) { return false; } return it->second.HasProperty(property_name); } - bool GetProperty(const std::string §ion_name, - const std::string &property_name) const { - auto &&it = map_.find(section_name); + bool GetProperty(const std::string §ion_name, const std::string &version, + const std::string &property_name) const { + auto &&it = map_.find({section_name, version}); if (it == map_.end()) { return false; } return it->second.GetProperty(property_name); } - const_iterator begin() const { - return map_.begin(); - } - - const_iterator end() const { - return map_.end(); - } + const_iterator begin() const { return map_.begin(); } + const_iterator end() const { return map_.end(); } private: ConfigFile(const ConfigFile &) = delete; diff --git a/vndk/tools/header-checker/src/utils/config_file_test.cpp b/vndk/tools/header-checker/src/utils/config_file_test.cpp index 2660de4ba..806d15353 100644 --- a/vndk/tools/header-checker/src/utils/config_file_test.cpp +++ b/vndk/tools/header-checker/src/utils/config_file_test.cpp @@ -28,28 +28,39 @@ TEST(ConfigFileTest, Parse) { // Comment line starts with slash /* embedded comment */ { - "section1": { + "global": { "flags": { "key1": true, "key2": false, }, }, - "section2": { - "flags": { - "key1": true, - "key2": false, + "library1": [ + { + "target_version": "current", + "flags": { + "key1": true, + "key2": false, + }, }, - }, + { + "target_version": "34", + "flags": { + "key1": false, + "key2": true, + }, + }, + ], } )"); ConfigFile cfg; cfg.Load(stream); - EXPECT_TRUE(cfg.HasSection("section1")); - EXPECT_TRUE(cfg.HasSection("section2")); - EXPECT_FALSE(cfg.HasSection("section3")); + EXPECT_TRUE(cfg.HasSection("global", "")); + EXPECT_TRUE(cfg.HasSection("library1", "current")); + EXPECT_FALSE(cfg.HasSection("library1", "35")); + EXPECT_FALSE(cfg.HasSection("library2", "current")); - auto &§ion1 = cfg.GetSection("section1"); + auto &§ion1 = cfg.GetSection("global", ""); EXPECT_TRUE(section1.HasProperty("key1")); EXPECT_TRUE(section1.GetProperty("key1")); EXPECT_TRUE(section1.HasProperty("key2")); @@ -58,17 +69,17 @@ TEST(ConfigFileTest, Parse) { EXPECT_FALSE(section1.HasProperty("key3")); EXPECT_FALSE(section1.GetProperty("key3")); - auto &§ion2 = cfg.GetSection("section2"); + auto &§ion2 = 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_TRUE(cfg.GetProperty("section1", "key1")); - EXPECT_FALSE(cfg.GetProperty("section1", "key2")); + EXPECT_TRUE(cfg.GetProperty("global", "", "key1")); + EXPECT_TRUE(cfg.GetProperty("library1", "34", "key2")); - EXPECT_TRUE(cfg["section1"]["key1"]); - EXPECT_FALSE(cfg["section1"]["key2"]); + EXPECT_TRUE(section1["key1"]); + EXPECT_FALSE(section2["key2"]); } TEST(ConfigFileTest, BadJsonFormat) {