From b1ca6df79dfb662011b81cf20677708b2a6c59f4 Mon Sep 17 00:00:00 2001 From: Logan Chien Date: Tue, 7 May 2019 16:36:47 -0700 Subject: [PATCH 1/2] header-checker: Add -allow-adding-removing-weak-symbols This commit adds `-allow-adding-removing-weak-symbols` option to `header-abi-diff`. This allows vendor-specific branch to add or remove weak symbols from their VNDK shared libs. This is sometimes necessary when venders have their compiler toolchain or use different compiler flags (e.g. inline heuristics). Bug: 112760591 Test: ./tests/test.py Change-Id: I94bceacc90cab093e8aa602bb7d866d6f433c5c7 --- .../header-checker/src/diff/abi_diff.cpp | 56 +++++++++++++------ vndk/tools/header-checker/src/diff/abi_diff.h | 9 ++- .../src/diff/header_abi_diff.cpp | 15 +++-- .../src/repr/symbol/version_script_parser.cpp | 17 ++++-- .../src/repr/symbol/version_script_parser.h | 1 + .../tests/integration/weak_symbols/example.c | 8 +++ .../weak_symbols/libexample_new.map.txt | 3 + .../weak_symbols/libexample_old.map.txt | 4 ++ vndk/tools/header-checker/tests/module.py | 31 ++++++++++ .../arm64/libweak_symbols_new.so.lsdump | 15 +++++ .../arm64/libweak_symbols_old.so.lsdump | 37 ++++++++++++ vndk/tools/header-checker/tests/test.py | 19 +++++++ 12 files changed, 189 insertions(+), 26 deletions(-) create mode 100644 vndk/tools/header-checker/tests/integration/weak_symbols/example.c create mode 100644 vndk/tools/header-checker/tests/integration/weak_symbols/libexample_new.map.txt create mode 100644 vndk/tools/header-checker/tests/integration/weak_symbols/libexample_old.map.txt create mode 100644 vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_new.so.lsdump create mode 100644 vndk/tools/header-checker/tests/reference_dumps/arm64/libweak_symbols_old.so.lsdump 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..c3c2b7188 100644 --- a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp +++ b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp @@ -121,6 +121,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; @@ -154,11 +161,11 @@ int main(int argc, const char **argv) { 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/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') From 0bcd8e31e38124ddbd47108d21a966cf7a387482 Mon Sep 17 00:00:00 2001 From: Logan Chien Date: Thu, 9 May 2019 12:12:07 -0700 Subject: [PATCH 2/2] header-checker: Add config file support This commit add config file support to header-abi-diff so that we can have per-branch configuration. Bug: 112760591 Test: ./tests/test.py Test: ${ANDROID_HOST_OUT}/nativetest64/header-checker-unittests/\ header-checker-unittests Change-Id: I1fbd76c93991a2b36121a5f4397c62eb72d486ec --- vndk/tools/header-checker/Android.bp | 2 + .../src/diff/header_abi_diff.cpp | 50 ++++- .../header-checker/src/utils/config_file.cpp | 87 ++++++++ .../header-checker/src/utils/config_file.h | 193 ++++++++++++++++++ .../src/utils/config_file_test.cpp | 111 ++++++++++ .../header-checker/src/utils/string_utils.cpp | 9 + .../header-checker/src/utils/string_utils.h | 2 + .../src/utils/string_utils_test.cpp | 14 ++ 8 files changed, 464 insertions(+), 4 deletions(-) create mode 100644 vndk/tools/header-checker/src/utils/config_file.cpp create mode 100644 vndk/tools/header-checker/src/utils/config_file.h create mode 100644 vndk/tools/header-checker/src/utils/config_file_test.cpp 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/header_abi_diff.cpp b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp index c3c2b7188..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"), @@ -142,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"; @@ -157,11 +194,16 @@ 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); + 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, 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"));