From 959646a857d4993d920660b77afb66f1ca8e521f Mon Sep 17 00:00:00 2001 From: Hsin-Yi Chen Date: Thu, 13 Oct 2022 14:36:44 +0800 Subject: [PATCH] Fix the comparison of opaque types that have multiple definitions The types defined in more than one source file are identified with "#ODR:" and the source paths. The paths can be in intermediate directories and differ between build targets. They cause ABI check failure on opaque types. This commit fixes the bug by removing the suffixes before the comparison. Test: make libcamera2ndk Bug: 253095767 Change-Id: I79e6e843460c981afcf2ce0e0d2ad9335d0b3e90 --- .../src/linker/module_merger.cpp | 6 +- .../src/repr/abi_diff_helpers.cpp | 6 +- .../src/repr/ir_representation.h | 10 + .../merge_multi_definitions/include/def1.h | 6 +- .../merge_multi_definitions/include/def2.h | 7 +- .../include/link_to_def2.h | 1 + .../merge_multi_definitions/map.txt | 1 + vndk/tools/header-checker/tests/module.py | 12 ++ .../arm64/libdiff_multi_definitions.so.lsdump | 174 ++++++++++++++++++ .../libmerge_multi_definitions.so.lsdump | 123 +++++++------ vndk/tools/header-checker/tests/test.py | 5 + 11 files changed, 287 insertions(+), 64 deletions(-) create mode 120000 vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h create mode 100644 vndk/tools/header-checker/tests/reference_dumps/arm64/libdiff_multi_definitions.so.lsdump diff --git a/vndk/tools/header-checker/src/linker/module_merger.cpp b/vndk/tools/header-checker/src/linker/module_merger.cpp index 2ec368bc9..9b5f31d58 100644 --- a/vndk/tools/header-checker/src/linker/module_merger.cpp +++ b/vndk/tools/header-checker/src/linker/module_merger.cpp @@ -206,7 +206,8 @@ ModuleMerger::UpdateUDTypeAccounting( std::string added_type_id = addend_node->GetSelfType(); auto type_id_it = module_->type_graph_.find(added_type_id); if (type_id_it != module_->type_graph_.end()) { - added_type_id = added_type_id + "#ODR:" + addend_compilation_unit_path; + added_type_id = repr::FormatMultiDefinitionTypeId( + added_type_id, addend_compilation_unit_path); } // Add the ud-type with type-id to the type_graph_, since if there are generic @@ -390,7 +391,8 @@ MergeStatus ModuleMerger::MergeReferencingType( addend.GetCompilationUnitPath(final_referenced_type); // The path is empty for built-in types. if (compilation_unit_path != "") { - added_type_id = added_type_id + "#ODR:" + compilation_unit_path; + added_type_id = repr::FormatMultiDefinitionTypeId( + added_type_id, compilation_unit_path); } } } 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 377568c17..a121c3780 100644 --- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp +++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp @@ -264,8 +264,12 @@ bool AbiDiffHelper::CompareVTables( bool AbiDiffHelper::AreOpaqueTypesEqual(const std::string &old_type_id, const std::string &new_type_id) const { + // 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_ || - old_type_id == new_type_id) { + ExtractMultiDefinitionTypeId(old_type_id) == + ExtractMultiDefinitionTypeId(new_type_id)) { return true; } // __va_list is an opaque type defined by the compiler. ARM ABI requires diff --git a/vndk/tools/header-checker/src/repr/ir_representation.h b/vndk/tools/header-checker/src/repr/ir_representation.h index 01b68bd2a..bcc07ba7c 100644 --- a/vndk/tools/header-checker/src/repr/ir_representation.h +++ b/vndk/tools/header-checker/src/repr/ir_representation.h @@ -98,6 +98,16 @@ std::map CreateInverseMap(const std::map &m) { return inverse_map; } +static inline std::string FormatMultiDefinitionTypeId( + const std::string &type_id, const std::string &compilation_unit_path) { + return type_id + "#ODR:" + compilation_unit_path; +} + +static inline std::string_view ExtractMultiDefinitionTypeId( + std::string_view type_id) { + return type_id.substr(0, type_id.find("#ODR:")); +} + class LinkableMessageIR { public: virtual ~LinkableMessageIR() {} diff --git a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h index 54686ba13..c45751c50 100644 --- a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h +++ b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h @@ -6,4 +6,8 @@ struct Struct { struct Opaque; -void func(const struct Struct *, const struct Opaque *); +struct DefinedInOneHeader; + +extern "C" { +void func(Struct *, Opaque *, DefinedInOneHeader *); +} diff --git a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h index 075f78cab..46fd401f6 100644 --- a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h +++ b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h @@ -6,4 +6,9 @@ struct Struct { struct Opaque; -void func(const struct Opaque *, const struct Struct *); +struct DefinedInOneHeader {}; + +extern "C" { +void func(Opaque *, Struct *, DefinedInOneHeader *); +void func2(DefinedInOneHeader *); +} diff --git a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h new file mode 120000 index 000000000..8023898a6 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h @@ -0,0 +1 @@ +./def2.h \ No newline at end of file diff --git a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/map.txt b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/map.txt index 654d2c4bf..f75ae402e 100644 --- a/vndk/tools/header-checker/tests/integration/merge_multi_definitions/map.txt +++ b/vndk/tools/header-checker/tests/integration/merge_multi_definitions/map.txt @@ -1,5 +1,6 @@ libmerge_multi_definitions { global: func; + func2; var; }; diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py index af882ad39..2688a2165 100755 --- a/vndk/tools/header-checker/tests/module.py +++ b/vndk/tools/header-checker/tests/module.py @@ -679,6 +679,18 @@ TEST_MODULES = [ linker_flags=['-output-format', 'Json', '-sources-per-thread', '1'], has_reference_dump=True, ), + LsdumpModule( + name='libdiff_multi_definitions', + arch='arm64', + srcs=[ + 'integration/merge_multi_definitions/include/def1.h', + 'integration/merge_multi_definitions/include/link_to_def2.h', + ], + version_script='integration/merge_multi_definitions/map.txt', + export_include_dirs=['integration/merge_multi_definitions/include'], + linker_flags=['-output-format', 'Json', '-sources-per-thread', '1'], + has_reference_dump=True, + ), LsdumpModule( name='libstruct_extensions', arch='arm64', diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libdiff_multi_definitions.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libdiff_multi_definitions.so.lsdump new file mode 100644 index 000000000..08c944f73 --- /dev/null +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libdiff_multi_definitions.so.lsdump @@ -0,0 +1,174 @@ +{ + "array_types" : [], + "builtin_types" : + [ + { + "alignment" : 1, + "is_integral" : true, + "is_unsigned" : true, + "linker_set_key" : "_ZTIc", + "name" : "char", + "referenced_type" : "_ZTIc", + "self_type" : "_ZTIc", + "size" : 1 + }, + { + "alignment" : 4, + "is_integral" : true, + "linker_set_key" : "_ZTIi", + "name" : "int", + "referenced_type" : "_ZTIi", + "self_type" : "_ZTIi", + "size" : 4 + }, + { + "linker_set_key" : "_ZTIv", + "name" : "void", + "referenced_type" : "_ZTIv", + "self_type" : "_ZTIv" + } + ], + "elf_functions" : + [ + { + "name" : "func" + }, + { + "name" : "func2" + }, + { + "name" : "var" + } + ], + "elf_objects" : [], + "enum_types" : [], + "function_types" : [], + "functions" : + [ + { + "function_name" : "func", + "linker_set_key" : "func", + "parameters" : + [ + { + "referenced_type" : "_ZTIP6Struct" + }, + { + "referenced_type" : "_ZTIP6Opaque" + }, + { + "referenced_type" : "_ZTIP18DefinedInOneHeader" + } + ], + "return_type" : "_ZTIv", + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "function_name" : "func2", + "linker_set_key" : "func2", + "parameters" : + [ + { + "referenced_type" : "_ZTIP18DefinedInOneHeader#ODR:/link_to_def2.h.sdump" + } + ], + "return_type" : "_ZTIv", + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h" + } + ], + "global_vars" : + [ + { + "linker_set_key" : "var", + "name" : "var", + "referenced_type" : "_ZTIc", + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + } + ], + "lvalue_reference_types" : [], + "pointer_types" : + [ + { + "alignment" : 8, + "linker_set_key" : "_ZTIP18DefinedInOneHeader", + "name" : "DefinedInOneHeader *", + "referenced_type" : "_ZTI18DefinedInOneHeader", + "self_type" : "_ZTIP18DefinedInOneHeader", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "alignment" : 8, + "linker_set_key" : "_ZTIP6Opaque", + "name" : "Opaque *", + "referenced_type" : "_ZTI6Opaque", + "self_type" : "_ZTIP6Opaque", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "alignment" : 8, + "linker_set_key" : "_ZTIP6Struct", + "name" : "Struct *", + "referenced_type" : "_ZTI6Struct", + "self_type" : "_ZTIP6Struct", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "alignment" : 8, + "linker_set_key" : "_ZTIP6Struct", + "name" : "Struct *", + "referenced_type" : "_ZTI6Struct#ODR:/link_to_def2.h.sdump", + "self_type" : "_ZTIP6Struct#ODR:/link_to_def2.h.sdump", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h" + } + ], + "qualified_types" : [], + "record_types" : + [ + { + "alignment" : 1, + "linker_set_key" : "_ZTI18DefinedInOneHeader", + "name" : "DefinedInOneHeader", + "referenced_type" : "_ZTI18DefinedInOneHeader", + "self_type" : "_ZTI18DefinedInOneHeader", + "size" : 1, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h" + }, + { + "alignment" : 8, + "fields" : + [ + { + "field_name" : "member1", + "referenced_type" : "_ZTIP6Struct" + } + ], + "linker_set_key" : "_ZTI6Struct", + "name" : "Struct", + "referenced_type" : "_ZTI6Struct", + "self_type" : "_ZTI6Struct", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "alignment" : 8, + "fields" : + [ + { + "field_name" : "member2", + "referenced_type" : "_ZTIP6Struct#ODR:/link_to_def2.h.sdump" + } + ], + "linker_set_key" : "_ZTI6Struct", + "name" : "Struct", + "referenced_type" : "_ZTI6Struct#ODR:/link_to_def2.h.sdump", + "self_type" : "_ZTI6Struct#ODR:/link_to_def2.h.sdump", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/link_to_def2.h" + } + ], + "rvalue_reference_types" : [] +} diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libmerge_multi_definitions.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libmerge_multi_definitions.so.lsdump index ab6a33965..1c425d715 100644 --- a/vndk/tools/header-checker/tests/reference_dumps/arm64/libmerge_multi_definitions.so.lsdump +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libmerge_multi_definitions.so.lsdump @@ -33,6 +33,9 @@ { "name" : "func" }, + { + "name" : "func2" + }, { "name" : "var" } @@ -40,7 +43,39 @@ "elf_objects" : [], "enum_types" : [], "function_types" : [], - "functions" : [], + "functions" : + [ + { + "function_name" : "func", + "linker_set_key" : "func", + "parameters" : + [ + { + "referenced_type" : "_ZTIP6Struct" + }, + { + "referenced_type" : "_ZTIP6Opaque" + }, + { + "referenced_type" : "_ZTIP18DefinedInOneHeader" + } + ], + "return_type" : "_ZTIv", + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "function_name" : "func2", + "linker_set_key" : "func2", + "parameters" : + [ + { + "referenced_type" : "_ZTIP18DefinedInOneHeader#ODR:/def2.h.sdump" + } + ], + "return_type" : "_ZTIv", + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h" + } + ], "global_vars" : [ { @@ -53,6 +88,24 @@ "lvalue_reference_types" : [], "pointer_types" : [ + { + "alignment" : 8, + "linker_set_key" : "_ZTIP18DefinedInOneHeader", + "name" : "DefinedInOneHeader *", + "referenced_type" : "_ZTI18DefinedInOneHeader", + "self_type" : "_ZTIP18DefinedInOneHeader", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, + { + "alignment" : 8, + "linker_set_key" : "_ZTIP6Opaque", + "name" : "Opaque *", + "referenced_type" : "_ZTI6Opaque", + "self_type" : "_ZTIP6Opaque", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" + }, { "alignment" : 8, "linker_set_key" : "_ZTIP6Struct", @@ -70,68 +123,20 @@ "self_type" : "_ZTIP6Struct#ODR:/def2.h.sdump", "size" : 8, "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h" - }, - { - "alignment" : 8, - "linker_set_key" : "_ZTIPK6Opaque", - "name" : "const Opaque *", - "referenced_type" : "_ZTIK6Opaque", - "self_type" : "_ZTIPK6Opaque", - "size" : 8, - "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" - }, - { - "alignment" : 8, - "linker_set_key" : "_ZTIPK6Struct", - "name" : "const Struct *", - "referenced_type" : "_ZTIK6Struct", - "self_type" : "_ZTIPK6Struct", - "size" : 8, - "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" - }, - { - "alignment" : 8, - "linker_set_key" : "_ZTIPK6Struct", - "name" : "const Struct *", - "referenced_type" : "_ZTIK6Struct#ODR:/def2.h.sdump", - "self_type" : "_ZTIPK6Struct#ODR:/def2.h.sdump", - "size" : 8, - "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h" - } - ], - "qualified_types" : - [ - { - "is_const" : true, - "linker_set_key" : "_ZTIK6Opaque", - "name" : "const Opaque", - "referenced_type" : "_ZTI6Opaque", - "self_type" : "_ZTIK6Opaque", - "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" - }, - { - "alignment" : 8, - "is_const" : true, - "linker_set_key" : "_ZTIK6Struct", - "name" : "const Struct", - "referenced_type" : "_ZTI6Struct#ODR:/def2.h.sdump", - "self_type" : "_ZTIK6Struct#ODR:/def2.h.sdump", - "size" : 8, - "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h" - }, - { - "alignment" : 8, - "is_const" : true, - "linker_set_key" : "_ZTIK6Struct", - "name" : "const Struct", - "referenced_type" : "_ZTI6Struct", - "self_type" : "_ZTIK6Struct", - "size" : 8, - "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def1.h" } ], + "qualified_types" : [], "record_types" : [ + { + "alignment" : 1, + "linker_set_key" : "_ZTI18DefinedInOneHeader", + "name" : "DefinedInOneHeader", + "referenced_type" : "_ZTI18DefinedInOneHeader", + "self_type" : "_ZTI18DefinedInOneHeader", + "size" : 1, + "source_file" : "development/vndk/tools/header-checker/tests/integration/merge_multi_definitions/include/def2.h" + }, { "alignment" : 8, "fields" : diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py index e33ff97c4..7c0f43c7f 100755 --- a/vndk/tools/header-checker/tests/test.py +++ b/vndk/tools/header-checker/tests/test.py @@ -387,6 +387,11 @@ class HeaderCheckerTest(unittest.TestCase): def test_merge_multi_definitions(self): self.prepare_and_absolute_diff_all_archs( "libmerge_multi_definitions", "libmerge_multi_definitions") + self.prepare_and_run_abi_diff_all_archs( + "libmerge_multi_definitions", "libdiff_multi_definitions", 0, + flags=["-input-format-new", "Json", "-input-format-old", "Json", + "-consider-opaque-types-different"], + create_old=False, create_new=False) def test_print_resource_dir(self): dumper_path = shutil.which("header-abi-dumper")