Fix readFromParcel in every backend am: 9e85bce7f9 am: e498eb9808

Original change: https://android-review.googlesource.com/c/platform/system/tools/aidl/+/1688277

Change-Id: Ibd10efd1edd44ee6cfd8ab002a1435f7d2272ea0

29 files changed, 191 insertions, 77 deletions

diff --git a/Android.bp b/Android.bp
index 2d0eb1db..38ad98c9 100644
--- a/Android.bp
+++ b/Android.bp
@@ -382,6 +382,8 @@ cc_test {
     static_libs: [
         "libcutils",
         "libgmock",
+        //  client uses the latest version
+        "aidl-test-versioned-interface-V2-ndk_platform",
         "aidl-test-interface-ndk_platform",
         "aidl_test_loggable_interface-ndk_platform"
     ],
@@ -390,6 +392,7 @@ cc_test {
     ],
     srcs: [
         "tests/aidl_test_client_ndk_loggable_interface.cpp",
+        "tests/aidl_test_client_ndk_versioned_interface.cpp",
     ],
 }
 
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index f2e6bf5a..58f87e21 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -1124,21 +1124,22 @@ void BuildReadFromParcel(const AidlStructuredParcelable& parcel, const AidlTypen
       "static_cast<size_t>(_aidl_parcelable_raw_size);\n"
       "if (_aidl_start_pos > SIZE_MAX - _aidl_parcelable_size) return ::android::BAD_VALUE;\n");
 
+  auto checkAvailableData = StringPrintf(
+      "if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {\n"
+      "  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);\n"
+      "  return %s;\n"
+      "}",
+      kAndroidStatusVarName);
   for (const auto& variable : parcel.GetFields()) {
+    read_block->AddLiteral(checkAvailableData);
     string method = ParcelReadMethodOf(variable->GetType(), typenames);
-
     read_block->AddStatement(new Assignment(
         kAndroidStatusVarName, new MethodCall(StringPrintf("_aidl_parcel->%s", method.c_str()),
                                               ParcelReadCastOf(variable->GetType(), typenames,
                                                                "&" + variable->GetName()))));
     read_block->AddStatement(ReturnOnStatusNotOk());
-    read_block->AddLiteral(StringPrintf(
-        "if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {\n"
-        "  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);\n"
-        "  return %s;\n"
-        "}",
-        kAndroidStatusVarName));
   }
+  read_block->AddLiteral("_aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size)");
   read_block->AddLiteral(StringPrintf("return %s", kAndroidStatusVarName));
 }
 
diff --git a/generate_java.cpp b/generate_java.cpp
index 17756c91..f3b7a937 100644
--- a/generate_java.cpp
+++ b/generate_java.cpp
@@ -581,11 +581,12 @@ std::unique_ptr<android::aidl::java::Class> generate_parcel_class(
   }
   out << ";\n";
 
-  std::shared_ptr<LiteralStatement> sizeCheck = nullptr;
+  std::shared_ptr<LiteralStatement> sizeCheck = std::make_shared<LiteralStatement>(out.str());
   // keep this across different fields in order to create the classloader
   // at most once.
   bool is_classloader_created = false;
   for (const auto& field : parcel->GetFields()) {
+    read_or_create_method->statements->Add(sizeCheck);
     const auto field_variable_name =
         (parcel->IsJavaOnlyImmutable() ? "_aidl_temp_" : "") + field->GetName();
     string code;
@@ -610,8 +611,6 @@ std::unique_ptr<android::aidl::java::Class> generate_parcel_class(
     }
     writer->Close();
     read_or_create_method->statements->Add(std::make_shared<LiteralStatement>(code));
-    if (!sizeCheck) sizeCheck = std::make_shared<LiteralStatement>(out.str());
-    read_or_create_method->statements->Add(sizeCheck);
   }
 
   out.str("");
diff --git a/generate_ndk.cpp b/generate_ndk.cpp
index fec00b81..89d65cb7 100644
--- a/generate_ndk.cpp
+++ b/generate_ndk.cpp
@@ -1102,14 +1102,14 @@ void GenerateParcelSource(CodeWriter& out, const AidlTypenames& types,
   StatusCheckReturn(out);
 
   for (const auto& variable : defined_type.GetFields()) {
-    out << "_aidl_ret_status = ";
-    ReadFromParcelFor({out, types, variable->GetType(), "parcel", "&" + variable->GetName()});
-    out << ";\n";
-    StatusCheckReturn(out);
     out << "if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {\n"
         << "  AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);\n"
         << "  return _aidl_ret_status;\n"
         << "}\n";
+    out << "_aidl_ret_status = ";
+    ReadFromParcelFor({out, types, variable->GetType(), "parcel", "&" + variable->GetName()});
+    out << ";\n";
+    StatusCheckReturn(out);
   }
   out << "AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);\n"
       << "return _aidl_ret_status;\n";
diff --git a/generate_rust.cpp b/generate_rust.cpp
index 0b36ff60..3d8be91e 100644
--- a/generate_rust.cpp
+++ b/generate_rust.cpp
@@ -562,25 +562,29 @@ void GenerateParcelDeserializeBody(CodeWriter& out, const AidlStructuredParcelab
   out << "let parcelable_size: i32 = parcel.read()?;\n";
   out << "if parcelable_size < 0 { return Err(binder::StatusCode::BAD_VALUE); }\n";
 
-  // Pre-emit the common field epilogue code, shared between all fields:
-  ostringstream epilogue;
-  epilogue << "if (parcel.get_data_position() - start_pos) == parcelable_size {\n";
+  // Pre-emit the common field prolog code, shared between all fields:
+  ostringstream prolog;
+  prolog << "if (parcel.get_data_position() - start_pos) == parcelable_size {\n";
   // We assume the lhs can never be > parcelable_size, because then the read
   // immediately preceding this check would have returned NOT_ENOUGH_DATA
-  epilogue << "  return Ok(Some(result));\n";
-  epilogue << "}\n";
-  string epilogue_str = epilogue.str();
+  prolog << "  return Ok(Some(result));\n";
+  prolog << "}\n";
+  string prolog_str = prolog.str();
 
   out << "let mut result = Self::default();\n";
   for (const auto& variable : parcel->GetFields()) {
+    out << prolog_str;
     if (!TypeHasDefault(variable->GetType(), typenames)) {
       out << "result." << variable->GetName() << " = Some(parcel.read()?);\n";
     } else {
       out << "result." << variable->GetName() << " = parcel.read()?;\n";
     }
-    out << epilogue_str;
   }
-
+  // Now we read all fields.
+  // Skip remaining data in case we're reading from a newer version
+  out << "unsafe {\n";
+  out << "  parcel.set_data_position(start_pos + parcelable_size)?;\n";
+  out << "}\n";
   out << "Ok(Some(result))\n";
 }
 
diff --git a/tests/aidl_test_client_ndk_versioned_interface.cpp b/tests/aidl_test_client_ndk_versioned_interface.cpp
new file mode 100644
index 00000000..2e3f3ba5
--- /dev/null
+++ b/tests/aidl_test_client_ndk_versioned_interface.cpp
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2021 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 <aidl/android/aidl/versioned/tests/IFooInterface.h>
+
+#include <android/binder_auto_utils.h>
+#include <android/binder_manager.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+using aidl::android::aidl::versioned::tests::BazUnion;
+using aidl::android::aidl::versioned::tests::Foo;
+using aidl::android::aidl::versioned::tests::IFooInterface;
+using std::optional;
+using std::pair;
+using std::shared_ptr;
+using std::string;
+using std::vector;
+using testing::Eq;
+
+struct VersionedInterfaceTest : ::testing::Test {
+  void SetUp() override {
+    ndk::SpAIBinder binder = ndk::SpAIBinder(AServiceManager_getService(IFooInterface::descriptor));
+    versioned = IFooInterface::fromBinder(binder);
+    ASSERT_NE(nullptr, versioned);
+  }
+  shared_ptr<IFooInterface> versioned;
+};
+
+TEST_F(VersionedInterfaceTest, getInterfaceVersion) {
+  int32_t version;
+  auto status = versioned->getInterfaceVersion(&version);
+  EXPECT_TRUE(status.isOk()) << status.getDescription();
+  EXPECT_EQ(1, version);
+}
+
+TEST_F(VersionedInterfaceTest, getInterfaceHash) {
+  string hash;
+  auto status = versioned->getInterfaceHash(&hash);
+  EXPECT_TRUE(status.isOk()) << status.getDescription();
+  EXPECT_EQ("4b32bf2134c87894404e935d52c5c64886f23215", hash);
+}
+
+TEST_F(VersionedInterfaceTest, parcelableParamContainsNewField) {
+  Foo outFoo;
+  auto status = versioned->callWithFoo(&outFoo);
+  EXPECT_TRUE(status.isOk()) << status.getDescription();
+  EXPECT_EQ(42, outFoo.intDefault42);
+}
\ No newline at end of file
diff --git a/tests/aidl_test_client_versioned_interface.cpp b/tests/aidl_test_client_versioned_interface.cpp
index 07ac29e7..9574aff0 100644
--- a/tests/aidl_test_client_versioned_interface.cpp
+++ b/tests/aidl_test_client_versioned_interface.cpp
@@ -25,6 +25,7 @@ using android::OK;
 using android::sp;
 using android::String16;
 using android::aidl::versioned::tests::BazUnion;
+using android::aidl::versioned::tests::Foo;
 using android::aidl::versioned::tests::IFooInterface;
 
 class VersionedInterfaceTest : public AidlTest {
@@ -67,3 +68,10 @@ TEST_F(VersionedInterfaceTest, errorWhenPassingAUnionWithNewField) {
     EXPECT_EQ(::android::BAD_VALUE, status.transactionError()) << status;
   }
 }
+
+TEST_F(VersionedInterfaceTest, parcelableParamContainsNewField) {
+  Foo outFoo;
+  auto status = versioned->callWithFoo(&outFoo);
+  EXPECT_TRUE(status.isOk());
+  EXPECT_EQ(42, outFoo.intDefault42);
+}
\ No newline at end of file
diff --git a/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/OtherParcelableForToString.cpp b/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/OtherParcelableForToString.cpp
index f80a21db..5ee87ace 100644
--- a/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/OtherParcelableForToString.cpp
+++ b/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/OtherParcelableForToString.cpp
@@ -14,14 +14,15 @@ namespace tests {
   [[maybe_unused]] size_t _aidl_parcelable_size = static_cast<size_t>(_aidl_parcelable_raw_size);
   if (_aidl_start_pos > SIZE_MAX - _aidl_parcelable_size) return ::android::BAD_VALUE;
   ;
-  _aidl_ret_status = _aidl_parcel->readString16(&field);
-  if (((_aidl_ret_status) != (::android::OK))) {
-    return _aidl_ret_status;
-  }
   if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
     _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
     return _aidl_ret_status;
   };
+  _aidl_ret_status = _aidl_parcel->readString16(&field);
+  if (((_aidl_ret_status) != (::android::OK))) {
+    return _aidl_ret_status;
+  }
+  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
 
diff --git a/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/ParcelableForToString.cpp b/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/ParcelableForToString.cpp
index 03c85a84..4882b7df 100644
--- a/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/ParcelableForToString.cpp
+++ b/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/ParcelableForToString.cpp
@@ -14,6 +14,10 @@ namespace tests {
   [[maybe_unused]] size_t _aidl_parcelable_size = static_cast<size_t>(_aidl_parcelable_raw_size);
   if (_aidl_start_pos > SIZE_MAX - _aidl_parcelable_size) return ::android::BAD_VALUE;
   ;
+  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
+    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  };
   _aidl_ret_status = _aidl_parcel->readInt32(&intValue);
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
@@ -194,10 +198,7 @@ namespace tests {
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
   }
-  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
-    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  };
+  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
 
diff --git a/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/StructuredParcelable.cpp b/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/StructuredParcelable.cpp
index f9a6cce7..c73c02da 100644
--- a/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/StructuredParcelable.cpp
+++ b/tests/golden_output/aidl-test-interface-cpp-source/gen/android/aidl/tests/StructuredParcelable.cpp
@@ -14,6 +14,10 @@ namespace tests {
   [[maybe_unused]] size_t _aidl_parcelable_size = static_cast<size_t>(_aidl_parcelable_raw_size);
   if (_aidl_start_pos > SIZE_MAX - _aidl_parcelable_size) return ::android::BAD_VALUE;
   ;
+  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
+    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  };
   _aidl_ret_status = _aidl_parcel->readInt32Vector(&shouldContainThreeFs);
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
@@ -434,10 +438,7 @@ namespace tests {
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
   }
-  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
-    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  };
+  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
 
diff --git a/tests/golden_output/aidl-test-interface-cpp-source/gen/include/android/aidl/tests/GenericStructuredParcelable.h b/tests/golden_output/aidl-test-interface-cpp-source/gen/include/android/aidl/tests/GenericStructuredParcelable.h
index 0208235e..f64aba32 100644
--- a/tests/golden_output/aidl-test-interface-cpp-source/gen/include/android/aidl/tests/GenericStructuredParcelable.h
+++ b/tests/golden_output/aidl-test-interface-cpp-source/gen/include/android/aidl/tests/GenericStructuredParcelable.h
@@ -75,6 +75,10 @@ template <typename T, typename U, typename B>
   [[maybe_unused]] size_t _aidl_parcelable_size = static_cast<size_t>(_aidl_parcelable_raw_size);
   if (_aidl_start_pos > SIZE_MAX - _aidl_parcelable_size) return ::android::BAD_VALUE;
   ;
+  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
+    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  };
   _aidl_ret_status = _aidl_parcel->readInt32(&a);
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
@@ -87,10 +91,7 @@ template <typename T, typename U, typename B>
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
   }
-  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
-    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  };
+  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
 
diff --git a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/GenericStructuredParcelable.java b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/GenericStructuredParcelable.java
index dbcd69d3..94b6a2e3 100644
--- a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/GenericStructuredParcelable.java
+++ b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/GenericStructuredParcelable.java
@@ -35,10 +35,10 @@ public class GenericStructuredParcelable<T,U,B> implements android.os.Parcelable
     int _aidl_parcelable_size = _aidl_parcel.readInt();
     try {
       if (_aidl_parcelable_size < 0) return;
+      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       a = _aidl_parcel.readInt();
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       b = _aidl_parcel.readInt();
-      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
     } finally {
       if (_aidl_start_pos > (Integer.MAX_VALUE - _aidl_parcelable_size)) {
         throw new android.os.BadParcelableException("Overflow in the size of parcelable");
diff --git a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/OtherParcelableForToString.java b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/OtherParcelableForToString.java
index e0ea0751..a2824eeb 100644
--- a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/OtherParcelableForToString.java
+++ b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/OtherParcelableForToString.java
@@ -33,8 +33,8 @@ public class OtherParcelableForToString implements android.os.Parcelable
     int _aidl_parcelable_size = _aidl_parcel.readInt();
     try {
       if (_aidl_parcelable_size < 0) return;
-      field = _aidl_parcel.readString();
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
+      field = _aidl_parcel.readString();
     } finally {
       if (_aidl_start_pos > (Integer.MAX_VALUE - _aidl_parcelable_size)) {
         throw new android.os.BadParcelableException("Overflow in the size of parcelable");
diff --git a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/ParcelableForToString.java b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/ParcelableForToString.java
index 8223a330..9c1bb13e 100644
--- a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/ParcelableForToString.java
+++ b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/ParcelableForToString.java
@@ -95,6 +95,7 @@ public class ParcelableForToString implements android.os.Parcelable
     int _aidl_parcelable_size = _aidl_parcel.readInt();
     try {
       if (_aidl_parcelable_size < 0) return;
+      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       intValue = _aidl_parcel.readInt();
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       intArray = _aidl_parcel.createIntArray();
@@ -155,7 +156,6 @@ public class ParcelableForToString implements android.os.Parcelable
       else {
         unionValue = null;
       }
-      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
     } finally {
       if (_aidl_start_pos > (Integer.MAX_VALUE - _aidl_parcelable_size)) {
         throw new android.os.BadParcelableException("Overflow in the size of parcelable");
diff --git a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/StructuredParcelable.java b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/StructuredParcelable.java
index 0359161f..806a634a 100644
--- a/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/StructuredParcelable.java
+++ b/tests/golden_output/aidl-test-interface-java-source/gen/android/aidl/tests/StructuredParcelable.java
@@ -153,6 +153,7 @@ public class StructuredParcelable implements android.os.Parcelable
     int _aidl_parcelable_size = _aidl_parcel.readInt();
     try {
       if (_aidl_parcelable_size < 0) return;
+      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       shouldContainThreeFs = _aidl_parcel.createIntArray();
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       f = _aidl_parcel.readInt();
@@ -268,7 +269,6 @@ public class StructuredParcelable implements android.os.Parcelable
       }
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       defaultWithFoo = _aidl_parcel.readInt();
-      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
     } finally {
       if (_aidl_start_pos > (Integer.MAX_VALUE - _aidl_parcelable_size)) {
         throw new android.os.BadParcelableException("Overflow in the size of parcelable");
diff --git a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/OtherParcelableForToString.cpp b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/OtherParcelableForToString.cpp
index 857c86e3..17322d83 100644
--- a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/OtherParcelableForToString.cpp
+++ b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/OtherParcelableForToString.cpp
@@ -16,13 +16,13 @@ binder_status_t OtherParcelableForToString::readFromParcel(const AParcel* parcel
   if (_aidl_parcelable_size < 0) return STATUS_BAD_VALUE;
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
-  _aidl_ret_status = ::ndk::AParcel_readString(parcel, &field);
-  if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
-
   if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
     AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
     return _aidl_ret_status;
   }
+  _aidl_ret_status = ::ndk::AParcel_readString(parcel, &field);
+  if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
+
   AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
diff --git a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/ParcelableForToString.cpp b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/ParcelableForToString.cpp
index 3a3cd5ef..a5ad0a70 100644
--- a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/ParcelableForToString.cpp
+++ b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/ParcelableForToString.cpp
@@ -16,6 +16,10 @@ binder_status_t ParcelableForToString::readFromParcel(const AParcel* parcel) {
   if (_aidl_parcelable_size < 0) return STATUS_BAD_VALUE;
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
+  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
+    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  }
   _aidl_ret_status = AParcel_readInt32(parcel, &intValue);
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
@@ -173,10 +177,6 @@ binder_status_t ParcelableForToString::readFromParcel(const AParcel* parcel) {
   _aidl_ret_status = ::ndk::AParcel_readParcelable(parcel, &unionValue);
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
-  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
-    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  }
   AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
diff --git a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/StructuredParcelable.cpp b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/StructuredParcelable.cpp
index 740f16a1..2d7588a9 100644
--- a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/StructuredParcelable.cpp
+++ b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/android/aidl/tests/StructuredParcelable.cpp
@@ -16,6 +16,10 @@ binder_status_t StructuredParcelable::readFromParcel(const AParcel* parcel) {
   if (_aidl_parcelable_size < 0) return STATUS_BAD_VALUE;
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
+  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
+    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  }
   _aidl_ret_status = ::ndk::AParcel_readVector(parcel, &shouldContainThreeFs);
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
@@ -383,10 +387,6 @@ binder_status_t StructuredParcelable::readFromParcel(const AParcel* parcel) {
   _aidl_ret_status = AParcel_readInt32(parcel, reinterpret_cast<int32_t*>(&defaultWithFoo));
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
-  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
-    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  }
   AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
diff --git a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/include/aidl/android/aidl/tests/GenericStructuredParcelable.h b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/include/aidl/android/aidl/tests/GenericStructuredParcelable.h
index 7e6e7140..113aff49 100644
--- a/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/include/aidl/android/aidl/tests/GenericStructuredParcelable.h
+++ b/tests/golden_output/aidl-test-interface-ndk_platform-source/gen/include/aidl/android/aidl/tests/GenericStructuredParcelable.h
@@ -79,20 +79,20 @@ binder_status_t GenericStructuredParcelable<T, U, B>::readFromParcel(const AParc
   if (_aidl_parcelable_size < 0) return STATUS_BAD_VALUE;
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
-  _aidl_ret_status = AParcel_readInt32(parcel, &a);
-  if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
-
   if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
     AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
     return _aidl_ret_status;
   }
-  _aidl_ret_status = AParcel_readInt32(parcel, &b);
+  _aidl_ret_status = AParcel_readInt32(parcel, &a);
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
   if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
     AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
     return _aidl_ret_status;
   }
+  _aidl_ret_status = AParcel_readInt32(parcel, &b);
+  if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
+
   AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
diff --git a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/GenericStructuredParcelable.rs b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/GenericStructuredParcelable.rs
index b98ba2e2..ba2c74a1 100644
--- a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/GenericStructuredParcelable.rs
+++ b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/GenericStructuredParcelable.rs
@@ -49,14 +49,17 @@ impl binder::parcel::DeserializeOption for GenericStructuredParcelable {
     let parcelable_size: i32 = parcel.read()?;
     if parcelable_size < 0 { return Err(binder::StatusCode::BAD_VALUE); }
     let mut result = Self::default();
-    result.a = parcel.read()?;
     if (parcel.get_data_position() - start_pos) == parcelable_size {
       return Ok(Some(result));
     }
-    result.b = parcel.read()?;
+    result.a = parcel.read()?;
     if (parcel.get_data_position() - start_pos) == parcelable_size {
       return Ok(Some(result));
     }
+    result.b = parcel.read()?;
+    unsafe {
+      parcel.set_data_position(start_pos + parcelable_size)?;
+    }
     Ok(Some(result))
   }
 }
diff --git a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/OtherParcelableForToString.rs b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/OtherParcelableForToString.rs
index d6e90165..960a06f7 100644
--- a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/OtherParcelableForToString.rs
+++ b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/OtherParcelableForToString.rs
@@ -46,10 +46,13 @@ impl binder::parcel::DeserializeOption for OtherParcelableForToString {
     let parcelable_size: i32 = parcel.read()?;
     if parcelable_size < 0 { return Err(binder::StatusCode::BAD_VALUE); }
     let mut result = Self::default();
-    result.field = parcel.read()?;
     if (parcel.get_data_position() - start_pos) == parcelable_size {
       return Ok(Some(result));
     }
+    result.field = parcel.read()?;
+    unsafe {
+      parcel.set_data_position(start_pos + parcelable_size)?;
+    }
     Ok(Some(result))
   }
 }
diff --git a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/ParcelableForToString.rs b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/ParcelableForToString.rs
index de43b70b..959802c1 100644
--- a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/ParcelableForToString.rs
+++ b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/ParcelableForToString.rs
@@ -112,6 +112,9 @@ impl binder::parcel::DeserializeOption for ParcelableForToString {
     let parcelable_size: i32 = parcel.read()?;
     if parcelable_size < 0 { return Err(binder::StatusCode::BAD_VALUE); }
     let mut result = Self::default();
+    if (parcel.get_data_position() - start_pos) == parcelable_size {
+      return Ok(Some(result));
+    }
     result.intValue = parcel.read()?;
     if (parcel.get_data_position() - start_pos) == parcelable_size {
       return Ok(Some(result));
@@ -201,8 +204,8 @@ impl binder::parcel::DeserializeOption for ParcelableForToString {
       return Ok(Some(result));
     }
     result.unionValue = parcel.read()?;
-    if (parcel.get_data_position() - start_pos) == parcelable_size {
-      return Ok(Some(result));
+    unsafe {
+      parcel.set_data_position(start_pos + parcelable_size)?;
     }
     Ok(Some(result))
   }
diff --git a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs
index f46149d9..52a1c3d5 100644
--- a/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs
+++ b/tests/golden_output/aidl-test-interface-rust-source/gen/android/aidl/tests/StructuredParcelable.rs
@@ -205,6 +205,9 @@ impl binder::parcel::DeserializeOption for StructuredParcelable {
     let parcelable_size: i32 = parcel.read()?;
     if parcelable_size < 0 { return Err(binder::StatusCode::BAD_VALUE); }
     let mut result = Self::default();
+    if (parcel.get_data_position() - start_pos) == parcelable_size {
+      return Ok(Some(result));
+    }
     result.shouldContainThreeFs = parcel.read()?;
     if (parcel.get_data_position() - start_pos) == parcelable_size {
       return Ok(Some(result));
@@ -414,8 +417,8 @@ impl binder::parcel::DeserializeOption for StructuredParcelable {
       return Ok(Some(result));
     }
     result.defaultWithFoo = parcel.read()?;
-    if (parcel.get_data_position() - start_pos) == parcelable_size {
-      return Ok(Some(result));
+    unsafe {
+      parcel.set_data_position(start_pos + parcelable_size)?;
     }
     Ok(Some(result))
   }
diff --git a/tests/golden_output/aidl_test_loggable_interface-cpp-source/gen/android/aidl/loggable/Data.cpp b/tests/golden_output/aidl_test_loggable_interface-cpp-source/gen/android/aidl/loggable/Data.cpp
index b6c8f303..789aac0e 100644
--- a/tests/golden_output/aidl_test_loggable_interface-cpp-source/gen/android/aidl/loggable/Data.cpp
+++ b/tests/golden_output/aidl_test_loggable_interface-cpp-source/gen/android/aidl/loggable/Data.cpp
@@ -14,6 +14,10 @@ namespace loggable {
   [[maybe_unused]] size_t _aidl_parcelable_size = static_cast<size_t>(_aidl_parcelable_raw_size);
   if (_aidl_start_pos > SIZE_MAX - _aidl_parcelable_size) return ::android::BAD_VALUE;
   ;
+  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
+    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  };
   _aidl_ret_status = _aidl_parcel->readInt32(&num);
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
@@ -42,10 +46,7 @@ namespace loggable {
   if (((_aidl_ret_status) != (::android::OK))) {
     return _aidl_ret_status;
   }
-  if (_aidl_parcel->dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) {
-    _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  };
+  _aidl_parcel->setDataPosition(_aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
 
diff --git a/tests/golden_output/aidl_test_loggable_interface-java-source/gen/android/aidl/loggable/Data.java b/tests/golden_output/aidl_test_loggable_interface-java-source/gen/android/aidl/loggable/Data.java
index cc4645f1..4e1ebfdd 100644
--- a/tests/golden_output/aidl_test_loggable_interface-java-source/gen/android/aidl/loggable/Data.java
+++ b/tests/golden_output/aidl_test_loggable_interface-java-source/gen/android/aidl/loggable/Data.java
@@ -45,6 +45,7 @@ public class Data implements android.os.Parcelable
     int _aidl_parcelable_size = _aidl_parcel.readInt();
     try {
       if (_aidl_parcelable_size < 0) return;
+      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       num = _aidl_parcel.readInt();
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       str = _aidl_parcel.readString();
@@ -57,7 +58,6 @@ public class Data implements android.os.Parcelable
       }
       if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
       nestedEnum = _aidl_parcel.readByte();
-      if (_aidl_parcel.dataPosition() - _aidl_start_pos >= _aidl_parcelable_size) return;
     } finally {
       if (_aidl_start_pos > (Integer.MAX_VALUE - _aidl_parcelable_size)) {
         throw new android.os.BadParcelableException("Overflow in the size of parcelable");
diff --git a/tests/golden_output/aidl_test_loggable_interface-ndk-source/gen/android/aidl/loggable/Data.cpp b/tests/golden_output/aidl_test_loggable_interface-ndk-source/gen/android/aidl/loggable/Data.cpp
index fb304964..29ba3c3d 100644
--- a/tests/golden_output/aidl_test_loggable_interface-ndk-source/gen/android/aidl/loggable/Data.cpp
+++ b/tests/golden_output/aidl_test_loggable_interface-ndk-source/gen/android/aidl/loggable/Data.cpp
@@ -16,6 +16,10 @@ binder_status_t Data::readFromParcel(const AParcel* parcel) {
   if (_aidl_parcelable_size < 0) return STATUS_BAD_VALUE;
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
+  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
+    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  }
   _aidl_ret_status = AParcel_readInt32(parcel, &num);
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
@@ -40,10 +44,6 @@ binder_status_t Data::readFromParcel(const AParcel* parcel) {
   _aidl_ret_status = AParcel_readByte(parcel, reinterpret_cast<int8_t*>(&nestedEnum));
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
-  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
-    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  }
   AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
diff --git a/tests/golden_output/aidl_test_loggable_interface-ndk_platform-source/gen/android/aidl/loggable/Data.cpp b/tests/golden_output/aidl_test_loggable_interface-ndk_platform-source/gen/android/aidl/loggable/Data.cpp
index fb304964..29ba3c3d 100644
--- a/tests/golden_output/aidl_test_loggable_interface-ndk_platform-source/gen/android/aidl/loggable/Data.cpp
+++ b/tests/golden_output/aidl_test_loggable_interface-ndk_platform-source/gen/android/aidl/loggable/Data.cpp
@@ -16,6 +16,10 @@ binder_status_t Data::readFromParcel(const AParcel* parcel) {
   if (_aidl_parcelable_size < 0) return STATUS_BAD_VALUE;
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
+  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
+    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
+    return _aidl_ret_status;
+  }
   _aidl_ret_status = AParcel_readInt32(parcel, &num);
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
@@ -40,10 +44,6 @@ binder_status_t Data::readFromParcel(const AParcel* parcel) {
   _aidl_ret_status = AParcel_readByte(parcel, reinterpret_cast<int8_t*>(&nestedEnum));
   if (_aidl_ret_status != STATUS_OK) return _aidl_ret_status;
 
-  if (AParcel_getDataPosition(parcel) - _aidl_start_pos >= _aidl_parcelable_size) {
-    AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
-    return _aidl_ret_status;
-  }
   AParcel_setDataPosition(parcel, _aidl_start_pos + _aidl_parcelable_size);
   return _aidl_ret_status;
 }
diff --git a/tests/java/src/android/aidl/tests/TestVersionedInterface.java b/tests/java/src/android/aidl/tests/TestVersionedInterface.java
index 49236075..1223a79e 100644
--- a/tests/java/src/android/aidl/tests/TestVersionedInterface.java
+++ b/tests/java/src/android/aidl/tests/TestVersionedInterface.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 import android.aidl.versioned.tests.BazUnion;
+import android.aidl.versioned.tests.Foo;
 import android.aidl.versioned.tests.IFooInterface;
 import android.os.IBinder;
 import android.os.RemoteException;
@@ -76,4 +77,11 @@ public class TestVersionedInterface {
 
       service.acceptUnionAndReturnString(BazUnion.longNum(42L));
     }
+
+    @Test
+    public void testPacelableParamWithNewFields() throws RemoteException {
+      Foo outFoo = new Foo();
+      service.callWithFoo(outFoo);
+      assertThat(outFoo.intDefault42, is(42));
+    }
 }
diff --git a/tests/rust/test_client.rs b/tests/rust/test_client.rs
index 0004748b..4be90aa8 100644
--- a/tests/rust/test_client.rs
+++ b/tests/rust/test_client.rs
@@ -656,6 +656,18 @@ fn test_versioned_unknown_union_field_triggers_error() {
     }
 }
 
+#[test]
+fn test_parcelable_param_with_new_fields() {
+    let service: binder::Strong<dyn IFooInterface::IFooInterface> =
+        binder::get_interface(<BpFooInterface as IFooInterface::IFooInterface>::get_descriptor())
+            .expect("did not get binder service");
+
+    let mut out_foo = Default::default();
+    let ret = service.callWithFoo(&mut out_foo);
+    assert!(ret.is_ok());
+    assert_eq!(out_foo.intDefault42, 42);
+}
+
 fn test_renamed_interface<F>(f: F)
 where
     F: FnOnce(binder::Strong<dyn IOldName::IOldName>, binder::Strong<dyn INewName::INewName>),