Do not exempt deleted functions and global variables
header-abi-diff reports ABI difference for the functions and global
variables whose declarations are deleted, but the symbols are unchanged.
This case happens when a developer modifies the availability attributes
annotated on the declarations.
"-allow-adding-removing-referenced-apis" disables the check. The flag is
needed for C runtime libraries where not all functions are implemented
in C.
Test: ANDROID_BUILD_TOP=`realpath .` \
PATH=$PATH:`realpath out/soong/dist/bin` \
development/vndk/tools/header-checker/tests/test.py
Bug: 342516369
Change-Id: I2aafe37450994f3e8527f7980fb6753d295e9a80
diff --git a/vndk/tools/header-checker/src/diff/abi_diff.cpp b/vndk/tools/header-checker/src/diff/abi_diff.cpp
index cfd559c..9fc86ba 100644
--- a/vndk/tools/header-checker/src/diff/abi_diff.cpp
+++ b/vndk/tools/header-checker/src/diff/abi_diff.cpp
@@ -333,14 +333,12 @@
continue;
}
- // 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.
+ // If this FunctionIR/GlobalVarIR is missing from the new ABI dump but a
+ // corresponding ElfFunctionIR/ElfObjectIR can be found in the new ABI dump,
+ // don't emit error on this element.
const std::string &element_linker_set_key = element->GetLinkerSetKey();
- if (new_elf_map &&
+ if (diff_policy_options_.allow_adding_removing_referenced_apis &&
+ new_elf_map &&
new_elf_map->find(element_linker_set_key) != new_elf_map->end()) {
continue;
}
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 62476ec..c187bed 100644
--- a/vndk/tools/header-checker/src/diff/header_abi_diff.cpp
+++ b/vndk/tools/header-checker/src/diff/header_abi_diff.cpp
@@ -93,6 +93,14 @@
" APIs."),
llvm::cl::Optional, llvm::cl::cat(header_checker_category));
+static llvm::cl::opt<bool> allow_adding_removing_referenced_apis(
+ "allow-adding-removing-referenced-apis",
+ llvm::cl::desc("Do not return a non zero status on addition or removal of "
+ "functions and variables that have corresponding symbols. "
+ "This option ignores the functions built in runtime "
+ "libraries and may not be declared in headers."),
+ llvm::cl::Optional, llvm::cl::cat(header_checker_category));
+
static llvm::cl::opt<bool> consider_opaque_types_different(
"consider-opaque-types-different",
llvm::cl::desc("Consider opaque types with different names as different. "
@@ -185,6 +193,8 @@
allow_unreferenced_elf_symbol_changes = value_bool;
} else if (key == "allow_unreferenced_changes") {
allow_unreferenced_changes = value_bool;
+ } else if (key == "allow_adding_removing_referenced_apis") {
+ allow_adding_removing_referenced_apis = value_bool;
} else if (key == "consider_opaque_types_different") {
consider_opaque_types_different = value_bool;
}
@@ -244,7 +254,10 @@
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);
+ const DiffPolicyOptions diff_policy_options{
+ .consider_opaque_types_different = consider_opaque_types_different,
+ .allow_adding_removing_referenced_apis =
+ allow_adding_removing_referenced_apis};
HeaderAbiDiff judge(lib_name, arch, old_dump, new_dump, compatibility_report,
ignored_symbols, ignored_linker_set_keys_set,
diff --git a/vndk/tools/header-checker/src/linker/module_merger.cpp b/vndk/tools/header-checker/src/linker/module_merger.cpp
index 7cba816..517fbad 100644
--- a/vndk/tools/header-checker/src/linker/module_merger.cpp
+++ b/vndk/tools/header-checker/src/linker/module_merger.cpp
@@ -61,7 +61,9 @@
// Initialize type comparator (which will compare the referenced types
// recursively).
std::set<std::string> type_cache;
- repr::DiffPolicyOptions diff_policy_options(false);
+ const repr::DiffPolicyOptions diff_policy_options{
+ .consider_opaque_types_different = false,
+ .allow_adding_removing_referenced_apis = false};
repr::AbiDiffHelper diff_helper(module_->type_graph_, addend.type_graph_,
diff_policy_options, &type_cache, {}, nullptr);
diff --git a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
index c00d218..a90ea5d 100644
--- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
+++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
@@ -327,7 +327,7 @@
// b/253095767: In T, some dump files contain opaque types whose IDs end with
// "#ODR:" and the source paths. This function removes the suffixes before
// comparing the type IDs.
- if (!diff_policy_options_.consider_opaque_types_different_ ||
+ if (!diff_policy_options_.consider_opaque_types_different ||
ExtractMultiDefinitionTypeId(old_type_id) ==
ExtractMultiDefinitionTypeId(new_type_id)) {
return true;
diff --git a/vndk/tools/header-checker/src/repr/abi_diff_helpers.h b/vndk/tools/header-checker/src/repr/abi_diff_helpers.h
index 5604358..79f6892 100644
--- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.h
+++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.h
@@ -85,10 +85,8 @@
};
struct DiffPolicyOptions {
- DiffPolicyOptions(bool consider_opaque_types_different)
- : consider_opaque_types_different_(consider_opaque_types_different) {}
-
- bool consider_opaque_types_different_;
+ bool consider_opaque_types_different = false;
+ bool allow_adding_removing_referenced_apis = false;
};
class AbiDiffHelper {
diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py
index 6af5433..c7ae008 100755
--- a/vndk/tools/header-checker/tests/test.py
+++ b/vndk/tools/header-checker/tests/test.py
@@ -213,7 +213,11 @@
def test_libgolden_cpp_fabricated_function_ast_removed_diff(self):
self.prepare_and_run_abi_diff_all_archs(
"libgolden_cpp_add_function_sybmol_only",
- "libgolden_cpp_add_function", 0, [], False, False)
+ "libgolden_cpp_add_function", 4, [], False, False)
+ self.prepare_and_run_abi_diff_all_archs(
+ "libgolden_cpp_add_function_sybmol_only",
+ "libgolden_cpp_add_function", 0,
+ ["-allow-adding-removing-referenced-apis"], False, False)
def test_libgolden_cpp_change_function_access(self):
self.prepare_and_run_abi_diff_all_archs(
