Merge "binderStabilityTest: rewrite pieces w/ no AIDL"

4 files changed, 151 insertions, 133 deletions

diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index d59a40d8d9..bc457ce30c 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -142,7 +142,6 @@ aidl_interface {
     name: "binderStabilityTestIface",
     srcs: [
         "IBinderStabilityTest.aidl",
-        "IBinderStabilityTestSub.aidl",
     ],
 }
 
diff --git a/libs/binder/tests/IBinderStabilityTest.aidl b/libs/binder/tests/IBinderStabilityTest.aidl
index 9559196db4..36e1c2cbc4 100644
--- a/libs/binder/tests/IBinderStabilityTest.aidl
+++ b/libs/binder/tests/IBinderStabilityTest.aidl
@@ -14,34 +14,32 @@
  * limitations under the License.
  */
 
-import IBinderStabilityTestSub;
-
 // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
 // THIS IS ONLY FOR TESTING!
 interface IBinderStabilityTest {
     // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
     // THIS IS ONLY FOR TESTING!
-    void sendBinder(IBinderStabilityTestSub binder);
+    void sendBinder(IBinder binder);
 
     // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
     // THIS IS ONLY FOR TESTING!
-    void sendAndCallBinder(IBinderStabilityTestSub binder);
+    void sendAndCallBinder(IBinder binder);
 
     // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
     // THIS IS ONLY FOR TESTING!
-    IBinderStabilityTestSub returnNoStabilityBinder();
+    IBinder returnNoStabilityBinder();
 
     // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
     // THIS IS ONLY FOR TESTING!
-    IBinderStabilityTestSub returnLocalStabilityBinder();
+    IBinder returnLocalStabilityBinder();
 
     // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
     // THIS IS ONLY FOR TESTING!
-    IBinderStabilityTestSub returnVintfStabilityBinder();
+    IBinder returnVintfStabilityBinder();
 
     // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
     // THIS IS ONLY FOR TESTING!
-    IBinderStabilityTestSub returnVendorStabilityBinder();
+    IBinder returnVendorStabilityBinder();
 }
 // DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
 // THIS IS ONLY FOR TESTING!
diff --git a/libs/binder/tests/IBinderStabilityTestSub.aidl b/libs/binder/tests/IBinderStabilityTestSub.aidl
deleted file mode 100644
index a81ebf97b9..0000000000
--- a/libs/binder/tests/IBinderStabilityTestSub.aidl
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * Copyright (C) 2019 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.
- */
-
-interface IBinderStabilityTestSub {
-    void userDefinedTransaction();
-}
diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp
index 936b82109a..490850ebf4 100644
--- a/libs/binder/tests/binderStabilityTest.cpp
+++ b/libs/binder/tests/binderStabilityTest.cpp
@@ -26,9 +26,7 @@
 
 #include <sys/prctl.h>
 
-#include "aidl/BnBinderStabilityTestSub.h"
 #include "aidl/BnBinderStabilityTest.h"
-#include "BnBinderStabilityTestSub.h"
 #include "BnBinderStabilityTest.h"
 
 using namespace android;
@@ -36,167 +34,217 @@ using namespace ndk;
 using android::binder::Status;
 using android::internal::Stability; // for testing only!
 
-const String16 kNoStabilityServer = String16("binder_stability_test_service_low");
-const String16 kCompilationUnitServer = String16("binder_stability_test_service_compl");
-const String16 kVintfServer = String16("binder_stability_test_service_vintf");
+const String16 kSystemStabilityServer = String16("binder_stability_test_service_system");
 
-const String16 kCompilationUnitNdkServer = String16("binder_stability_test_service_compl");
-
-class BadStabilityTestSub : public BnBinderStabilityTestSub {
+// This is handwritten so that we can test different stability levels w/o having the AIDL
+// compiler assign them. Hand-writing binder interfaces is considered a bad practice
+// sanity reasons. YOU SHOULD DEFINE AN AIDL INTERFACE INSTEAD!
+class BadStableBinder : public BBinder {
 public:
-    Status userDefinedTransaction() {
-        return Status::ok();
+    static constexpr uint32_t USER_TRANSACTION = IBinder::FIRST_CALL_TRANSACTION;
+    static String16 kDescriptor;
+
+    bool gotUserTransaction = false;
+
+    static status_t doUserTransaction(const sp<IBinder>& binder) {
+        Parcel data, reply;
+        data.writeInterfaceToken(kDescriptor);
+        return binder->transact(USER_TRANSACTION, data, &reply, 0/*flags*/);
+    }
+
+    status_t onTransact(uint32_t code,
+            const Parcel& data, Parcel* reply, uint32_t flags) override {
+        if (code == USER_TRANSACTION) {
+            // not interested in this kind of stability. Make sure
+            // we have a test failure
+            LOG_ALWAYS_FATAL_IF(!data.enforceInterface(kDescriptor));
+
+            gotUserTransaction = true;
+
+            ALOGE("binder stability: Got user transaction");
+            return OK;
+        }
+        return BBinder::onTransact(code, data, reply, flags);
     }
 
-    static sp<IBinderStabilityTestSub> system() {
-        sp<BnBinderStabilityTestSub> iface = new BadStabilityTestSub();
-        // NO! NO! NO! NO! DO NOT EVERY DO SOMETHING LIKE THIS?
-        // WHAT ARE YOU CRAZY? IT'S VERY DANGEROUS
-        Stability::markCompilationUnit(iface.get()); // <- BAD, NO! DO NOT COPY
+    static sp<BadStableBinder> undef() {
+        sp<BadStableBinder> iface = new BadStableBinder();
         return iface;
     }
 
-    static sp<IBinderStabilityTestSub> vintf() {
-        sp<BnBinderStabilityTestSub> iface = new BadStabilityTestSub();
-        // NO! NO! NO! NO! DO NOT EVERY DO SOMETHING LIKE THIS?
-        // WHAT ARE YOU CRAZY? IT'S VERY DANGEROUS
-        Stability::markVintf(iface.get()); // <- BAD, NO! DO NOT COPY
+    static sp<BadStableBinder> system() {
+        sp<BadStableBinder> iface = new BadStableBinder();
+        Stability::markCompilationUnit(iface.get()); // <- for test only
         return iface;
     }
 
-    static sp<IBinderStabilityTestSub> vendor() {
-        sp<BnBinderStabilityTestSub> iface = new BadStabilityTestSub();
-        // NO! NO! NO! NO! DO NOT EVERY DO SOMETHING LIKE THIS?
-        // WHAT ARE YOU CRAZY? IT'S VERY DANGEROUS
-        Stability::markVndk(iface.get()); // <- BAD, NO! DO NOT COPY
+    static sp<BadStableBinder> vintf() {
+        sp<BadStableBinder> iface = new BadStableBinder();
+        Stability::markVintf(iface.get()); // <- for test only
+        return iface;
+    }
+
+    static sp<BadStableBinder> vendor() {
+        sp<BadStableBinder> iface = new BadStableBinder();
+        Stability::markVndk(iface.get()); // <- for test only
         return iface;
     }
 };
+String16 BadStableBinder::kDescriptor = String16("BadStableBinder.test");
 
 // NO! NO! NO! Do not even think of doing something like this!
 // This is for testing! If a class like this was actually used in production,
 // it would ruin everything!
-class BadStabilityTester : public BnBinderStabilityTest {
+class MyBinderStabilityTest : public BnBinderStabilityTest {
 public:
-    Status sendBinder(const sp<IBinderStabilityTestSub>& /*binder*/) override {
+    Status sendBinder(const sp<IBinder>& /*binder*/) override {
         return Status::ok();
     }
-    Status sendAndCallBinder(const sp<IBinderStabilityTestSub>& binder) override {
-        Stability::debugLogStability("sendAndCallBinder got binder", IInterface::asBinder(binder));
-        return binder->userDefinedTransaction();
+    Status sendAndCallBinder(const sp<IBinder>& binder) override {
+        Stability::debugLogStability("sendAndCallBinder got binder", binder);
+        return Status::fromExceptionCode(BadStableBinder::doUserTransaction(binder));
     }
-    Status returnNoStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
-        *_aidl_return = new BadStabilityTestSub();
+    Status returnNoStabilityBinder(sp<IBinder>* _aidl_return) override {
+        *_aidl_return = BadStableBinder::undef();
         return Status::ok();
     }
-    Status returnLocalStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
-        *_aidl_return = BadStabilityTestSub::system();
+    Status returnLocalStabilityBinder(sp<IBinder>* _aidl_return) override {
+        *_aidl_return = BadStableBinder::system();
         return Status::ok();
     }
-    Status returnVintfStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
-        *_aidl_return = BadStabilityTestSub::vintf();
+    Status returnVintfStabilityBinder(sp<IBinder>* _aidl_return) override {
+        *_aidl_return = BadStableBinder::vintf();
         return Status::ok();
     }
-    Status returnVendorStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
-        *_aidl_return = BadStabilityTestSub::vendor();
+    Status returnVendorStabilityBinder(sp<IBinder>* _aidl_return) override {
+        *_aidl_return = BadStableBinder::vendor();
         return Status::ok();
     }
 };
 
-void checkSystemStabilityBinder(const sp<IBinderStabilityTest>& complServer) {
-    EXPECT_TRUE(complServer->sendBinder(new BadStabilityTestSub()).isOk());
-    EXPECT_TRUE(complServer->sendBinder(BadStabilityTestSub::system()).isOk());
-    EXPECT_TRUE(complServer->sendBinder(BadStabilityTestSub::vintf()).isOk());
-    EXPECT_TRUE(complServer->sendBinder(BadStabilityTestSub::vendor()).isOk());
+TEST(BinderStability, CantCallVendorBinderInSystemContext) {
+    sp<IBinder> serverBinder = android::defaultServiceManager()->getService(kSystemStabilityServer);
+    auto server = interface_cast<IBinderStabilityTest>(serverBinder);
+
+    ASSERT_NE(nullptr, server.get());
+    ASSERT_NE(nullptr, IInterface::asBinder(server)->remoteBinder());
 
-    EXPECT_TRUE(complServer->sendAndCallBinder(new BadStabilityTestSub()).isOk());
-    EXPECT_TRUE(complServer->sendAndCallBinder(BadStabilityTestSub::system()).isOk());
-    EXPECT_TRUE(complServer->sendAndCallBinder(BadStabilityTestSub::vintf()).isOk());
+    EXPECT_TRUE(server->sendBinder(BadStableBinder::undef()).isOk());
+    EXPECT_TRUE(server->sendBinder(BadStableBinder::system()).isOk());
+    EXPECT_TRUE(server->sendBinder(BadStableBinder::vintf()).isOk());
+    EXPECT_TRUE(server->sendBinder(BadStableBinder::vendor()).isOk());
 
-    // !!! user-defined transaction may not be stable for remote server !!!
-    EXPECT_FALSE(complServer->sendAndCallBinder(BadStabilityTestSub::vendor()).isOk());
+    {
+        sp<BadStableBinder> binder = BadStableBinder::undef();
+        EXPECT_TRUE(server->sendAndCallBinder(binder).isOk());
+        EXPECT_TRUE(binder->gotUserTransaction);
+    }
+    {
+        sp<BadStableBinder> binder = BadStableBinder::system();
+        EXPECT_TRUE(server->sendAndCallBinder(binder).isOk());
+        EXPECT_TRUE(binder->gotUserTransaction);
+    }
+    {
+        sp<BadStableBinder> binder = BadStableBinder::vintf();
+        EXPECT_TRUE(server->sendAndCallBinder(binder).isOk());
+        EXPECT_TRUE(binder->gotUserTransaction);
+    }
+    {
+        // !!! user-defined transaction may not be stable for remote server !!!
+        // !!! so, it does not work !!!
+        sp<BadStableBinder> binder = BadStableBinder::vendor();
+        EXPECT_EQ(BAD_TYPE, server->sendAndCallBinder(binder).exceptionCode());
+        EXPECT_FALSE(binder->gotUserTransaction);
+    }
 
-    sp<IBinderStabilityTestSub> out;
-    EXPECT_TRUE(complServer->returnNoStabilityBinder(&out).isOk());
+    sp<IBinder> out;
+    EXPECT_TRUE(server->returnNoStabilityBinder(&out).isOk());
     ASSERT_NE(nullptr, out.get());
-    EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
-    EXPECT_TRUE(out->userDefinedTransaction().isOk());
+    EXPECT_EQ(OK, out->pingBinder());
+    EXPECT_EQ(OK, BadStableBinder::doUserTransaction(out));
 
-    EXPECT_TRUE(complServer->returnLocalStabilityBinder(&out).isOk());
+    EXPECT_TRUE(server->returnLocalStabilityBinder(&out).isOk());
     ASSERT_NE(nullptr, out.get());
-    EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
-    EXPECT_TRUE(out->userDefinedTransaction().isOk());
+    EXPECT_EQ(OK, out->pingBinder());
+    EXPECT_EQ(OK, BadStableBinder::doUserTransaction(out));
 
-    EXPECT_TRUE(complServer->returnVintfStabilityBinder(&out).isOk());
+    EXPECT_TRUE(server->returnVintfStabilityBinder(&out).isOk());
     ASSERT_NE(nullptr, out.get());
-    EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
-    EXPECT_TRUE(out->userDefinedTransaction().isOk());
+    EXPECT_EQ(OK, out->pingBinder());
+    EXPECT_EQ(OK, BadStableBinder::doUserTransaction(out));
 
-    EXPECT_TRUE(complServer->returnVendorStabilityBinder(&out).isOk());
+    EXPECT_TRUE(server->returnVendorStabilityBinder(&out).isOk());
     ASSERT_NE(nullptr, out.get());
 
     // !!! libbinder-defined transaction works !!!
-    EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
+    EXPECT_EQ(OK, out->pingBinder());
 
     // !!! user-defined transaction may not be stable !!!
-    EXPECT_FALSE(out->userDefinedTransaction().isOk());
+    // !!! so, it does not work !!!
+    EXPECT_EQ(BAD_TYPE, BadStableBinder::doUserTransaction(out));
 }
 
-TEST(BinderStability, RemoteNoStabilityServer) {
-    sp<IBinder> remoteBinder = android::defaultServiceManager()->getService(kNoStabilityServer);
-    auto remoteServer = interface_cast<IBinderStabilityTest>(remoteBinder);
+// This is handwritten so that we can test different stability levels w/o having the AIDL
+// compiler assign them. Hand-writing binder interfaces is considered a bad practice
+// sanity reasons. YOU SHOULD DEFINE AN AIDL INTERFACE INSTEAD!
 
-    ASSERT_NE(nullptr, remoteServer.get());
-    ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
-
-    checkSystemStabilityBinder(remoteServer);
+struct NdkBinderStable_DataClass {
+    bool gotUserTransaction = false;
+};
+void* NdkBadStableBinder_Class_onCreate(void* args) {
+    LOG_ALWAYS_FATAL_IF(args != nullptr, "Takes no args");
+    return static_cast<void*>(new NdkBinderStable_DataClass);
 }
-
-TEST(BinderStability, RemoteLowStabilityServer) {
-    sp<IBinder> remoteBinder = android::defaultServiceManager()->getService(kCompilationUnitServer);
-    auto remoteServer = interface_cast<IBinderStabilityTest>(remoteBinder);
-
-    ASSERT_NE(nullptr, remoteServer.get());
-    ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
-
-    checkSystemStabilityBinder(remoteServer);
+void NdkBadStableBinder_Class_onDestroy(void* userData) {
+    delete static_cast<NdkBinderStable_DataClass*>(userData);
 }
+NdkBinderStable_DataClass* NdkBadStableBinder_getUserData(AIBinder* binder) {
+    LOG_ALWAYS_FATAL_IF(binder == nullptr);
+    void* userData = AIBinder_getUserData(binder);
+    LOG_ALWAYS_FATAL_IF(userData == nullptr, "null data - binder is remote?");
 
-TEST(BinderStability, RemoteVintfServer) {
-    sp<IBinder> remoteBinder = android::defaultServiceManager()->getService(kVintfServer);
-    auto remoteServer = interface_cast<IBinderStabilityTest>(remoteBinder);
+    return static_cast<NdkBinderStable_DataClass*>(userData);
+}
+binder_status_t NdkBadStableBinder_Class_onTransact(
+    AIBinder* binder, transaction_code_t code, const AParcel* /*in*/, AParcel* /*out*/) {
 
-    ASSERT_NE(nullptr, remoteServer.get());
-    ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
+    if (code == BadStableBinder::USER_TRANSACTION) {
+        ALOGE("ndk binder stability: Got user transaction");
+        NdkBadStableBinder_getUserData(binder)->gotUserTransaction = true;
+        return STATUS_OK;
+    }
 
-    checkSystemStabilityBinder(remoteServer);
+    return STATUS_UNKNOWN_TRANSACTION;
 }
 
-class NdkBadStabilityTestSub : public aidl::BnBinderStabilityTestSub {
-    ScopedAStatus userDefinedTransaction() {
-        return ScopedAStatus::ok();
-    }
-};
+static AIBinder_Class* kNdkBadStableBinder =
+    AIBinder_Class_define(String8(BadStableBinder::kDescriptor).c_str(),
+                          NdkBadStableBinder_Class_onCreate,
+                          NdkBadStableBinder_Class_onDestroy,
+                          NdkBadStableBinder_Class_onTransact);
+
 // for testing only to get around __ANDROID_VNDK__ guard.
 extern "C" void AIBinder_markVendorStability(AIBinder* binder); // <- BAD DO NOT COPY
 
-TEST(BinderStability, NdkClientOfRemoteServer) {
+TEST(BinderStability, NdkCantCallVendorBinderInSystemContext) {
     SpAIBinder binder = SpAIBinder(AServiceManager_getService(
-        String8(kCompilationUnitServer).c_str()));
+        String8(kSystemStabilityServer).c_str()));
 
     std::shared_ptr<aidl::IBinderStabilityTest> remoteServer =
         aidl::IBinderStabilityTest::fromBinder(binder);
 
     ASSERT_NE(nullptr, remoteServer.get());
 
-    std::shared_ptr<aidl::IBinderStabilityTestSub> vendor = SharedRefBase::make<NdkBadStabilityTestSub>();
-
-    // TODO: not ideal: binder must be held once it is marked
-    SpAIBinder vendorBinder = vendor->asBinder();
-    AIBinder_markVendorStability(vendorBinder.get());
+    SpAIBinder comp = SpAIBinder(AIBinder_new(kNdkBadStableBinder, nullptr /*args*/));
+    EXPECT_TRUE(remoteServer->sendBinder(comp).isOk());
+    EXPECT_TRUE(remoteServer->sendAndCallBinder(comp).isOk());
+    EXPECT_TRUE(NdkBadStableBinder_getUserData(comp.get())->gotUserTransaction);
 
+    SpAIBinder vendor = SpAIBinder(AIBinder_new(kNdkBadStableBinder, nullptr /*args*/));
+    AIBinder_markVendorStability(vendor.get());
     EXPECT_TRUE(remoteServer->sendBinder(vendor).isOk());
     EXPECT_FALSE(remoteServer->sendAndCallBinder(vendor).isOk());
+    EXPECT_FALSE(NdkBadStableBinder_getUserData(vendor.get())->gotUserTransaction);
 }
 
 class MarksStabilityInConstructor : public BBinder {
@@ -234,16 +282,8 @@ int main(int argc, char** argv) {
         // child process
         prctl(PR_SET_PDEATHSIG, SIGHUP);
 
-        sp<IBinder> noStability = new BadStabilityTester;
-        android::defaultServiceManager()->addService(kNoStabilityServer, noStability);
-
-        sp<IBinder> compil = new BadStabilityTester;
-        Stability::markCompilationUnit(compil.get());
-        android::defaultServiceManager()->addService(kCompilationUnitServer, compil);
-
-        sp<IBinder> vintf = new BadStabilityTester;
-        Stability::markVintf(vintf.get());
-        android::defaultServiceManager()->addService(kVintfServer, vintf);
+        sp<IBinder> server = new MyBinderStabilityTest;
+        android::defaultServiceManager()->addService(kSystemStabilityServer, server);
 
         IPCThreadState::self()->joinThreadPool(true);
         exit(1);  // should not reach