Merge "Do not allow extending pass-by-value parameters or return types" am: fc212f2a34 am: 2b3d297c0b am: c0f3567f03

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

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

vndk/tools/header-checker/src/diff/abi_diff_wrappers.cpp

@@ -90,18 +90,14 @@ bool DiffWrapper<repr::FunctionIR>::DumpDiff(
   std::deque<std::string> type_queue;
   type_queue.push_back(oldp_->GetName());
 
-  DiffStatus param_diffs = CompareFunctionParameters(
+  DiffStatus function_type_diff =
-      oldp_->GetParameters(), newp_->GetParameters(), &type_queue, diff_kind);
+      CompareFunctionTypes(oldp_, newp_, &type_queue, diff_kind);
-
-  DiffStatus return_type_diff = CompareAndDumpTypeDiff(
-      oldp_->GetReturnType(), newp_->GetReturnType(), &type_queue, diff_kind);
 
   CompareTemplateInfo(oldp_->GetTemplateElements(),
                       newp_->GetTemplateElements(),
                       &type_queue, diff_kind);
 
-  if ((param_diffs == DiffStatus::direct_diff ||
+  if ((function_type_diff == DiffStatus::direct_diff) ||
-       return_type_diff == DiffStatus::direct_diff) ||
       (oldp_->GetAccess() != newp_->GetAccess())) {
     repr::FunctionIR old_function = *oldp_;
     repr::FunctionIR new_function = *newp_;

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

@@ -260,13 +260,26 @@ bool AbiDiffHelper::CompareVTables(
   return true;
 }
 
-bool AbiDiffHelper::CompareSizeAndAlignment(
+static bool CompareSizeAndAlignment(const TypeIR *old_type,
-    const TypeIR *old_type,
+                                    const TypeIR *new_type) {
-    const TypeIR *new_type) {
   return old_type->GetSize() == new_type->GetSize() &&
       old_type->GetAlignment() == new_type->GetAlignment();
 }
 
+bool AbiDiffHelper::AreTypeSizeAndAlignmentEqual(
+    const std::string &old_type_id, const std::string &new_type_id) const {
+  AbiElementMap<const TypeIR *>::const_iterator old_it =
+      old_types_.find(old_type_id);
+  AbiElementMap<const TypeIR *>::const_iterator new_it =
+      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 CompareSizeAndAlignment(old_it->second, new_it->second);
+}
+
 DiffStatusPair<std::unique_ptr<RecordFieldDiffIR>>
 AbiDiffHelper::CompareCommonRecordFields(
     const RecordFieldIR *old_field,
@@ -496,17 +509,14 @@ AbiDiffHelper::FixupDiffedFieldTypeIds(
 }
 
 DiffStatus AbiDiffHelper::CompareFunctionTypes(
-    const FunctionTypeIR *old_type,
+    const CFunctionLikeIR *old_type, const CFunctionLikeIR *new_type,
-    const FunctionTypeIR *new_type,
+    std::deque<std::string> *type_queue, DiffMessageIR::DiffKind diff_kind) {
-    std::deque<std::string> *type_queue,
-    DiffMessageIR::DiffKind diff_kind) {
   DiffStatus param_diffs = CompareFunctionParameters(old_type->GetParameters(),
                                                      new_type->GetParameters(),
                                                      type_queue, diff_kind);
-  DiffStatus return_type_diff =
+  DiffStatus return_type_diff = CompareParameterOrReturnType(
-      CompareAndDumpTypeDiff(old_type->GetReturnType(),
+      old_type->GetReturnType(), new_type->GetReturnType(), type_queue,
-                             new_type->GetReturnType(),
+      diff_kind);
-                             type_queue, diff_kind);
 
   if (param_diffs == DiffStatus::direct_diff ||
       return_type_diff == DiffStatus::direct_diff) {
@@ -698,10 +708,10 @@ DiffStatus AbiDiffHelper::CompareFunctionParameters(
   while (i < old_parameters_size) {
     const ParamIR &old_parameter = old_parameters.at(i);
     const ParamIR &new_parameter = new_parameters.at(i);
-    if ((CompareAndDumpTypeDiff(old_parameter.GetReferencedType(),
+    if (CompareParameterOrReturnType(old_parameter.GetReferencedType(),
-                               new_parameter.GetReferencedType(),
+                                     new_parameter.GetReferencedType(),
-                               type_queue, diff_kind) ==
+                                     type_queue,
-        DiffStatus::direct_diff) ||
+                                     diff_kind) == DiffStatus::direct_diff ||
         (old_parameter.GetIsDefault() != new_parameter.GetIsDefault())) {
       return DiffStatus::direct_diff;
     }
@@ -710,6 +720,16 @@ DiffStatus AbiDiffHelper::CompareFunctionParameters(
   return DiffStatus::no_diff;
 }
 
+DiffStatus AbiDiffHelper::CompareParameterOrReturnType(
+    const std::string &old_type_id, const std::string &new_type_id,
+    std::deque<std::string> *type_queue, DiffMessageIR::DiffKind diff_kind) {
+  if (!AreTypeSizeAndAlignmentEqual(old_type_id, new_type_id)) {
+    return DiffStatus::direct_diff;
+  }
+  return CompareAndDumpTypeDiff(old_type_id, new_type_id, type_queue,
+                                diff_kind);
+}
+
 DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
     const TypeIR *old_type, const TypeIR *new_type,
     LinkableMessageKind kind, std::deque<std::string> *type_queue,

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 AreTypeSizeAndAlignmentEqual(const std::string &old_type_str,
+                                    const std::string &new_type_str) const;
+
   DiffStatus CompareAndDumpTypeDiff(
       const std::string &old_type_str, const std::string &new_type_str,
       std::deque<std::string> *type_queue = nullptr,
@@ -110,8 +113,8 @@ class AbiDiffHelper {
                               std::deque<std::string> *type_queue,
                               IRDiffDumper::DiffKind diff_kind);
 
-  DiffStatus CompareFunctionTypes(const FunctionTypeIR *old_type,
+  DiffStatus CompareFunctionTypes(const CFunctionLikeIR *old_type,
-                                  const FunctionTypeIR *new_type,
+                                  const CFunctionLikeIR *new_type,
                                   std::deque<std::string> *type_queue,
                                   DiffMessageIR::DiffKind diff_kind);
 
@@ -121,6 +124,11 @@ class AbiDiffHelper {
       std::deque<std::string> *type_queue,
       IRDiffDumper::DiffKind diff_kind);
 
+  DiffStatus CompareParameterOrReturnType(const std::string &old_type_id,
+                                          const std::string &new_type_id,
+                                          std::deque<std::string> *type_queue,
+                                          IRDiffDumper::DiffKind diff_kind);
+
   DiffStatus CompareTemplateInfo(
       const std::vector<TemplateElementIR> &old_template_elements,
       const std::vector<TemplateElementIR> &new_template_elements,
@@ -198,9 +206,6 @@ class AbiDiffHelper {
       const VTableComponentIR &old_component,
       const VTableComponentIR &new_component);
 
-  bool CompareSizeAndAlignment(const TypeIR *old_ti,
-                               const TypeIR *new_ti);
-
   template <typename DiffType, typename DiffElement>
   bool AddToDiff(DiffType *mutable_diff, const DiffElement *oldp,
                  const DiffElement *newp,

vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h

@@ -0,0 +1,10 @@
+struct Parameter {
+  int member;
+};
+
+struct Return {
+  int member;
+};
+
+void PassByValue(Parameter);
+Return ReturnByValue();

vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h

@@ -0,0 +1,11 @@
+struct Parameter {
+  int member;
+  int extra_member;
+};
+
+struct Return {
+  int member;
+};
+
+void PassByValue(Parameter);
+Return ReturnByValue();

vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h

@@ -0,0 +1,11 @@
+struct Parameter {
+  int member;
+};
+
+struct Return {
+  int member;
+  int extra_member;
+};
+
+void PassByValue(Parameter);
+Return ReturnByValue();

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

@@ -0,0 +1,5 @@
+libpass_by_value {
+  global:
+    _Z11PassByValue9Parameter;
+    _Z13ReturnByValuev;
+};

vndk/tools/header-checker/tests/module.py

@@ -688,6 +688,33 @@ TEST_MODULES = [
         linker_flags=['-output-format', 'Json'],
         has_reference_dump=True,
     ),
+    LsdumpModule(
+        name='libpass_by_value',
+        arch='arm64',
+        srcs=['integration/pass_by_value/include/base.h'],
+        version_script='integration/pass_by_value/map.txt',
+        export_include_dirs=['integration/pass_by_value/include'],
+        linker_flags=['-output-format', 'Json'],
+        has_reference_dump=True,
+    ),
+    LsdumpModule(
+        name='libparam_size_diff',
+        arch='arm64',
+        srcs=['integration/pass_by_value/include/param_size_diff.h'],
+        version_script='integration/pass_by_value/map.txt',
+        export_include_dirs=['integration/pass_by_value/include'],
+        linker_flags=['-output-format', 'Json'],
+        has_reference_dump=True,
+    ),
+    LsdumpModule(
+        name='libreturn_size_diff',
+        arch='arm64',
+        srcs=['integration/pass_by_value/include/return_size_diff.h'],
+        version_script='integration/pass_by_value/map.txt',
+        export_include_dirs=['integration/pass_by_value/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/libparam_size_diff.so.lsdump

@@ -0,0 +1,99 @@
+{
+ "array_types" : [],
+ "builtin_types" :
+ [
+  {
+   "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" : "_Z11PassByValue9Parameter"
+  },
+  {
+   "name" : "_Z13ReturnByValuev"
+  }
+ ],
+ "elf_objects" : [],
+ "enum_types" : [],
+ "function_types" : [],
+ "functions" :
+ [
+  {
+   "function_name" : "PassByValue",
+   "linker_set_key" : "_Z11PassByValue9Parameter",
+   "parameters" :
+   [
+    {
+     "referenced_type" : "_ZTI9Parameter"
+    }
+   ],
+   "return_type" : "_ZTIv",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
+  },
+  {
+   "function_name" : "ReturnByValue",
+   "linker_set_key" : "_Z13ReturnByValuev",
+   "return_type" : "_ZTI6Return",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
+  }
+ ],
+ "global_vars" : [],
+ "lvalue_reference_types" : [],
+ "pointer_types" : [],
+ "qualified_types" : [],
+ "record_types" :
+ [
+  {
+   "alignment" : 4,
+   "fields" :
+   [
+    {
+     "field_name" : "member",
+     "referenced_type" : "_ZTIi"
+    }
+   ],
+   "linker_set_key" : "_ZTI6Return",
+   "name" : "Return",
+   "referenced_type" : "_ZTI6Return",
+   "self_type" : "_ZTI6Return",
+   "size" : 4,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
+  },
+  {
+   "alignment" : 4,
+   "fields" :
+   [
+    {
+     "field_name" : "member",
+     "referenced_type" : "_ZTIi"
+    },
+    {
+     "field_name" : "extra_member",
+     "field_offset" : 32,
+     "referenced_type" : "_ZTIi"
+    }
+   ],
+   "linker_set_key" : "_ZTI9Parameter",
+   "name" : "Parameter",
+   "referenced_type" : "_ZTI9Parameter",
+   "self_type" : "_ZTI9Parameter",
+   "size" : 8,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
+  }
+ ],
+ "rvalue_reference_types" : []
+}

vndk/tools/header-checker/tests/reference_dumps/arm64/libpass_by_value.so.lsdump

@@ -0,0 +1,94 @@
+{
+ "array_types" : [],
+ "builtin_types" :
+ [
+  {
+   "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" : "_Z11PassByValue9Parameter"
+  },
+  {
+   "name" : "_Z13ReturnByValuev"
+  }
+ ],
+ "elf_objects" : [],
+ "enum_types" : [],
+ "function_types" : [],
+ "functions" :
+ [
+  {
+   "function_name" : "PassByValue",
+   "linker_set_key" : "_Z11PassByValue9Parameter",
+   "parameters" :
+   [
+    {
+     "referenced_type" : "_ZTI9Parameter"
+    }
+   ],
+   "return_type" : "_ZTIv",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
+  },
+  {
+   "function_name" : "ReturnByValue",
+   "linker_set_key" : "_Z13ReturnByValuev",
+   "return_type" : "_ZTI6Return",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
+  }
+ ],
+ "global_vars" : [],
+ "lvalue_reference_types" : [],
+ "pointer_types" : [],
+ "qualified_types" : [],
+ "record_types" :
+ [
+  {
+   "alignment" : 4,
+   "fields" :
+   [
+    {
+     "field_name" : "member",
+     "referenced_type" : "_ZTIi"
+    }
+   ],
+   "linker_set_key" : "_ZTI6Return",
+   "name" : "Return",
+   "referenced_type" : "_ZTI6Return",
+   "self_type" : "_ZTI6Return",
+   "size" : 4,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
+  },
+  {
+   "alignment" : 4,
+   "fields" :
+   [
+    {
+     "field_name" : "member",
+     "referenced_type" : "_ZTIi"
+    }
+   ],
+   "linker_set_key" : "_ZTI9Parameter",
+   "name" : "Parameter",
+   "referenced_type" : "_ZTI9Parameter",
+   "self_type" : "_ZTI9Parameter",
+   "size" : 4,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
+  }
+ ],
+ "rvalue_reference_types" : []
+}

vndk/tools/header-checker/tests/reference_dumps/arm64/libreturn_size_diff.so.lsdump

@@ -0,0 +1,99 @@
+{
+ "array_types" : [],
+ "builtin_types" :
+ [
+  {
+   "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" : "_Z11PassByValue9Parameter"
+  },
+  {
+   "name" : "_Z13ReturnByValuev"
+  }
+ ],
+ "elf_objects" : [],
+ "enum_types" : [],
+ "function_types" : [],
+ "functions" :
+ [
+  {
+   "function_name" : "PassByValue",
+   "linker_set_key" : "_Z11PassByValue9Parameter",
+   "parameters" :
+   [
+    {
+     "referenced_type" : "_ZTI9Parameter"
+    }
+   ],
+   "return_type" : "_ZTIv",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
+  },
+  {
+   "function_name" : "ReturnByValue",
+   "linker_set_key" : "_Z13ReturnByValuev",
+   "return_type" : "_ZTI6Return",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
+  }
+ ],
+ "global_vars" : [],
+ "lvalue_reference_types" : [],
+ "pointer_types" : [],
+ "qualified_types" : [],
+ "record_types" :
+ [
+  {
+   "alignment" : 4,
+   "fields" :
+   [
+    {
+     "field_name" : "member",
+     "referenced_type" : "_ZTIi"
+    },
+    {
+     "field_name" : "extra_member",
+     "field_offset" : 32,
+     "referenced_type" : "_ZTIi"
+    }
+   ],
+   "linker_set_key" : "_ZTI6Return",
+   "name" : "Return",
+   "referenced_type" : "_ZTI6Return",
+   "self_type" : "_ZTI6Return",
+   "size" : 8,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
+  },
+  {
+   "alignment" : 4,
+   "fields" :
+   [
+    {
+     "field_name" : "member",
+     "referenced_type" : "_ZTIi"
+    }
+   ],
+   "linker_set_key" : "_ZTI9Parameter",
+   "name" : "Parameter",
+   "referenced_type" : "_ZTI9Parameter",
+   "self_type" : "_ZTI9Parameter",
+   "size" : 4,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
+  }
+ ],
+ "rvalue_reference_types" : []
+}

vndk/tools/header-checker/tests/test.py

@@ -407,6 +407,18 @@ class HeaderCheckerTest(unittest.TestCase):
             flags=["-input-format-new", "Json", "-input-format-old", "Json"],
             create_old=False, create_new=False)
 
+    def test_param_size_diff(self):
+        self.prepare_and_run_abi_diff_all_archs(
+            "libpass_by_value", "libparam_size_diff", 8,
+            flags=["-input-format-new", "Json", "-input-format-old", "Json"],
+            create_old=False, create_new=False)
+
+    def test_return_size_diff(self):
+        self.prepare_and_run_abi_diff_all_archs(
+            "libpass_by_value", "libreturn_size_diff", 8,
+            flags=["-input-format-new", "Json", "-input-format-old", "Json"],
+            create_old=False, create_new=False)
+
 
 if __name__ == '__main__':
     unittest.main()