From 5cc255dee690c33fd2d5d0ff6c5c201dcba259b6 Mon Sep 17 00:00:00 2001 From: Hsin-Yi Chen Date: Wed, 5 Oct 2022 17:15:10 +0800 Subject: [PATCH] Fix the comparison between opaque parameters Function parameters can be opaque and have no size information. For example, __va_list in AArch64 ABI. header-abi-diff considers opaque types compatible if their mangled names are the same. The mangled name of __va_list can be _ZTI9__va_list or _ZTISt9__va_list. They are also compatible. Test: ./test.py Bug: 248418092 Change-Id: I812abcabb620301eb575c54c7e3d2ff63dade488 --- .../src/repr/abi_diff_helpers.cpp | 25 ++++++++--- .../src/repr/abi_diff_helpers.h | 3 ++ .../tests/abi_dumps/opaque_ptr_types.lsdump | 29 ------------ .../opaque_type/include/opaque_type.h | 6 +++ .../tests/integration/opaque_type/map.txt | 4 ++ vndk/tools/header-checker/tests/module.py | 9 ++++ .../arm64/libopaque_type.so.lsdump | 45 +++++++++++++++++++ vndk/tools/header-checker/tests/test.py | 9 ++-- 8 files changed, 91 insertions(+), 39 deletions(-) delete mode 100644 vndk/tools/header-checker/tests/abi_dumps/opaque_ptr_types.lsdump create mode 100644 vndk/tools/header-checker/tests/integration/opaque_type/include/opaque_type.h create mode 100644 vndk/tools/header-checker/tests/integration/opaque_type/map.txt create mode 100644 vndk/tools/header-checker/tests/reference_dumps/arm64/libopaque_type.so.lsdump 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 39f418024..377568c17 100644 --- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp +++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp @@ -18,6 +18,8 @@ #include +#include + namespace header_checker { namespace repr { @@ -260,6 +262,20 @@ bool AbiDiffHelper::CompareVTables( return true; } +bool AbiDiffHelper::AreOpaqueTypesEqual(const std::string &old_type_id, + const std::string &new_type_id) const { + if (!diff_policy_options_.consider_opaque_types_different_ || + old_type_id == new_type_id) { + return true; + } + // __va_list is an opaque type defined by the compiler. ARM ABI requires + // __va_list to be in std namespace. Its mangled name is _ZTISt9__va_list, but + // some versions of clang produce _ZTI9__va_list. The names are equivalent. + static const std::unordered_set va_list_names{ + "_ZTI9__va_list", "_ZTISt9__va_list"}; + return va_list_names.count(old_type_id) && va_list_names.count(new_type_id); +} + static bool CompareSizeAndAlignment(const TypeIR *old_type, const TypeIR *new_type) { return old_type->GetSize() == new_type->GetSize() && @@ -274,7 +290,7 @@ bool AbiDiffHelper::AreTypeSizeAndAlignmentEqual( new_types_.find(new_type_id); if (old_it == old_types_.end() || new_it == new_types_.end()) { - return !diff_policy_options_.consider_opaque_types_different_; + return AreOpaqueTypesEqual(old_type_id, new_type_id); } return CompareSizeAndAlignment(old_it->second, new_it->second); @@ -825,10 +841,9 @@ DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff( if (old_it == old_types_.end() || new_it == new_types_.end()) { TypeQueueCheckAndPop(type_queue); // One of the types were hidden, we cannot compare further. - if (diff_policy_options_.consider_opaque_types_different_) { - return DiffStatus::opaque_diff; - } - return DiffStatus::no_diff; + return AreOpaqueTypesEqual(old_type_id, new_type_id) + ? DiffStatus::no_diff + : DiffStatus::opaque_diff; } LinkableMessageKind old_kind = old_it->second->GetKind(); 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 a1673e583..b4d28bdea 100644 --- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.h +++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.h @@ -88,6 +88,9 @@ class AbiDiffHelper { ignored_linker_set_keys_(ignored_linker_set_keys), ir_diff_dumper_(ir_diff_dumper) {} + bool AreOpaqueTypesEqual(const std::string &old_type_str, + const std::string &new_type_str) const; + bool AreTypeSizeAndAlignmentEqual(const std::string &old_type_str, const std::string &new_type_str) const; diff --git a/vndk/tools/header-checker/tests/abi_dumps/opaque_ptr_types.lsdump b/vndk/tools/header-checker/tests/abi_dumps/opaque_ptr_types.lsdump deleted file mode 100644 index 2ecd7c313..000000000 --- a/vndk/tools/header-checker/tests/abi_dumps/opaque_ptr_types.lsdump +++ /dev/null @@ -1,29 +0,0 @@ -{ - "elf_objects" : - [ - { - "name" : "test" - } - ], - "global_vars" : - [ - { - "linker_set_key" : "test", - "name" : "test", - "referenced_type" : "type-1", - "source_file" : "opaque_ptr_types.h" - } - ], - "pointer_types" : - [ - { - "alignment" : 8, - "linker_set_key" : "OpaqueType *", - "name" : "OpaqueType *", - "referenced_type" : "type-2", - "self_type" : "type-1", - "size" : 8, - "source_file" : "opaque_ptr_types.h" - } - ] -} diff --git a/vndk/tools/header-checker/tests/integration/opaque_type/include/opaque_type.h b/vndk/tools/header-checker/tests/integration/opaque_type/include/opaque_type.h new file mode 100644 index 000000000..3e4ccf9d2 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/opaque_type/include/opaque_type.h @@ -0,0 +1,6 @@ +#include + +extern "C" { +struct OpaqueType; +OpaqueType *function(va_list); +} diff --git a/vndk/tools/header-checker/tests/integration/opaque_type/map.txt b/vndk/tools/header-checker/tests/integration/opaque_type/map.txt new file mode 100644 index 000000000..8c7ac1ac2 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/opaque_type/map.txt @@ -0,0 +1,4 @@ +libopaque_type { + global: + function; +}; \ No newline at end of file diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py index 2824f7cb4..af882ad39 100755 --- a/vndk/tools/header-checker/tests/module.py +++ b/vndk/tools/header-checker/tests/module.py @@ -518,6 +518,15 @@ TEST_MODULES = [ linker_flags=['-input-format', 'Json', '-output-format', 'Json'], has_reference_dump=True, ), + LsdumpModule( + name='libopaque_type', + arch='arm64', + srcs=['integration/opaque_type/include/opaque_type.h'], + version_script='integration/opaque_type/map.txt', + export_include_dirs=['integration/opaque_type/include'], + linker_flags=['-output-format', 'Json'], + has_reference_dump=True, + ), LsdumpModule( name='libversion_script_example', arch='arm64', diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libopaque_type.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libopaque_type.so.lsdump new file mode 100644 index 000000000..f54831115 --- /dev/null +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libopaque_type.so.lsdump @@ -0,0 +1,45 @@ +{ + "array_types" : [], + "builtin_types" : [], + "elf_functions" : + [ + { + "name" : "function" + } + ], + "elf_objects" : [], + "enum_types" : [], + "function_types" : [], + "functions" : + [ + { + "function_name" : "function", + "linker_set_key" : "function", + "parameters" : + [ + { + "referenced_type" : "_ZTISt9__va_list" + } + ], + "return_type" : "_ZTIP10OpaqueType", + "source_file" : "development/vndk/tools/header-checker/tests/integration/opaque_type/include/opaque_type.h" + } + ], + "global_vars" : [], + "lvalue_reference_types" : [], + "pointer_types" : + [ + { + "alignment" : 8, + "linker_set_key" : "_ZTIP10OpaqueType", + "name" : "OpaqueType *", + "referenced_type" : "_ZTI10OpaqueType", + "self_type" : "_ZTIP10OpaqueType", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/opaque_type/include/opaque_type.h" + } + ], + "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 e103fc3ee..e33ff97c4 100755 --- a/vndk/tools/header-checker/tests/test.py +++ b/vndk/tools/header-checker/tests/test.py @@ -311,12 +311,11 @@ class HeaderCheckerTest(unittest.TestCase): "-input-format-new", "Json"]) def test_opaque_type_self_diff(self): - lsdump = os.path.join( - SCRIPT_DIR, "abi_dumps", "opaque_ptr_types.lsdump") - self.run_and_compare_abi_diff( - lsdump, lsdump, "libexample", "arm64", 0, + self.prepare_and_run_abi_diff_all_archs( + "libopaque_type", "libopaque_type", 0, ["-input-format-old", "Json", "-input-format-new", "Json", - "-consider-opaque-types-different"]) + "-consider-opaque-types-different"], + create_old=False, create_new=False) def test_allow_adding_removing_weak_symbols(self): module_old = Module.get_test_modules_by_name("libweak_symbols_old")[0]