diff --git a/vndk/tools/header-checker/Android.bp b/vndk/tools/header-checker/Android.bp index d83bd477f..aff2d0554 100644 --- a/vndk/tools/header-checker/Android.bp +++ b/vndk/tools/header-checker/Android.bp @@ -165,6 +165,7 @@ cc_library_host_static { "src/repr/symbol/so_file_parser.cpp", "src/repr/symbol/version_script_parser.cpp", "src/utils/api_level.cpp", + "src/utils/config_file.cpp", "src/utils/collect_exported_headers.cpp", "src/utils/string_utils.cpp", ], @@ -198,6 +199,7 @@ cc_test_host { "src/repr/symbol/exported_symbol_set_test.cpp", "src/repr/symbol/version_script_parser_test.cpp", "src/utils/api_level_test.cpp", + "src/utils/config_file_test.cpp", "src/utils/string_utils_test.cpp", ], diff --git a/vndk/tools/header-checker/src/diff/abi_diff.cpp b/vndk/tools/header-checker/src/diff/abi_diff.cpp index c06654b68..3d561b3fb 100644 --- a/vndk/tools/header-checker/src/diff/abi_diff.cpp +++ b/vndk/tools/header-checker/src/diff/abi_diff.cpp @@ -66,7 +66,7 @@ repr::CompatibilityStatusIR HeaderAbiDiff::CompareTUs( const AbiElementMap new_types = new_tu.GetTypeGraph(); - // Collect fills in added, removed ,unsafe and safe function diffs. + // CollectDynsymExportables() fills in added, removed, unsafe, and safe function diffs. if (!CollectDynsymExportables(old_tu.GetFunctions(), new_tu.GetFunctions(), old_tu.GetElfFunctions(), new_tu.GetElfFunctions(), @@ -230,11 +230,11 @@ bool HeaderAbiDiff::Collect( const AbiElementMap &old_types_map, const AbiElementMap &new_types_map) { if (!PopulateRemovedElements( - old_elements_map, new_elements_map, new_elf_map, ir_diff_dumper, - repr::DiffMessageIR::Removed, old_types_map) || + old_elements_map, new_elements_map, old_elf_map, new_elf_map, + ir_diff_dumper, repr::DiffMessageIR::Removed, old_types_map) || !PopulateRemovedElements( - new_elements_map, old_elements_map, old_elf_map, ir_diff_dumper, - repr::IRDiffDumper::DiffKind::Added, new_types_map)) { + new_elements_map, old_elements_map, new_elf_map, old_elf_map, + ir_diff_dumper, repr::DiffMessageIR::Added, new_types_map)) { llvm::errs() << "Populating functions in report failed\n"; return false; } @@ -262,6 +262,10 @@ bool HeaderAbiDiff::PopulateElfElements( repr::IRDiffDumper *ir_diff_dumper, repr::IRDiffDumper::DiffKind diff_kind) { for (auto &&elf_element : elf_elements) { + if (allow_adding_removing_weak_symbols_ && + elf_element->GetBinding() == repr::ElfSymbolIR::Weak) { + continue; + } if (!ir_diff_dumper->AddElfSymbolMessageIR(elf_element, diff_kind)) { return false; } @@ -273,15 +277,16 @@ template bool HeaderAbiDiff::PopulateRemovedElements( const AbiElementMap &old_elements_map, const AbiElementMap &new_elements_map, - const AbiElementMap *elf_map, + const AbiElementMap *old_elf_map, + const AbiElementMap *new_elf_map, repr::IRDiffDumper *ir_diff_dumper, repr::IRDiffDumper::DiffKind diff_kind, const AbiElementMap &removed_types_map) { std::vector removed_elements = utils::FindRemovedElements(old_elements_map, new_elements_map); - if (!DumpLoneElements(removed_elements, elf_map, ir_diff_dumper, diff_kind, - removed_types_map)) { - llvm::errs() << "Dumping added / removed element to report failed\n"; + if (!DumpLoneElements(removed_elements, old_elf_map, new_elf_map, + ir_diff_dumper, diff_kind, removed_types_map)) { + llvm::errs() << "Dumping added or removed element to report failed\n"; return false; } return true; @@ -311,33 +316,52 @@ bool HeaderAbiDiff::PopulateCommonElements( template bool HeaderAbiDiff::DumpLoneElements( std::vector &elements, - const AbiElementMap *elf_map, + const AbiElementMap *old_elf_map, + const AbiElementMap *new_elf_map, repr::IRDiffDumper *ir_diff_dumper, repr::IRDiffDumper::DiffKind diff_kind, const AbiElementMap &types_map) { - // If the record / enum has source file information, skip it. std::smatch source_file_match; std::regex source_file_regex(" at "); + for (auto &&element : elements) { if (IgnoreSymbol(element, ignored_symbols_, [](const T *e) {return e->GetLinkerSetKey();})) { continue; } - // The element does exist in the .dynsym table, we do not have meta-data - // surrounding the element. + + // If an element (FunctionIR or GlobalVarIR) is missing from the new ABI + // dump but a corresponding ELF symbol (ElfFunctionIR or ElfObjectIR) can + // be found in the new ABI dump file, don't emit error on this element. + // This may happen when the standard reference target implements the + // function (or the global variable) in C/C++ and the target-under-test + // implements the function (or the global variable) in assembly. const std::string &element_linker_set_key = element->GetLinkerSetKey(); - if ((elf_map != nullptr) && - (elf_map->find(element_linker_set_key) != elf_map->end())) { + if (new_elf_map && + new_elf_map->find(element_linker_set_key) != new_elf_map->end()) { continue; } + + // If the `-ignore-weak-symbols` option is enabled, ignore the element if + // it was a weak symbol. + if (allow_adding_removing_weak_symbols_ && old_elf_map) { + auto elem_it = old_elf_map->find(element_linker_set_key); + if (elem_it != old_elf_map->end() && + elem_it->second->GetBinding() == repr::ElfSymbolIR::Weak) { + continue; + } + } + + // If the record / enum has source file information, skip it. if (std::regex_search(element_linker_set_key, source_file_match, source_file_regex)) { continue; } + auto element_copy = *element; ReplaceTypeIdsWithTypeNames(types_map, &element_copy); if (!ir_diff_dumper->AddLinkableMessageIR(&element_copy, diff_kind)) { - llvm::errs() << "Couldn't dump added /removed element\n"; + llvm::errs() << "Couldn't dump added or removed element\n"; return false; } } diff --git a/vndk/tools/header-checker/src/diff/abi_diff.h b/vndk/tools/header-checker/src/diff/abi_diff.h index cbda960c1..ca64716a5 100644 --- a/vndk/tools/header-checker/src/diff/abi_diff.h +++ b/vndk/tools/header-checker/src/diff/abi_diff.h @@ -36,6 +36,7 @@ class HeaderAbiDiff { const std::string &old_dump, const std::string &new_dump, const std::string &compatibility_report, const std::set &ignored_symbols, + bool allow_adding_removing_weak_symbols, const DiffPolicyOptions &diff_policy_options, bool check_all_apis, repr::TextFormatIR text_format_old, repr::TextFormatIR text_format_new, @@ -44,6 +45,7 @@ class HeaderAbiDiff { new_dump_(new_dump), cr_(compatibility_report), ignored_symbols_(ignored_symbols), diff_policy_options_(diff_policy_options), + allow_adding_removing_weak_symbols_(allow_adding_removing_weak_symbols), check_all_apis_(check_all_apis), text_format_old_(text_format_old), text_format_new_(text_format_new), text_format_diff_(text_format_diff) {} @@ -90,7 +92,8 @@ class HeaderAbiDiff { bool PopulateRemovedElements( const AbiElementMap &old_elements_map, const AbiElementMap &new_elements_map, - const AbiElementMap *elf_map, + const AbiElementMap *old_elf_map, + const AbiElementMap *new_elf_map, repr::IRDiffDumper *ir_diff_dumper, repr::IRDiffDumper::DiffKind diff_kind, const AbiElementMap &types_map); @@ -115,7 +118,8 @@ class HeaderAbiDiff { template bool DumpLoneElements( std::vector &elements, - const AbiElementMap *elf_map, + const AbiElementMap *old_elf_map, + const AbiElementMap *new_elf_map, repr::IRDiffDumper *ir_diff_dumper, repr::IRDiffDumper::DiffKind diff_kind, const AbiElementMap &old_types_map); @@ -147,6 +151,7 @@ class HeaderAbiDiff { const std::string &cr_; const std::set &ignored_symbols_; const DiffPolicyOptions &diff_policy_options_; + bool allow_adding_removing_weak_symbols_; bool check_all_apis_; std::set type_cache_; repr::TextFormatIR text_format_old_; 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 ac13b535f..e0ead7423 100644 --- a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp +++ b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp @@ -14,8 +14,13 @@ #include "diff/abi_diff.h" +#include "utils/config_file.h" +#include "utils/string_utils.h" + +#include #include #include +#include #include #include @@ -25,6 +30,9 @@ using header_checker::diff::HeaderAbiDiff; using header_checker::repr::CompatibilityStatusIR; using header_checker::repr::DiffPolicyOptions; using header_checker::repr::TextFormatIR; +using header_checker::utils::ConfigFile; +using header_checker::utils::ConfigParser; +using header_checker::utils::ParseBool; static llvm::cl::OptionCategory header_checker_category( @@ -70,10 +78,6 @@ static llvm::cl::opt check_all_apis( " the dynsym table of a shared library are checked"), llvm::cl::Optional, llvm::cl::cat(header_checker_category)); -static llvm::cl::opt suppress_local_warnings( - "suppress_local_warnings", llvm::cl::desc("suppress local warnings"), - llvm::cl::Optional, llvm::cl::cat(header_checker_category)); - static llvm::cl::opt allow_extensions( "allow-extensions", llvm::cl::desc("Do not return a non zero status on extensions"), @@ -121,6 +125,13 @@ static llvm::cl::opt text_format_diff( llvm::cl::init(TextFormatIR::ProtobufTextFormat), llvm::cl::cat(header_checker_category)); +static llvm::cl::opt allow_adding_removing_weak_symbols( + "allow-adding-removing-weak-symbols", + llvm::cl::desc("Do not treat addition or removal of weak symbols as " + "incompatible changes."), + llvm::cl::init(false), 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; @@ -135,6 +146,39 @@ static std::set LoadIgnoredSymbols(std::string &symbol_list_path) { return ignored_symbols; } +static std::string GetConfigFilePath(const std::string &dump_file_path) { + llvm::SmallString<128> config_file_path(dump_file_path); + llvm::sys::path::remove_filename(config_file_path); + llvm::sys::path::append(config_file_path, "config.ini"); + return config_file_path.str(); +} + +static void ReadConfigFile(const std::string &config_file_path) { + ConfigFile cfg = ConfigParser::ParseFile(config_file_path); + if (cfg.HasSection("global")) { + for (auto &&[key, value] : cfg.GetSection("global")) { + bool value_bool = ParseBool(value); + 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 const char kWarn[] = "\033[36;1mwarning: \033[0m"; static const char kError[] = "\033[31;1merror: \033[0m"; @@ -150,15 +194,20 @@ bool ShouldEmitWarningMessage(CompatibilityStatusIR status) { int main(int argc, const char **argv) { llvm::cl::ParseCommandLineOptions(argc, argv, "header-checker"); + + ReadConfigFile(GetConfigFilePath(old_dump)); + std::set ignored_symbols; if (llvm::sys::fs::exists(ignore_symbol_list)) { ignored_symbols = LoadIgnoredSymbols(ignore_symbol_list); } - DiffPolicyOptions diff_policy_options( - consider_opaque_types_different); + + DiffPolicyOptions diff_policy_options(consider_opaque_types_different); + HeaderAbiDiff judge(lib_name, arch, old_dump, new_dump, compatibility_report, - ignored_symbols, diff_policy_options, check_all_apis, - text_format_old, text_format_new, text_format_diff); + ignored_symbols, allow_adding_removing_weak_symbols, + diff_policy_options, check_all_apis, text_format_old, + text_format_new, text_format_diff); CompatibilityStatusIR status = judge.GenerateCompatibilityReport(); diff --git a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp index 98ecea3f3..eecbfe554 100644 --- a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp +++ b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.cpp @@ -120,11 +120,17 @@ VersionScriptParser::ParsedTags VersionScriptParser::ParseSymbolTags( continue; } - // Check future tags. + // Check the future tag. if (tag == "future") { result.has_future_tag_ = true; continue; } + + // Check the weak binding tag. + if (tag == "weak") { + result.has_weak_tag_ = true; + continue; + } } return result; @@ -183,11 +189,14 @@ bool VersionScriptParser::ParseSymbolLine(const std::string &line, return true; } + ElfSymbolIR::ElfSymbolBinding binding = + tags.has_weak_tag_ ? ElfSymbolIR::ElfSymbolBinding::Weak + : ElfSymbolIR::ElfSymbolBinding::Global; + if (tags.has_var_tag_) { - exported_symbols_->AddVar(symbol, ElfSymbolIR::ElfSymbolBinding::Global); + exported_symbols_->AddVar(symbol, binding); } else { - exported_symbols_->AddFunction(symbol, - ElfSymbolIR::ElfSymbolBinding::Global); + exported_symbols_->AddFunction(symbol, binding); } return true; } diff --git a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h index 7a047a299..d3bb63708 100644 --- a/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h +++ b/vndk/tools/header-checker/src/repr/symbol/version_script_parser.h @@ -44,6 +44,7 @@ class VersionScriptParser { unsigned has_excluded_tags_ : 1; unsigned has_future_tag_ : 1; unsigned has_var_tag_ : 1; + unsigned has_weak_tag_ : 1; utils::ApiLevel introduced_; diff --git a/vndk/tools/header-checker/src/utils/config_file.cpp b/vndk/tools/header-checker/src/utils/config_file.cpp new file mode 100644 index 000000000..e4861c62a --- /dev/null +++ b/vndk/tools/header-checker/src/utils/config_file.cpp @@ -0,0 +1,87 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "utils/config_file.h" + +#include "utils/string_utils.h" + +#include +#include +#include + + +namespace header_checker { +namespace utils { + + +ConfigFile ConfigParser::ParseFile(std::istream &istream) { + ConfigParser parser(istream); + return parser.ParseFile(); +} + + +ConfigFile ConfigParser::ParseFile(const std::string &path) { + std::ifstream stream(path, std::ios_base::in); + return ParseFile(stream); +} + + +ConfigFile ConfigParser::ParseFile() { + size_t line_no = 0; + std::string line; + while (std::getline(stream_, line)) { + ParseLine(++line_no, line); + } + return std::move(cfg_); +} + + +void ConfigParser::ParseLine(size_t line_no, std::string_view line) { + if (line.empty() || line[0] == ';' || line[0] == '#') { + // Skip empty or comment line. + return; + } + + // Parse section name line. + if (line[0] == '[') { + std::string::size_type pos = line.rfind(']'); + if (pos == std::string::npos) { + ReportError(line_no, "bad section name line"); + return; + } + std::string_view section_name = line.substr(1, pos - 1); + section_ = &cfg_.map_[std::string(section_name)]; + return; + } + + // Parse key-value line. + std::string::size_type pos = line.find('='); + if (pos == std::string::npos) { + ReportError(line_no, "bad key-value line"); + return; + } + + // Add key-value entry to current section. + std::string_view key = Trim(line.substr(0, pos)); + std::string_view value = Trim(line.substr(pos + 1)); + + if (!section_) { + section_ = &cfg_.map_[""]; + } + section_->map_[std::string(key)] = std::string(value); +} + + +} // 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 new file mode 100644 index 000000000..cf8c6d905 --- /dev/null +++ b/vndk/tools/header-checker/src/utils/config_file.h @@ -0,0 +1,193 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CONFIG_FILE_H_ +#define CONFIG_FILE_H_ + +#include +#include +#include +#include + + +namespace header_checker { +namespace utils { + + +class ConfigParser; + + +class ConfigSection { + public: + using MapType = std::map; + using const_iterator = MapType::const_iterator; + + + public: + ConfigSection() = default; + ConfigSection(ConfigSection &&) = default; + ConfigSection &operator=(ConfigSection &&) = default; + + bool HasProperty(const std::string &name) const { + return map_.find(name) != map_.end(); + } + + std::string GetProperty(const std::string &name) const { + auto &&it = map_.find(name); + if (it == map_.end()) { + return ""; + } + return it->second; + } + + std::string operator[](const std::string &name) const { + return GetProperty(name); + } + + const_iterator begin() const { + return map_.begin(); + } + + const_iterator end() const { + return map_.end(); + } + + + private: + ConfigSection(const ConfigSection &) = delete; + ConfigSection &operator=(const ConfigSection &) = delete; + + + private: + std::map map_; + + friend class ConfigParser; +}; + + +class ConfigFile { + public: + using MapType = std::map; + using const_iterator = MapType::const_iterator; + + + public: + ConfigFile() = default; + ConfigFile(ConfigFile &&) = default; + ConfigFile &operator=(ConfigFile &&) = default; + + bool HasSection(const std::string §ion_name) const { + return map_.find(section_name) != map_.end(); + } + + const ConfigSection &GetSection(const std::string §ion_name) const { + auto &&it = map_.find(section_name); + assert(it != map_.end()); + return it->second; + } + + const ConfigSection &operator[](const std::string §ion_name) const { + return GetSection(section_name); + } + + bool HasProperty(const std::string §ion_name, + const std::string &property_name) const { + auto &&it = map_.find(section_name); + if (it == map_.end()) { + return false; + } + return it->second.HasProperty(property_name); + } + + std::string GetProperty(const std::string §ion_name, + const std::string &property_name) const { + auto &&it = map_.find(section_name); + if (it == map_.end()) { + return ""; + } + return it->second.GetProperty(property_name); + } + + const_iterator begin() const { + return map_.begin(); + } + + const_iterator end() const { + return map_.end(); + } + + + private: + ConfigFile(const ConfigFile &) = delete; + ConfigFile &operator=(const ConfigFile &) = delete; + + + private: + std::map map_; + + friend class ConfigParser; +}; + + +class ConfigParser { + public: + using ErrorListener = std::function; + + + public: + ConfigParser(std::istream &stream) + : stream_(stream), section_(nullptr) { } + + ConfigFile ParseFile(); + + static ConfigFile ParseFile(std::istream &istream); + + static ConfigFile ParseFile(const std::string &path); + + void SetErrorListener(ErrorListener listener) { + error_listener_ = std::move(listener); + } + + + private: + void ParseLine(size_t line_no, std::string_view line); + + void ReportError(size_t line_no, const char *cause) { + if (error_listener_) { + error_listener_(line_no, cause); + } + } + + + private: + ConfigParser(const ConfigParser &) = delete; + ConfigParser &operator=(const ConfigParser &) = delete; + + + private: + std::istream &stream_; + + ErrorListener error_listener_; + + ConfigSection *section_; + + ConfigFile cfg_; +}; + + +} // namespace utils +} // namespace header_checker + + +#endif // CONFIG_FILE_H_ diff --git a/vndk/tools/header-checker/src/utils/config_file_test.cpp b/vndk/tools/header-checker/src/utils/config_file_test.cpp new file mode 100644 index 000000000..28ead1040 --- /dev/null +++ b/vndk/tools/header-checker/src/utils/config_file_test.cpp @@ -0,0 +1,111 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "utils/config_file.h" + +#include + +#include + + +namespace header_checker { +namespace utils { + + +TEST(ConfigParserTest, Parse) { + std::stringstream stream(R"( +# Comment line starts with hash symbol +; Comment line starts with semicolon + +[section1] +key1 = value1 +key2 = value2 + +[section2] +key1 = true +key2 = false +)"); + + auto &&cfg = ConfigParser::ParseFile(stream); + EXPECT_TRUE(cfg.HasSection("section1")); + EXPECT_TRUE(cfg.HasSection("section2")); + EXPECT_FALSE(cfg.HasSection("section3")); + + auto &§ion1 = cfg.GetSection("section1"); + EXPECT_TRUE(section1.HasProperty("key1")); + EXPECT_EQ("value1", section1.GetProperty("key1")); + EXPECT_TRUE(section1.HasProperty("key2")); + EXPECT_EQ("value2", section1.GetProperty("key2")); + + EXPECT_FALSE(section1.HasProperty("key3")); + EXPECT_EQ("", section1.GetProperty("key3")); + + auto &§ion2 = cfg.GetSection("section2"); + EXPECT_TRUE(section2.HasProperty("key1")); + EXPECT_EQ("true", section2.GetProperty("key1")); + EXPECT_TRUE(section2.HasProperty("key2")); + EXPECT_EQ("false", section2.GetProperty("key2")); + + EXPECT_EQ("value1", cfg.GetProperty("section1", "key1")); + EXPECT_EQ("value2", cfg.GetProperty("section1", "key2")); + + EXPECT_EQ("value1", cfg["section1"]["key1"]); + EXPECT_EQ("value2", cfg["section1"]["key2"]); +} + + +TEST(ConfigParserTest, BadSectionNameLine) { + std::stringstream stream(R"( +[section1 +key1 = value1 +)"); + + size_t num_errors = 0; + + ConfigParser parser(stream); + parser.SetErrorListener( + [&num_errors](size_t line_no, const char *cause) { + ++num_errors; + EXPECT_EQ(2, line_no); + EXPECT_STREQ("bad section name line", cause); + }); + parser.ParseFile(); + + EXPECT_EQ(1, num_errors); +} + + +TEST(ConfigParserTest, BadKeyValueLine) { + std::stringstream stream(R"( +[section1] +key1 +)"); + + size_t num_errors = 0; + + ConfigParser parser(stream); + parser.SetErrorListener( + [&num_errors](size_t line_no, const char *cause) { + ++num_errors; + EXPECT_EQ(3, line_no); + EXPECT_STREQ("bad key-value line", cause); + }); + parser.ParseFile(); + + EXPECT_EQ(1, num_errors); +} + + +} // namespace utils +} // namespace header_checker diff --git a/vndk/tools/header-checker/src/utils/string_utils.cpp b/vndk/tools/header-checker/src/utils/string_utils.cpp index ced07f754..2dd97aded 100644 --- a/vndk/tools/header-checker/src/utils/string_utils.cpp +++ b/vndk/tools/header-checker/src/utils/string_utils.cpp @@ -14,6 +14,8 @@ #include "utils/string_utils.h" +#include +#include #include #include #include @@ -86,6 +88,13 @@ std::optional ParseInt(const std::string &s) { } +bool ParseBool(const std::string &s) { + std::string value(s); + std::transform(value.begin(), value.end(), value.begin(), std::tolower); + return (value == "true" || value == "on" || value == "1"); +} + + bool IsGlobPattern(std::string_view s) { return s.find_first_of("*?[") != std::string::npos; } diff --git a/vndk/tools/header-checker/src/utils/string_utils.h b/vndk/tools/header-checker/src/utils/string_utils.h index 8d546f400..bcb840504 100644 --- a/vndk/tools/header-checker/src/utils/string_utils.h +++ b/vndk/tools/header-checker/src/utils/string_utils.h @@ -35,6 +35,8 @@ std::vector Split(std::string_view s, std::optional ParseInt(const std::string &s); +bool ParseBool(const std::string &s); + bool IsGlobPattern(std::string_view s); diff --git a/vndk/tools/header-checker/src/utils/string_utils_test.cpp b/vndk/tools/header-checker/src/utils/string_utils_test.cpp index 6c5a874a8..4ad007440 100644 --- a/vndk/tools/header-checker/src/utils/string_utils_test.cpp +++ b/vndk/tools/header-checker/src/utils/string_utils_test.cpp @@ -94,6 +94,20 @@ TEST(StringUtilsTest, ParseInt) { } +TEST(StringUtilsTest, ParseBool) { + EXPECT_FALSE(ParseBool("")); + EXPECT_FALSE(ParseBool("false")); + EXPECT_FALSE(ParseBool("off")); + EXPECT_FALSE(ParseBool("0")); + + EXPECT_TRUE(ParseBool("TRUE")); + EXPECT_TRUE(ParseBool("True")); + EXPECT_TRUE(ParseBool("true")); + EXPECT_TRUE(ParseBool("ON")); + EXPECT_TRUE(ParseBool("1")); +} + + TEST(StringUtilsTest, IsGlobPattern) { EXPECT_TRUE(IsGlobPattern("*.so")); EXPECT_TRUE(IsGlobPattern("[ab].txt")); diff --git a/vndk/tools/header-checker/tests/integration/weak_symbols/example.c b/vndk/tools/header-checker/tests/integration/weak_symbols/example.c new file mode 100644 index 000000000..65c35bf47 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/weak_symbols/example.c @@ -0,0 +1,8 @@ +#if !defined(NEW) + +extern void example(void) __attribute__((weak)); + +void example(void) { +} + +#endif // !defined(NEW) diff --git a/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt new file mode 100644 index 000000000..060916683 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt @@ -0,0 +1,3 @@ +LIBTEST_1.0 { + global: +}; diff --git a/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt new file mode 100644 index 000000000..f457f27a6 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt @@ -0,0 +1,4 @@ +LIBTEST_1.0 { + global: + example; # weak +}; diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py index 3c281777d..9397ac19c 100755 --- a/vndk/tools/header-checker/tests/module.py +++ b/vndk/tools/header-checker/tests/module.py @@ -555,6 +555,37 @@ TEST_MODULES = [ '--exclude-symbol-tag', 'mytag', ] ), + + # Test data for test_allow_adding_removing_weak_symbols + LsdumpModule( + name='libweak_symbols_old', + arch='arm64', + srcs=[ + 'integration/weak_symbols/example.c', + ], + version_script='integration/weak_symbols/libexample_old.map.txt', + export_include_dirs=[], + dumper_flags=['-output-format', 'Json'], + linker_flags=[ + '-input-format', 'Json', + '-output-format', 'Json', + ] + ), + LsdumpModule( + name='libweak_symbols_new', + arch='arm64', + srcs=[ + 'integration/weak_symbols/example.c', + ], + version_script='integration/weak_symbols/libexample_new.map.txt', + export_include_dirs=[], + dumper_flags=['-output-format', 'Json'], + linker_flags=[ + '-input-format', 'Json', + '-output-format', 'Json', + ], + cflags=['-DNEW=1'] + ), ] TEST_MODULES = {m.name: m for m in TEST_MODULES} diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump new file mode 100644 index 000000000..30e1b67b6 --- /dev/null +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump @@ -0,0 +1,15 @@ +{ + "array_types" : [], + "builtin_types" : [], + "elf_functions" : [], + "elf_objects" : [], + "enum_types" : [], + "function_types" : [], + "functions" : [], + "global_vars" : [], + "lvalue_reference_types" : [], + "pointer_types" : [], + "qualified_types" : [], + "record_types" : [], + "rvalue_reference_types" : [] +} diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump new file mode 100644 index 000000000..a63ae1666 --- /dev/null +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump @@ -0,0 +1,37 @@ +{ + "array_types" : [], + "builtin_types" : + [ + { + "linker_set_key" : "void", + "name" : "void", + "referenced_type" : "type-1", + "self_type" : "type-1" + } + ], + "elf_functions" : + [ + { + "binding" : "weak", + "name" : "example" + } + ], + "elf_objects" : [], + "enum_types" : [], + "function_types" : [], + "functions" : + [ + { + "function_name" : "example", + "linker_set_key" : "example", + "return_type" : "type-1", + "source_file" : "/development/vndk/tools/header-checker/tests/integration/weak_symbols/example.c" + } + ], + "global_vars" : [], + "lvalue_reference_types" : [], + "pointer_types" : [], + "qualified_types" : [], + "record_types" : [], + "rvalue_reference_types" : [] +} diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py index 1ed8852dc..57df7d378 100755 --- a/vndk/tools/header-checker/tests/test.py +++ b/vndk/tools/header-checker/tests/test.py @@ -309,6 +309,25 @@ class HeaderCheckerTest(unittest.TestCase): ["-input-format-old", "Json", "-input-format-new", "Json", "-consider-opaque-types-different"]) + def test_allow_adding_removing_weak_symbols(self): + module_old = Module.get_test_modules_by_name("libweak_symbols_old")[0] + module_new = Module.get_test_modules_by_name("libweak_symbols_new")[0] + lsdump_old = self.get_or_create_ref_dump(module_old, False) + lsdump_new = self.get_or_create_ref_dump(module_new, False) + + options = ["-input-format-old", "Json", "-input-format-new", "Json"] + + # If `-allow-adding-removing-weak-symbols` is not specified, removing a + # weak symbol must be treated as an incompatible change. + self.run_and_compare_abi_diff( + lsdump_old, lsdump_new, "libweak_symbols", "arm64", 8, options) + + # If `-allow-adding-removing-weak-symbols` is specified, removing a + # weak symbol must be fine and mustn't be a fatal error. + self.run_and_compare_abi_diff( + lsdump_old, lsdump_new, "libweak_symbols", "arm64", 0, + options + ["-allow-adding-removing-weak-symbols"]) + def test_linker_shared_object_file_and_version_script(self): base_dir = os.path.join( SCRIPT_DIR, 'integration', 'version_script_example')