diff --git a/vndk/tools/header-checker/Android.bp b/vndk/tools/header-checker/Android.bp index e5b7e1d8b..251f6ae99 100644 --- a/vndk/tools/header-checker/Android.bp +++ b/vndk/tools/header-checker/Android.bp @@ -143,6 +143,7 @@ cc_library_host_static { srcs: [ "src/repr/abi_diff_helpers.cpp", "src/repr/ir_diff_dumper.cpp", + "src/repr/ir_diff_representation.cpp", "src/repr/ir_dumper.cpp", "src/repr/ir_reader.cpp", "src/repr/ir_representation.cpp", 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 9e4c578e6..2f5c9f9e5 100644 --- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp +++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp @@ -49,26 +49,6 @@ static void TypeQueueCheckAndPop(std::deque *type_queue) { } } -static bool IsAccessDownGraded(AccessSpecifierIR old_access, - AccessSpecifierIR new_access) { - bool access_downgraded = false; - switch (old_access) { - case AccessSpecifierIR::ProtectedAccess: - if (new_access == AccessSpecifierIR::PrivateAccess) { - access_downgraded = true; - } - break; - case AccessSpecifierIR::PublicAccess: - if (new_access != AccessSpecifierIR::PublicAccess) { - access_downgraded = true; - } - break; - default: - break; - } - return access_downgraded; -} - static std::string ConvertTypeIdToString( const AbiElementMap &type_graph, const std::string &type_id) { @@ -302,7 +282,7 @@ AbiDiffHelper::CompareCommonRecordFields( if (old_field->GetOffset() != new_field->GetOffset() || // TODO: Should this be an inquality check instead ? Some compilers can // make signatures dependant on absolute values of access specifiers. - IsAccessDownGraded(old_field->GetAccess(), new_field->GetAccess()) || + IsAccessDowngraded(old_field->GetAccess(), new_field->GetAccess()) || (field_diff_status == DiffStatus::direct_diff)) { return std::make_pair( DiffStatus::direct_diff, @@ -557,7 +537,7 @@ DiffStatus AbiDiffHelper::CompareRecordTypes( DiffStatus final_diff_status = DiffStatus::no_diff; record_type_diff_ir->SetName(old_type->GetName()); record_type_diff_ir->SetLinkerSetKey(old_type->GetLinkerSetKey()); - if (IsAccessDownGraded(old_type->GetAccess(), new_type->GetAccess())) { + if (IsAccessDowngraded(old_type->GetAccess(), new_type->GetAccess())) { final_diff_status = DiffStatus::indirect_diff; record_type_diff_ir->SetAccessDiff( std::make_unique( diff --git a/vndk/tools/header-checker/src/repr/ir_diff_representation.cpp b/vndk/tools/header-checker/src/repr/ir_diff_representation.cpp new file mode 100644 index 000000000..32f6f3066 --- /dev/null +++ b/vndk/tools/header-checker/src/repr/ir_diff_representation.cpp @@ -0,0 +1,71 @@ +// Copyright (C) 2022 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 "repr/ir_diff_representation.h" + +namespace header_checker { +namespace repr { + +bool RecordTypeDiffIR::IsExtended() const { + bool is_extended = false; + if (type_diff_ != nullptr) { + auto sizes = type_diff_->GetSizes(); + if (sizes.first < sizes.second) { + is_extended = true; + } + if (sizes.first > sizes.second) { + return false; + } + auto alignments = type_diff_->GetAlignments(); + if (alignments.first != alignments.second) { + return false; + } + } + if (access_diff_ != nullptr) { + if (IsAccessDowngraded(access_diff_->GetOldAccess(), + access_diff_->GetNewAccess())) { + return false; + } + is_extended = true; + } + if (base_specifier_diffs_ != nullptr) { + return false; + } + if (vtable_diffs_ != nullptr) { + // TODO(b/248418092): Compare vtables. + return false; + } + // This function skips comparing the access specifiers of field_diffs_ + // because AbiDiffHelper::CompareCommonRecordFields does not report + // upgraded access specifiers as ABI difference. + if (field_diffs_.size() != 0 || fields_removed_.size() != 0) { + return false; + } + if (fields_added_.size() != 0) { + if (type_diff_ != nullptr) { + const uint64_t old_size = type_diff_->GetSizes().first; + for (const RecordFieldIR *field_added : fields_added_) { + // The offset is in bits; the size is in bytes. + if (field_added->GetOffset() < old_size * 8) { + return false; + } + } + } + is_extended = true; + } + return is_extended; +} + +} // namespace repr +} // namespace header_checker diff --git a/vndk/tools/header-checker/src/repr/ir_diff_representation.h b/vndk/tools/header-checker/src/repr/ir_diff_representation.h index 85c903ec9..d4d4b74eb 100644 --- a/vndk/tools/header-checker/src/repr/ir_diff_representation.h +++ b/vndk/tools/header-checker/src/repr/ir_diff_representation.h @@ -31,7 +31,6 @@ namespace repr { class DiffMessageIR { public: enum DiffKind { - Extension, // Applicable for enums. Added, Removed, Referenced, @@ -61,6 +60,10 @@ class AccessSpecifierDiffIR { AccessSpecifierIR new_access) : old_access_(old_access), new_access_(new_access) {} + AccessSpecifierIR GetOldAccess() const { return old_access_; } + + AccessSpecifierIR GetNewAccess() const { return new_access_; } + protected: AccessSpecifierIR old_access_; AccessSpecifierIR new_access_; @@ -196,10 +199,13 @@ class RecordTypeDiffIR : public DiffMessageIR { bool DiffExists() const { return (type_diff_ != nullptr) || (vtable_diffs_ != nullptr) || - (fields_removed_.size() != 0) || (field_diffs_.size() != 0) || - (access_diff_ != nullptr) || (base_specifier_diffs_ != nullptr); + (field_diffs_.size() != 0) || (fields_removed_.size() != 0) || + (fields_added_.size() != 0) || (access_diff_ != nullptr) || + (base_specifier_diffs_ != nullptr); } + bool IsExtended() const; + const TypeDiffIR *GetTypeDiff() const { return type_diff_.get(); } diff --git a/vndk/tools/header-checker/src/repr/ir_representation.h b/vndk/tools/header-checker/src/repr/ir_representation.h index f74d3b7a7..01b68bd2a 100644 --- a/vndk/tools/header-checker/src/repr/ir_representation.h +++ b/vndk/tools/header-checker/src/repr/ir_representation.h @@ -70,6 +70,11 @@ enum AccessSpecifierIR { PrivateAccess = 3 }; +static inline bool IsAccessDowngraded(AccessSpecifierIR old_access, + AccessSpecifierIR new_access) { + return old_access < new_access; +} + enum LinkableMessageKind { RecordTypeKind, EnumTypeKind, diff --git a/vndk/tools/header-checker/src/repr/protobuf/ir_diff_dumper.cpp b/vndk/tools/header-checker/src/repr/protobuf/ir_diff_dumper.cpp index 12d7dedea..a0eb957d2 100644 --- a/vndk/tools/header-checker/src/repr/protobuf/ir_diff_dumper.cpp +++ b/vndk/tools/header-checker/src/repr/protobuf/ir_diff_dumper.cpp @@ -54,17 +54,19 @@ CompatibilityStatusIR ProtobufIRDiffDumper::GetCompatibilityStatusIR() { if (diff_tu_->enum_type_extension_diffs().size() != 0 || diff_tu_->functions_added().size() != 0 || - diff_tu_->global_vars_added().size() != 0) { + diff_tu_->global_vars_added().size() != 0 || + diff_tu_->record_type_extension_diffs().size() != 0) { combined_status = combined_status | CompatibilityStatusIR::Extension; } if (diff_tu_->unreferenced_enum_type_diffs().size() != 0 || - diff_tu_->unreferenced_enum_types_removed().size() != 0 || - diff_tu_->unreferenced_record_types_removed().size() != 0 || - diff_tu_->unreferenced_record_type_diffs().size() != 0 || diff_tu_->unreferenced_enum_type_extension_diffs().size() != 0 || + diff_tu_->unreferenced_enum_types_added().size() != 0 || + diff_tu_->unreferenced_enum_types_removed().size() != 0 || + diff_tu_->unreferenced_record_type_diffs().size() != 0 || + diff_tu_->unreferenced_record_type_extension_diffs().size() != 0 || diff_tu_->unreferenced_record_types_added().size() != 0 || - diff_tu_->unreferenced_enum_types_added().size()) { + diff_tu_->unreferenced_record_types_removed().size() != 0) { combined_status = combined_status | CompatibilityStatusIR::UnreferencedChanges; } @@ -279,12 +281,22 @@ bool ProtobufIRDiffDumper::AddRecordTypeDiffIR( const RecordTypeDiffIR *record_diff_ir, const std::string &type_stack, DiffKind diff_kind) { abi_diff::RecordTypeDiff *added_record_type_diff = nullptr; + bool is_extended = record_diff_ir->IsExtended(); switch (diff_kind) { case DiffKind::Unreferenced: - added_record_type_diff = diff_tu_->add_unreferenced_record_type_diffs(); + if (is_extended) { + added_record_type_diff = + diff_tu_->add_unreferenced_record_type_extension_diffs(); + } else { + added_record_type_diff = diff_tu_->add_unreferenced_record_type_diffs(); + } break; case DiffKind::Referenced: - added_record_type_diff = diff_tu_->add_record_type_diffs(); + if (is_extended) { + added_record_type_diff = diff_tu_->add_record_type_extension_diffs(); + } else { + added_record_type_diff = diff_tu_->add_record_type_diffs(); + } break; default: break; diff --git a/vndk/tools/header-checker/src/repr/protobuf/proto/abi_diff.proto b/vndk/tools/header-checker/src/repr/protobuf/proto/abi_diff.proto index 90a1096e4..384fccbe4 100644 --- a/vndk/tools/header-checker/src/repr/protobuf/proto/abi_diff.proto +++ b/vndk/tools/header-checker/src/repr/protobuf/proto/abi_diff.proto @@ -87,6 +87,8 @@ message TranslationUnitDiff { repeated RecordTypeDiff unreferenced_record_type_diffs = 4; repeated abi_dump.RecordType unreferenced_record_types_removed = 5; repeated abi_dump.RecordType unreferenced_record_types_added = 6; + repeated RecordTypeDiff record_type_extension_diffs = 24; + repeated RecordTypeDiff unreferenced_record_type_extension_diffs = 25; // Enums repeated EnumTypeDiff enum_type_diffs = 7; diff --git a/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h b/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h new file mode 100644 index 000000000..94f801a95 --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h @@ -0,0 +1,13 @@ +struct Struct1 { + protected: + int member; +}; + +struct Struct2 { + protected: + union Nested { + int nested_member; + } member; +}; + +Struct1 &PassByReference(Struct1 &, Struct2 &); diff --git a/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h b/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h new file mode 100644 index 000000000..683bd861e --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h @@ -0,0 +1,15 @@ +struct Struct1 { + public: + int member; + int added_member; +}; + +struct Struct2 { + public: + union Nested { + int nested_member; + int added_member[2]; + } member; +}; + +Struct1 &PassByReference(Struct1 &, Struct2 &); diff --git a/vndk/tools/header-checker/tests/integration/struct_extensions/map.txt b/vndk/tools/header-checker/tests/integration/struct_extensions/map.txt new file mode 100644 index 000000000..d3bbac59f --- /dev/null +++ b/vndk/tools/header-checker/tests/integration/struct_extensions/map.txt @@ -0,0 +1,4 @@ +libstruct_extensions { + global: + _Z15PassByReferenceR7Struct1R7Struct2; +}; diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py index 94160dc66..1849612ad 100755 --- a/vndk/tools/header-checker/tests/module.py +++ b/vndk/tools/header-checker/tests/module.py @@ -670,6 +670,24 @@ TEST_MODULES = [ linker_flags=['-output-format', 'Json', '-sources-per-thread', '1'], has_reference_dump=True, ), + LsdumpModule( + name='libstruct_extensions', + arch='arm64', + srcs=['integration/struct_extensions/include/base.h'], + version_script='integration/struct_extensions/map.txt', + export_include_dirs=['integration/struct_extensions/include'], + linker_flags=['-output-format', 'Json'], + has_reference_dump=True, + ), + LsdumpModule( + name='liballowed_struct_extensions', + arch='arm64', + srcs=['integration/struct_extensions/include/extensions.h'], + version_script='integration/struct_extensions/map.txt', + export_include_dirs=['integration/struct_extensions/include'], + linker_flags=['-output-format', 'Json'], + has_reference_dump=True, + ), ] TEST_MODULES = {m.name: m for m in TEST_MODULES} diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/liballowed_struct_extensions.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/liballowed_struct_extensions.so.lsdump new file mode 100644 index 000000000..4e33d2e20 --- /dev/null +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/liballowed_struct_extensions.so.lsdump @@ -0,0 +1,139 @@ +{ + "array_types" : + [ + { + "alignment" : 4, + "linker_set_key" : "_ZTIA2_i", + "name" : "int[2]", + "referenced_type" : "_ZTIi", + "self_type" : "_ZTIA2_i", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + } + ], + "builtin_types" : + [ + { + "alignment" : 4, + "is_integral" : true, + "linker_set_key" : "_ZTIi", + "name" : "int", + "referenced_type" : "_ZTIi", + "self_type" : "_ZTIi", + "size" : 4 + } + ], + "elf_functions" : + [ + { + "name" : "_Z15PassByReferenceR7Struct1R7Struct2" + } + ], + "elf_objects" : [], + "enum_types" : [], + "function_types" : [], + "functions" : + [ + { + "function_name" : "PassByReference", + "linker_set_key" : "_Z15PassByReferenceR7Struct1R7Struct2", + "parameters" : + [ + { + "referenced_type" : "_ZTIR7Struct1" + }, + { + "referenced_type" : "_ZTIR7Struct2" + } + ], + "return_type" : "_ZTIR7Struct1", + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + } + ], + "global_vars" : [], + "lvalue_reference_types" : + [ + { + "alignment" : 8, + "linker_set_key" : "_ZTIR7Struct1", + "name" : "Struct1 &", + "referenced_type" : "_ZTI7Struct1", + "self_type" : "_ZTIR7Struct1", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + }, + { + "alignment" : 8, + "linker_set_key" : "_ZTIR7Struct2", + "name" : "Struct2 &", + "referenced_type" : "_ZTI7Struct2", + "self_type" : "_ZTIR7Struct2", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + } + ], + "pointer_types" : [], + "qualified_types" : [], + "record_types" : + [ + { + "alignment" : 4, + "fields" : + [ + { + "field_name" : "member", + "referenced_type" : "_ZTIi" + }, + { + "field_name" : "added_member", + "field_offset" : 32, + "referenced_type" : "_ZTIi" + } + ], + "linker_set_key" : "_ZTI7Struct1", + "name" : "Struct1", + "referenced_type" : "_ZTI7Struct1", + "self_type" : "_ZTI7Struct1", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + }, + { + "alignment" : 4, + "fields" : + [ + { + "field_name" : "member", + "referenced_type" : "_ZTIN7Struct26NestedE" + } + ], + "linker_set_key" : "_ZTI7Struct2", + "name" : "Struct2", + "referenced_type" : "_ZTI7Struct2", + "self_type" : "_ZTI7Struct2", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + }, + { + "alignment" : 4, + "fields" : + [ + { + "field_name" : "nested_member", + "referenced_type" : "_ZTIi" + }, + { + "field_name" : "added_member", + "referenced_type" : "_ZTIA2_i" + } + ], + "linker_set_key" : "_ZTIN7Struct26NestedE", + "name" : "Struct2::Nested", + "record_kind" : "union", + "referenced_type" : "_ZTIN7Struct26NestedE", + "self_type" : "_ZTIN7Struct26NestedE", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/extensions.h" + } + ], + "rvalue_reference_types" : [] +} diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libstruct_extensions.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libstruct_extensions.so.lsdump new file mode 100644 index 000000000..0d0cc70b9 --- /dev/null +++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libstruct_extensions.so.lsdump @@ -0,0 +1,122 @@ +{ + "array_types" : [], + "builtin_types" : + [ + { + "alignment" : 4, + "is_integral" : true, + "linker_set_key" : "_ZTIi", + "name" : "int", + "referenced_type" : "_ZTIi", + "self_type" : "_ZTIi", + "size" : 4 + } + ], + "elf_functions" : + [ + { + "name" : "_Z15PassByReferenceR7Struct1R7Struct2" + } + ], + "elf_objects" : [], + "enum_types" : [], + "function_types" : [], + "functions" : + [ + { + "function_name" : "PassByReference", + "linker_set_key" : "_Z15PassByReferenceR7Struct1R7Struct2", + "parameters" : + [ + { + "referenced_type" : "_ZTIR7Struct1" + }, + { + "referenced_type" : "_ZTIR7Struct2" + } + ], + "return_type" : "_ZTIR7Struct1", + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h" + } + ], + "global_vars" : [], + "lvalue_reference_types" : + [ + { + "alignment" : 8, + "linker_set_key" : "_ZTIR7Struct1", + "name" : "Struct1 &", + "referenced_type" : "_ZTI7Struct1", + "self_type" : "_ZTIR7Struct1", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h" + }, + { + "alignment" : 8, + "linker_set_key" : "_ZTIR7Struct2", + "name" : "Struct2 &", + "referenced_type" : "_ZTI7Struct2", + "self_type" : "_ZTIR7Struct2", + "size" : 8, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h" + } + ], + "pointer_types" : [], + "qualified_types" : [], + "record_types" : + [ + { + "alignment" : 4, + "fields" : + [ + { + "access" : "protected", + "field_name" : "member", + "referenced_type" : "_ZTIi" + } + ], + "linker_set_key" : "_ZTI7Struct1", + "name" : "Struct1", + "referenced_type" : "_ZTI7Struct1", + "self_type" : "_ZTI7Struct1", + "size" : 4, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h" + }, + { + "alignment" : 4, + "fields" : + [ + { + "access" : "protected", + "field_name" : "member", + "referenced_type" : "_ZTIN7Struct26NestedE" + } + ], + "linker_set_key" : "_ZTI7Struct2", + "name" : "Struct2", + "referenced_type" : "_ZTI7Struct2", + "self_type" : "_ZTI7Struct2", + "size" : 4, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h" + }, + { + "access" : "protected", + "alignment" : 4, + "fields" : + [ + { + "field_name" : "nested_member", + "referenced_type" : "_ZTIi" + } + ], + "linker_set_key" : "_ZTIN7Struct26NestedE", + "name" : "Struct2::Nested", + "record_kind" : "union", + "referenced_type" : "_ZTIN7Struct26NestedE", + "self_type" : "_ZTIN7Struct26NestedE", + "size" : 4, + "source_file" : "development/vndk/tools/header-checker/tests/integration/struct_extensions/include/base.h" + } + ], + "rvalue_reference_types" : [] +} diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py index 69fdec59b..849e36e11 100755 --- a/vndk/tools/header-checker/tests/test.py +++ b/vndk/tools/header-checker/tests/test.py @@ -401,6 +401,12 @@ class HeaderCheckerTest(unittest.TestCase): os.path.join(common_dir, "lib64", "clang")) self.assertRegex(os.path.basename(resource_dir), r"^[\d.]+$") + def test_struct_extensions(self): + self.prepare_and_run_abi_diff_all_archs( + "libstruct_extensions", "liballowed_struct_extensions", 4, + flags=["-input-format-new", "Json", "-input-format-old", "Json"], + create_old=False, create_new=False) + if __name__ == '__main__': unittest.main()