Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Commit

Permalink
Merge "Do not exempt deleted functions and global variables" into main
Browse files Browse the repository at this point in the history
  • Loading branch information
hsinyichen-google authored and Gerrit Code Review committed Jan 8, 2025
2 parents b04667b + fd14db5 commit 5aef7e8
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 15 deletions.
12 changes: 5 additions & 7 deletions vndk/tools/header-checker/src/diff/abi_diff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,12 @@ bool HeaderAbiDiff::DumpLoneElements(
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;
}
Expand Down
15 changes: 14 additions & 1 deletion vndk/tools/header-checker/src/diff/header_abi_diff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ static llvm::cl::opt<bool> allow_unreferenced_changes(
" 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. "
Expand Down Expand Up @@ -185,6 +193,8 @@ static void UpdateFlags(const ConfigSection &section) {
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;
}
Expand Down Expand Up @@ -244,7 +254,10 @@ int main(int argc, const char **argv) {
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,
Expand Down
4 changes: 3 additions & 1 deletion vndk/tools/header-checker/src/linker/module_merger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ MergeStatus ModuleMerger::LookupUserDefinedType(
// 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);

Expand Down
2 changes: 1 addition & 1 deletion vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ bool AbiDiffHelper::AreOpaqueTypesEqual(const std::string &old_type_id,
// 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;
Expand Down
6 changes: 2 additions & 4 deletions vndk/tools/header-checker/src/repr/abi_diff_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,8 @@ class TypeStackGuard {
};

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 {
Expand Down
6 changes: 5 additions & 1 deletion vndk/tools/header-checker/tests/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ def test_libgolden_cpp_add_function_and_elf_symbol(self):
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(
Expand Down

0 comments on commit 5aef7e8

Please sign in to comment.