diff options
-rw-r--r-- | libs/binder/Parcel.cpp | 6 | ||||
-rw-r--r-- | libs/binder/Stability.cpp | 34 | ||||
-rw-r--r-- | libs/binder/include/binder/Stability.h | 10 | ||||
-rw-r--r-- | libs/binder/tests/binderStabilityTest.cpp | 37 |
4 files changed, 43 insertions, 44 deletions
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 51ca7b4f02..a2333ae07e 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -167,6 +167,8 @@ static void release_object(const sp<ProcessState>& proc, status_t Parcel::finishFlattenBinder( const sp<IBinder>& /*binder*/, const flat_binder_object& flat) { + // internal::Stability::tryMarkCompilationUnit(binder.get()); + status_t status = writeObject(flat, false); if (status != OK) return status; @@ -183,11 +185,11 @@ status_t Parcel::finishUnflattenBinder( // status_t status = readInt32(&stability); // if (status != OK) return status; - // if (!internal::Stability::check(stability, mRequiredStability)) { + // if (binder != nullptr && !internal::Stability::check(stability, mRequiredStability)) { // return BAD_TYPE; // } - // status = internal::Stability::set(binder.get(), stability); + // status = internal::Stability::set(binder.get(), stability, true /*log*/); // if (status != OK) return status; *out = binder; diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp index d6d312a842..f18bdca1fd 100644 --- a/libs/binder/Stability.cpp +++ b/libs/binder/Stability.cpp @@ -13,29 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #include <binder/Stability.h> namespace android { namespace internal { void Stability::markCompilationUnit(IBinder* binder) { -#ifdef __ANDROID_VNDK__ -constexpr Stability::Level kLocalStability = Stability::Level::VENDOR; -#else -constexpr Stability::Level kLocalStability = Stability::Level::SYSTEM; -#endif - - status_t result = set(binder, kLocalStability); + status_t result = set(binder, kLocalStability, true /*log*/); LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object."); } void Stability::markVintf(IBinder* binder) { - status_t result = set(binder, Level::VINTF); + status_t result = set(binder, Level::VINTF, true /*log*/); LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object."); } -status_t Stability::set(IBinder* binder, int32_t stability) { +void Stability::tryMarkCompilationUnit(IBinder* binder) { + (void) set(binder, kLocalStability, false /*log*/); +} + +status_t Stability::set(IBinder* binder, int32_t stability, bool log) { Level currentStability = get(binder); // null binder is always written w/ 'UNDECLARED' stability @@ -43,23 +40,26 @@ status_t Stability::set(IBinder* binder, int32_t stability) { if (stability == UNDECLARED) { return OK; } else { - ALOGE("Null binder written with stability %s.", stabilityString(stability).c_str()); + if (log) { + ALOGE("Null binder written with stability %s.", + stabilityString(stability).c_str()); + } return BAD_TYPE; } } if (!isDeclaredStability(stability)) { - // There are UNDECLARED sets because some binder interfaces don't set their stability, and - // then UNDECLARED stability is sent on the other side. - if (stability != UNDECLARED) { + if (log) { ALOGE("Can only set known stability, not %d.", stability); - return BAD_TYPE; } + return BAD_TYPE; } if (currentStability != Level::UNDECLARED && currentStability != stability) { - ALOGE("Interface being set with %s but it is already marked as %s.", - stabilityString(stability).c_str(), stabilityString(stability).c_str()); + if (log) { + ALOGE("Interface being set with %s but it is already marked as %s.", + stabilityString(stability).c_str(), stabilityString(stability).c_str()); + } return BAD_TYPE; } diff --git a/libs/binder/include/binder/Stability.h b/libs/binder/include/binder/Stability.h index 487f480d0e..148def60d3 100644 --- a/libs/binder/include/binder/Stability.h +++ b/libs/binder/include/binder/Stability.h @@ -45,6 +45,8 @@ private: // up the efficiency level of a binder object. So, we expose the underlying type. friend ::android::Parcel; + static void tryMarkCompilationUnit(IBinder* binder); + enum Level : int16_t { UNDECLARED = 0, @@ -53,9 +55,15 @@ private: VINTF = 0b111111, }; +#ifdef __ANDROID_VNDK__ + static constexpr Level kLocalStability = Level::VENDOR; +#else + static constexpr Level kLocalStability = Level::SYSTEM; +#endif + // applies stability to binder if stability level is known __attribute__((warn_unused_result)) - static status_t set(IBinder* binder, int32_t stability); + static status_t set(IBinder* binder, int32_t stability, bool log); static Level get(IBinder* binder); diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp index 38e7f52734..d8159e3e2d 100644 --- a/libs/binder/tests/binderStabilityTest.cpp +++ b/libs/binder/tests/binderStabilityTest.cpp @@ -87,30 +87,16 @@ public: } }; -void checkNoStabilityServer(const sp<IBinderStabilityTest>& unkemptServer) { - EXPECT_TRUE(unkemptServer->sendBinder(new BBinder()).isOk()); - EXPECT_TRUE(unkemptServer->sendBinder(getCompilationUnitStability()).isOk()); - EXPECT_TRUE(unkemptServer->sendBinder(getVintfStability()).isOk()); - - sp<IBinder> out; - EXPECT_TRUE(unkemptServer->returnNoStabilityBinder(&out).isOk()); - EXPECT_NE(nullptr, out.get()); - - EXPECT_TRUE(unkemptServer->returnLocalStabilityBinder(&out).isOk()); - EXPECT_NE(nullptr, out.get()); - - EXPECT_TRUE(unkemptServer->returnVintfStabilityBinder(&out).isOk()); - EXPECT_NE(nullptr, out.get()); -} - -void checkLowStabilityServer(const sp<IBinderStabilityTest>& complServer) { - EXPECT_FALSE(complServer->sendBinder(new BBinder()).isOk()); +void checkLocalStabilityBinder(const sp<IBinderStabilityTest>& complServer) { + // this binder should automatically be set to local stability + EXPECT_TRUE(complServer->sendBinder(new BBinder()).isOk()); EXPECT_TRUE(complServer->sendBinder(getCompilationUnitStability()).isOk()); EXPECT_TRUE(complServer->sendBinder(getVintfStability()).isOk()); sp<IBinder> out; - EXPECT_FALSE(complServer->returnNoStabilityBinder(&out).isOk()); - EXPECT_EQ(nullptr, out.get()); + // should automatically be set to local stability + EXPECT_TRUE(complServer->returnNoStabilityBinder(&out).isOk()); + EXPECT_NE(nullptr, out.get()); EXPECT_TRUE(complServer->returnLocalStabilityBinder(&out).isOk()); EXPECT_NE(nullptr, out.get()); @@ -140,13 +126,15 @@ TEST(BinderStability, LocalNoStabilityServer) { // or was written by hand. auto server = BadStabilityTester::getNoStabilityServer(); ASSERT_NE(nullptr, IInterface::asBinder(server)->localBinder()); - checkNoStabilityServer(server); + + // it should be considered to have local stability + checkLocalStabilityBinder(server); } TEST(BinderStability, LocalLowStabilityServer) { auto server = BadStabilityTester::getCompilationUnitStabilityServer(); ASSERT_NE(nullptr, IInterface::asBinder(server)->localBinder()); - checkLowStabilityServer(server); + checkLocalStabilityBinder(server); } TEST(BinderStability, LocalHighStabilityServer) { @@ -162,7 +150,8 @@ TEST(BinderStability, RemoteNoStabilityServer) { ASSERT_NE(nullptr, remoteServer.get()); ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder()); - checkNoStabilityServer(remoteServer); + // it should be considered to have local stability + checkLocalStabilityBinder(remoteServer); } TEST(BinderStability, RemoteLowStabilityServer) { @@ -172,7 +161,7 @@ TEST(BinderStability, RemoteLowStabilityServer) { ASSERT_NE(nullptr, remoteServer.get()); ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder()); - checkLowStabilityServer(remoteServer); + checkLocalStabilityBinder(remoteServer); } TEST(BinderStability, RemoteVintfServer) { |