Merge "Revert "Check transport before registering."" am: 809efd8763 am: 9e8db15403 am: 498d72be9d am: 4df245a2a9
am: 28160d2488
Change-Id: I4bacf867cb89f5ec106b62f4465d2b01aef8cf74
diff --git a/base/HidlSupport.cpp b/base/HidlSupport.cpp
index f97f216..af805b9 100644
--- a/base/HidlSupport.cpp
+++ b/base/HidlSupport.cpp
@@ -35,10 +35,8 @@
}
} // namespace details
-hidl_handle::hidl_handle() {
- memset(this, 0, sizeof(*this));
- // mHandle = nullptr;
- // mOwnsHandle = false;
+hidl_handle::hidl_handle() : mHandle(nullptr), mOwnsHandle(false) {
+ memset(mPad, 0, sizeof(mPad));
}
hidl_handle::~hidl_handle() {
@@ -138,11 +136,8 @@
static const char *const kEmptyString = "";
-hidl_string::hidl_string() {
- memset(this, 0, sizeof(*this));
- // mSize is zero
- // mOwnsBuffer is false
- mBuffer = kEmptyString;
+hidl_string::hidl_string() : mBuffer(kEmptyString), mSize(0), mOwnsBuffer(false) {
+ memset(mPad, 0, sizeof(mPad));
}
hidl_string::~hidl_string() {
diff --git a/base/include/hidl/HidlSupport.h b/base/include/hidl/HidlSupport.h
index 89aa5a9..71448da 100644
--- a/base/include/hidl/HidlSupport.h
+++ b/base/include/hidl/HidlSupport.h
@@ -20,18 +20,24 @@
#include <algorithm>
#include <array>
#include <iterator>
-#include <cutils/native_handle.h>
#include <hidl/HidlInternal.h>
-#include <hidl/Status.h>
#include <map>
#include <sstream>
#include <stddef.h>
#include <tuple>
#include <type_traits>
+#include <vector>
+
+// no requirements on types not used in scatter/gather
+// no requirements on other libraries
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wpadded"
+#include <cutils/native_handle.h>
+#include <hidl/Status.h>
#include <utils/Errors.h>
#include <utils/RefBase.h>
#include <utils/StrongPointer.h>
-#include <vector>
+#pragma clang diagnostic pop
namespace android {
@@ -123,8 +129,9 @@
private:
void freeHandle();
- details::hidl_pointer<const native_handle_t> mHandle __attribute__ ((aligned(8)));
- bool mOwnsHandle __attribute ((aligned(8)));
+ details::hidl_pointer<const native_handle_t> mHandle;
+ bool mOwnsHandle;
+ uint8_t mPad[7];
};
struct hidl_string {
@@ -174,6 +181,7 @@
details::hidl_pointer<const char> mBuffer;
uint32_t mSize; // NOT including the terminating '\0'.
bool mOwnsBuffer; // if true then mBuffer is a mutable char *
+ uint8_t mPad[3];
// copy from data with size. Assume that my memory is freed
// (through clear(), for example)
@@ -294,9 +302,9 @@
static const size_t kOffsetOfName;
private:
- hidl_handle mHandle __attribute__ ((aligned(8)));
- uint64_t mSize __attribute__ ((aligned(8)));
- hidl_string mName __attribute__ ((aligned(8)));
+ hidl_handle mHandle;
+ uint64_t mSize;
+ hidl_string mName;
};
// HidlMemory is a wrapper class to support sp<> for hidl_memory. It also
@@ -328,15 +336,12 @@
struct hidl_vec {
using value_type = T;
- hidl_vec() {
+ hidl_vec() : mBuffer(nullptr), mSize(0), mOwnsBuffer(true) {
static_assert(hidl_vec<T>::kOffsetOfBuffer == 0, "wrong offset");
- memset(this, 0, sizeof(*this));
- // mSize is 0
- // mBuffer is nullptr
+ // mOwnsBuffer true to match original implementation
- // this is for consistency with the original implementation
- mOwnsBuffer = true;
+ memset(mPad, 0, sizeof(mPad));
}
// Note, does not initialize primitive types.
@@ -581,6 +586,7 @@
details::hidl_pointer<T> mBuffer;
uint32_t mSize;
bool mOwnsBuffer;
+ uint8_t mPad[3];
// copy from an array-like object, assuming my resources are freed.
template <typename Array>
diff --git a/test_main.cpp b/test_main.cpp
index 7b6781a..083cee4 100644
--- a/test_main.cpp
+++ b/test_main.cpp
@@ -16,11 +16,16 @@
#define LOG_TAG "LibHidlTest"
+#pragma clang diagnostic push
+#pragma clang diagnostic fatal "-Wpadded"
+#include <hidl/HidlInternal.h>
+#include <hidl/HidlSupport.h>
+#pragma clang diagnostic pop
+
#include <android-base/logging.h>
#include <android/hidl/memory/1.0/IMemory.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
-#include <hidl/HidlSupport.h>
#include <hidl/ServiceManagement.h>
#include <hidl/Status.h>
#include <hidl/TaskRunner.h>
@@ -570,6 +575,51 @@
EXPECT_TRUE(isLibraryOpen(kLib));
}
+template <typename T, size_t start, size_t end>
+static void assertZeroInRange(const T* t) {
+ static_assert(start < sizeof(T));
+ static_assert(end <= sizeof(T));
+
+ const uint8_t* ptr = reinterpret_cast<const uint8_t*>(t);
+
+ for (size_t i = start; i < end; i++) {
+ EXPECT_EQ(0, ptr[i]);
+ }
+}
+
+template <typename T, size_t start, size_t end>
+static void uninitTest() {
+ uint8_t buf[sizeof(T)];
+ memset(buf, 0xFF, sizeof(T));
+
+ T* type = new (buf) T;
+ assertZeroInRange<T, start, end>(type);
+ type->~T();
+}
+
+TEST_F(LibHidlTest, HidlVecUninit) {
+ using ::android::hardware::hidl_vec;
+ struct SomeType {};
+ static_assert(sizeof(hidl_vec<SomeType>) == 16);
+
+ // padding after mOwnsBuffer
+ uninitTest<hidl_vec<SomeType>, 13, 16>();
+}
+TEST_F(LibHidlTest, HidlHandleUninit) {
+ using ::android::hardware::hidl_handle;
+ static_assert(sizeof(hidl_handle) == 16);
+
+ // padding after mOwnsHandle
+ uninitTest<hidl_handle, 9, 16>();
+}
+TEST_F(LibHidlTest, HidlStringUninit) {
+ using ::android::hardware::hidl_string;
+ static_assert(sizeof(hidl_string) == 16);
+
+ // padding after mOwnsBuffer
+ uninitTest<hidl_string, 13, 16>();
+}
+
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
diff --git a/transport/include/hidl/HidlBinderSupport.h b/transport/include/hidl/HidlBinderSupport.h
index 5dec5cd..05f7fab 100644
--- a/transport/include/hidl/HidlBinderSupport.h
+++ b/transport/include/hidl/HidlBinderSupport.h
@@ -204,10 +204,17 @@
if (binderIface.get() == nullptr) {
return nullptr;
}
+
if (binderIface->localBinder() == nullptr) {
return new ProxyType(binderIface);
}
+
+ // Ensure that IBinder is BnHwBase (not JHwBinder, for instance)
+ if (!binderIface->checkSubclass(IBase::descriptor)) {
+ return new ProxyType(binderIface);
+ }
sp<IBase> base = static_cast<BnHwBase*>(binderIface.get())->getImpl();
+
if (details::canCastInterface(base.get(), IType::descriptor)) {
StubType* stub = static_cast<StubType*>(binderIface.get());
return stub->getImpl();