Merge "Allow appending members to structs and classes" am: efd3826863

Original change: https://android-review.googlesource.com/c/platform/development/+/2229078

Change-Id: I86f9478f3177fbb4b0c96e7d4f3c6b91aa64f6c7
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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",

vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp

@@ -49,26 +49,6 @@ static void TypeQueueCheckAndPop(std::deque<std::string> *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<const TypeIR *> &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<AccessSpecifierDiffIR>(

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

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) ||
+           (field_diffs_.size() != 0) || (fields_removed_.size() != 0) ||
-        (access_diff_ != nullptr) || (base_specifier_diffs_ != nullptr);
+           (fields_added_.size() != 0) || (access_diff_ != nullptr) ||
+           (base_specifier_diffs_ != nullptr);
   }
 
+  bool IsExtended() const;
+
   const TypeDiffIR *GetTypeDiff() const {
     return type_diff_.get();
   }

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,

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;

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;

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 &);

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 &);

vndk/tools/header-checker/tests/integration/struct_extensions/map.txt

@@ -0,0 +1,4 @@
+libstruct_extensions {
+  global:
+    _Z15PassByReferenceR7Struct1R7Struct2;
+};

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}

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" : []
+}

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" : []
+}

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()