Merge "Adds FqInstance::getFqName()."
am: a74c7692b1
Change-Id: I5753bf7a1fdbdf877f18eadc2df2d368c2d755b2
diff --git a/CompoundType.cpp b/CompoundType.cpp
index 53020b2..0be78a8 100644
--- a/CompoundType.cpp
+++ b/CompoundType.cpp
@@ -1058,14 +1058,27 @@
<< fullName()
<< ", hidl_d) == 0, \"wrong offset\");\n";
+ const CompoundLayout layout = getCompoundAlignmentAndSize();
+
if (!containsPointer()) {
- CompoundLayout layout = getCompoundAlignmentAndSize();
- out << "static_assert(offsetof("
- << fullName()
- << ", hidl_u) == "
- << layout.innerStruct.offset
- << ", \"wrong offset\");\n";
+ out << "static_assert(offsetof(" << fullName()
+ << ", hidl_u) == " << layout.innerStruct.offset << ", \"wrong offset\");\n";
}
+
+ out.endl();
+
+ out << "::std::memset(&hidl_u, 0, sizeof(hidl_u));\n";
+
+ // union itself is zero'd when set
+ // padding after descriminator
+ size_t dpad = layout.innerStruct.offset - layout.discriminator.size;
+ emitPaddingZero(out, layout.discriminator.size /*offset*/, dpad /*size*/);
+
+ size_t innerStructEnd = layout.innerStruct.offset + layout.innerStruct.size;
+ // final padding of the struct
+ size_t fpad = layout.overall.size - innerStructEnd;
+ emitPaddingZero(out, innerStructEnd /*offset*/, fpad /*size*/);
+
out.endl();
CHECK(!mFields->empty());
@@ -1084,23 +1097,15 @@
}).endl().endl();
// Move constructor
- out << fullName()
- << "::"
- << localName()
- << "("
- << localName()
- << "&& other) ";
+ out << fullName() << "::" << localName() << "(" << localName() << "&& other) : " << fullName()
+ << "() ";
emitSafeUnionCopyAndAssignDefinition(
out, "other", true /* isCopyConstructor */, true /* usesMoveSemantics */);
// Copy constructor
- out << fullName()
- << "::"
- << localName()
- << "(const "
- << localName()
- << "& other) ";
+ out << fullName() << "::" << localName() << "(const " << localName()
+ << "& other) : " << fullName() << "() ";
emitSafeUnionCopyAndAssignDefinition(
out, "other", true /* isCopyConstructor */, false /* usesMoveSemantics */);
@@ -2015,20 +2020,35 @@
innerStruct.offset += Layout::getPad(innerStruct.offset,
innerStruct.align);
- overall.size = innerStruct.offset + innerStruct.size;
-
// An empty struct/union still occupies a byte of space in C++.
- if (overall.size == 0) {
- overall.size = 1;
+ if (innerStruct.size == 0) {
+ innerStruct.size = 1;
}
+ overall.size = innerStruct.offset + innerStruct.size;
+
// Pad the overall structure's size
overall.align = std::max(innerStruct.align, discriminator.align);
overall.size += Layout::getPad(overall.size, overall.align);
+ if (mStyle != STYLE_SAFE_UNION) {
+ CHECK(overall.offset == innerStruct.offset) << overall.offset << " " << innerStruct.offset;
+ CHECK(overall.align == innerStruct.align) << overall.align << " " << innerStruct.align;
+ CHECK(overall.size == innerStruct.size) << overall.size << " " << innerStruct.size;
+ }
+
return compoundLayout;
}
+void CompoundType::emitPaddingZero(Formatter& out, size_t offset, size_t size) const {
+ if (size > 0) {
+ out << "::std::memset(reinterpret_cast<uint8_t*>(this) + " << offset << ", 0, " << size
+ << ");\n";
+ } else {
+ out << "// no padding to zero starting at offset " << offset << "\n";
+ }
+}
+
std::unique_ptr<ScalarType> CompoundType::getUnionDiscriminatorType() const {
static const std::vector<std::pair<int, ScalarType::Kind> > scalars {
{8, ScalarType::Kind::KIND_UINT8},
diff --git a/CompoundType.h b/CompoundType.h
index 2c5a055..a9e0f75 100644
--- a/CompoundType.h
+++ b/CompoundType.h
@@ -135,8 +135,12 @@
};
struct CompoundLayout {
+ // Layout of this entire object including metadata.
+ // For struct/union, this is the same as innerStruct.
Layout overall;
+ // Layout of user-specified data
Layout innerStruct;
+ // Layout of discriminator for safe union (otherwise zero)
Layout discriminator;
};
@@ -163,6 +167,7 @@
bool usesMoveSemantics) const;
CompoundLayout getCompoundAlignmentAndSize() const;
+ void emitPaddingZero(Formatter& out, size_t offset, size_t size) const;
void emitSafeUnionReaderWriterForInterfaces(
Formatter &out,
diff --git a/Interface.cpp b/Interface.cpp
index ff33972..92aaa62 100644
--- a/Interface.cpp
+++ b/Interface.cpp
@@ -385,23 +385,23 @@
{IMPL_INTERFACE,
[](auto &out) {
// getDebugInfo returns N/A for local objects.
- out << "_hidl_cb({ -1 /* pid */, 0 /* ptr */, \n"
- << sArch
- << "});\n"
- << "return ::android::hardware::Void();\n";
+ out << "::android::hidl::base::V1_0::DebugInfo info = {};\n";
+ out << "info.pid = -1;\n";
+ out << "info.ptr = 0;\n";
+ out << "info.arch = \n" << sArch << ";\n";
+ out << "_hidl_cb(info);\n";
+ out << "return ::android::hardware::Void();\n";
}
},
{IMPL_STUB_IMPL,
[](auto &out) {
- out << "_hidl_cb(";
- out.block([&] {
- out << "::android::hardware::details::getPidIfSharable(),\n"
- << "::android::hardware::details::debuggable()"
- << "? reinterpret_cast<uint64_t>(this) : 0 /* ptr */,\n"
- << sArch << "\n";
- });
- out << ");\n"
- << "return ::android::hardware::Void();\n";
+ out << "::android::hidl::base::V1_0::DebugInfo info = {};\n";
+ out << "info.pid = ::android::hardware::details::getPidIfSharable();\n";
+ out << "info.ptr = ::android::hardware::details::debuggable()"
+ << "? reinterpret_cast<uint64_t>(this) : 0;\n";
+ out << "info.arch = \n" << sArch << ";\n";
+ out << "_hidl_cb(info);\n";
+ out << "return ::android::hardware::Void();\n";
}
}
}, /* cppImpl */
diff --git a/build/hidl_interface.go b/build/hidl_interface.go
index 6fe9f8b..9b8b76d 100644
--- a/build/hidl_interface.go
+++ b/build/hidl_interface.go
@@ -639,16 +639,12 @@
Generated_headers: []string{name.headersName()},
Shared_libs: concat(cppDependencies, []string{
"libhidlbase",
- "libhidltransport",
- "libhwbinder",
"liblog",
"libutils",
"libcutils",
}),
Export_shared_lib_headers: concat(cppDependencies, []string{
"libhidlbase",
- "libhidltransport",
- "libhwbinder",
"libutils",
}),
Export_generated_headers: []string{name.headersName()},
@@ -740,8 +736,6 @@
"libbase",
"libcutils",
"libhidlbase",
- "libhidltransport",
- "libhwbinder",
"liblog",
"libutils",
},
@@ -750,7 +744,6 @@
}, wrap("", dependencies, "-adapter-helper"), cppDependencies, libraryIfExists),
Export_shared_lib_headers: []string{
"libhidlbase",
- "libhidltransport",
},
Export_static_lib_headers: concat([]string{
"libhidladapter",
@@ -775,8 +768,6 @@
"libbase",
"libcutils",
"libhidlbase",
- "libhidltransport",
- "libhwbinder",
"liblog",
"libutils",
},
@@ -921,11 +912,13 @@
}
var doubleLoadablePackageNames = []string{
+ "android.frameworks.bufferhub@1.0",
"android.hardware.cas@1.0",
"android.hardware.cas.native@1.0",
"android.hardware.configstore@",
"android.hardware.drm@1.0",
"android.hardware.drm@1.1",
+ "android.hardware.drm@1.2",
"android.hardware.graphics.allocator@",
"android.hardware.graphics.bufferqueue@",
"android.hardware.media@",
@@ -946,7 +939,7 @@
return false
}
-// packages in libhidltransport
+// packages in libhidlbase
var coreDependencyPackageNames = []string{
"android.hidl.base@",
"android.hidl.manager@",
diff --git a/c2hal/test/Android.bp b/c2hal/test/Android.bp
index 104d405..722d1a8 100644
--- a/c2hal/test/Android.bp
+++ b/c2hal/test/Android.bp
@@ -73,8 +73,6 @@
export_generated_headers: ["c2hal_test_genc++_headers"],
shared_libs: [
"libhidlbase",
- "libhidltransport",
- "libhwbinder",
"liblog",
"libutils",
"libcutils",
diff --git a/main.cpp b/main.cpp
index 2a13fff..ab8380c 100644
--- a/main.cpp
+++ b/main.cpp
@@ -465,7 +465,7 @@
bool isSystemProcessSupportedPackage(const FQName& fqName) {
// Technically, so is hidl IBase + IServiceManager, but
- // these are part of libhidltransport.
+ // these are part of libhidlbase.
return fqName.inPackage("android.hardware.graphics.common") ||
fqName.inPackage("android.hardware.graphics.mapper") ||
fqName.string() == "android.hardware.renderscript@1.0" ||
@@ -727,7 +727,6 @@
<< "shared_libs: [\n";
out.indent([&] {
out << "\"libhidlbase\",\n"
- << "\"libhidltransport\",\n"
<< "\"libutils\",\n"
<< "\"" << makeLibraryName(packageFQName) << "\",\n";
diff --git a/test/hidl_test/Android.bp b/test/hidl_test/Android.bp
index 63cce42..58a13f6 100644
--- a/test/hidl_test/Android.bp
+++ b/test/hidl_test/Android.bp
@@ -11,7 +11,6 @@
"libhidlbase",
"libhidlmemory",
"libhidltransport",
- "libhwbinder",
"liblog",
"libutils",
],
diff --git a/test/hidl_test/hidl_test_client.cpp b/test/hidl_test/hidl_test_client.cpp
index ac28d22..9f0ec39 100644
--- a/test/hidl_test/hidl_test_client.cpp
+++ b/test/hidl_test/hidl_test_client.cpp
@@ -1689,6 +1689,9 @@
//do nothing, this is expected
}
+ // further calls fail
+ EXPECT_FAIL(dyingBaz->ping());
+
std::unique_lock<std::mutex> lock(recipient->mutex);
recipient->condition.wait_for(lock, std::chrono::milliseconds(100), [&recipient]() {
return recipient->fired;
@@ -1849,31 +1852,44 @@
TEST_F(HidlTest, InvalidTransactionTest) {
using ::android::hardware::tests::bar::V1_0::BnHwBar;
- using ::android::hardware::tests::bar::V1_0::BpHwBar;
using ::android::hardware::IBinder;
using ::android::hardware::Parcel;
- using ::android::status_t;
- using ::android::OK;
+
+ sp<IBinder> binder = ::android::hardware::toBinder(bar);
Parcel request, reply;
- sp<IBinder> binder;
- status_t status = request.writeInterfaceToken(::android::hardware::tests::bar::V1_0::IBar::descriptor);
+ EXPECT_EQ(::android::OK, request.writeInterfaceToken(IBar::descriptor));
+ EXPECT_EQ(::android::UNKNOWN_TRANSACTION, binder->transact(1234, request, &reply));
- EXPECT_EQ(status, OK);
+ EXPECT_OK(bar->ping()); // still works
+}
- if (mode == BINDERIZED) {
- EXPECT_TRUE(bar->isRemote());
- binder = ::android::hardware::toBinder<IBar>(bar);
- } else {
- // For a local test, just wrap the implementation with a BnHwBar
- binder = new BnHwBar(bar);
- }
+TEST_F(HidlTest, EmptyTransactionTest) {
+ using ::android::hardware::IBinder;
+ using ::android::hardware::Parcel;
+ using ::android::hardware::tests::bar::V1_0::BnHwBar;
- status = binder->transact(1234, request, &reply);
+ sp<IBinder> binder = ::android::hardware::toBinder(bar);
- EXPECT_EQ(status, ::android::UNKNOWN_TRANSACTION);
- // Try another call, to make sure nothing is messed up
- EXPECT_OK(bar->thisIsNew());
+ Parcel request, reply;
+ EXPECT_EQ(::android::BAD_TYPE, binder->transact(2 /*someBoolMethod*/, request, &reply));
+
+ EXPECT_OK(bar->ping()); // still works
+}
+
+TEST_F(HidlTest, WrongDescriptorTest) {
+ using ::android::hardware::IBinder;
+ using ::android::hardware::Parcel;
+ using ::android::hardware::tests::bar::V1_0::BnHwBar;
+
+ sp<IBinder> binder = ::android::hardware::toBinder(bar);
+
+ Parcel request, reply;
+ // wrong descriptor
+ EXPECT_EQ(::android::OK, request.writeInterfaceToken("not a real descriptor"));
+ EXPECT_EQ(::android::BAD_TYPE, binder->transact(2 /*someBoolMethod*/, request, &reply));
+
+ EXPECT_OK(bar->ping()); // still works
}
TEST_F(HidlTest, TwowayMethodOnewayEnabledTest) {
@@ -2066,6 +2082,49 @@
}));
}
+template <typename T>
+void testZeroInit(const std::string& header) {
+ uint8_t buf[sizeof(T)];
+ memset(buf, 0xFF, sizeof(buf));
+
+ T* t = new (buf) T;
+
+ for (size_t i = 0; i < sizeof(T); i++) {
+ EXPECT_EQ(0, buf[i]) << header << " at offset: " << i;
+ }
+
+ t->~T();
+ t = nullptr;
+
+ memset(buf, 0xFF, sizeof(buf));
+ t = new (buf) T(T()); // copy constructor
+
+ for (size_t i = 0; i < sizeof(T); i++) {
+ EXPECT_EQ(0, buf[i]) << header << " at offset: " << i;
+ }
+
+ t->~T();
+ t = nullptr;
+
+ memset(buf, 0xFF, sizeof(buf));
+ const T aT = T();
+ t = new (buf) T(std::move(aT)); // move constructor
+
+ for (size_t i = 0; i < sizeof(T); i++) {
+ EXPECT_EQ(0, buf[i]) << header << " at offset: " << i;
+ }
+
+ t->~T();
+ t = nullptr;
+}
+
+TEST_F(HidlTest, SafeUnionUninit) {
+ testZeroInit<SmallSafeUnion>("SmallSafeUnion");
+ testZeroInit<LargeSafeUnion>("LargeSafeUnion");
+ testZeroInit<InterfaceTypeSafeUnion>("InterfaceTypeSafeUnion");
+ testZeroInit<HandleTypeSafeUnion>("HandleTypeSafeUnion");
+}
+
TEST_F(HidlTest, SafeUnionMoveConstructorTest) {
sp<IOtherInterface> otherInterface = new OtherInterface();
ASSERT_EQ(1, otherInterface->getStrongCount());
diff --git a/test/java_test/Android.bp b/test/java_test/Android.bp
index 4f18adf..87654aa 100644
--- a/test/java_test/Android.bp
+++ b/test/java_test/Android.bp
@@ -11,8 +11,6 @@
"libbase",
"libcutils",
"libhidlbase",
- "libhidltransport",
- "libhwbinder",
"liblog",
"libutils",
],
diff --git a/test/lazy_test/Android.bp b/test/lazy_test/Android.bp
index 8764b30..04acf6f 100644
--- a/test/lazy_test/Android.bp
+++ b/test/lazy_test/Android.bp
@@ -7,9 +7,7 @@
"libbase",
"liblog",
"libhidlbase",
- "libhidltransport",
"libhidlmemory",
- "libhwbinder",
"libutils",
],
}
diff --git a/test/lazy_test/main.cpp b/test/lazy_test/main.cpp
index 7126e85..48512e0 100644
--- a/test/lazy_test/main.cpp
+++ b/test/lazy_test/main.cpp
@@ -23,6 +23,7 @@
#include <unistd.h>
+#include <android/hidl/manager/1.2/IServiceManager.h>
#include <gtest/gtest.h>
#include <hidl/HidlSupport.h>
#include <hidl/HidlTransportSupport.h>
@@ -30,8 +31,11 @@
#include <hwbinder/IPCThreadState.h>
using ::android::sp;
+using ::android::hardware::hidl_string;
+using ::android::hardware::hidl_vec;
using ::android::hardware::IPCThreadState;
using ::android::hidl::base::V1_0::IBase;
+using ::android::hidl::manager::V1_2::IServiceManager;
static std::string gDescriptor;
static std::string gInstance;
@@ -41,12 +45,52 @@
true /*retry*/, false /*getStub*/);
}
+class HidlLazyTest : public ::testing::Test {
+ protected:
+ sp<IServiceManager> manager;
+
+ void SetUp() override {
+ manager = IServiceManager::getService();
+ ASSERT_NE(manager, nullptr);
+
+ ASSERT_FALSE(isServiceRunning()) << "Service '" << gDescriptor << "/" << gInstance
+ << "' is already running. Please ensure this "
+ << "service is implemented as a lazy HAL, then kill all "
+ << "clients of this service and try again.";
+ }
+
+ static constexpr size_t SHUTDOWN_WAIT_TIME = 10;
+ void TearDown() override {
+ std::cout << "Waiting " << SHUTDOWN_WAIT_TIME << " seconds before checking that the "
+ << "service has shut down." << std::endl;
+ IPCThreadState::self()->flushCommands();
+ sleep(SHUTDOWN_WAIT_TIME);
+ ASSERT_FALSE(isServiceRunning()) << "Service failed to shutdown.";
+ }
+
+ bool isServiceRunning() {
+ bool isRunning = false;
+ EXPECT_TRUE(
+ manager->listByInterface(gDescriptor,
+ [&isRunning](const hidl_vec<hidl_string>& instanceNames) {
+ for (const hidl_string& name : instanceNames) {
+ if (name == gInstance) {
+ isRunning = true;
+ break;
+ }
+ }
+ })
+ .isOk());
+ return isRunning;
+ }
+};
+
static constexpr size_t NUM_IMMEDIATE_GET_UNGETS = 100;
-TEST(LazyHidl, GetUnget) {
+TEST_F(HidlLazyTest, GetUnget) {
for (size_t i = 0; i < NUM_IMMEDIATE_GET_UNGETS; i++) {
IPCThreadState::self()->flushCommands();
sp<IBase> hal = getHal();
- ASSERT_NE(nullptr, hal.get());
+ ASSERT_NE(hal.get(), nullptr);
EXPECT_TRUE(hal->ping().isOk());
}
}
@@ -69,7 +113,7 @@
std::cout << "Thread waiting " << sleepTime << " while not holding HAL." << std::endl;
sleep(sleepTime);
sp<IBase> hal = getHal();
- ASSERT_NE(nullptr, hal.get());
+ ASSERT_NE(hal.get(), nullptr);
ASSERT_TRUE(hal->ping().isOk());
}
}
@@ -77,7 +121,7 @@
static constexpr size_t NUM_TIMES_GET_UNGET = 5;
static constexpr size_t MAX_WAITING_DURATION = 10;
static constexpr size_t NUM_CONCURRENT_THREADS = 5;
-TEST(LazyHidl, GetWithWaitConcurrent) {
+TEST_F(HidlLazyTest, GetWithWaitConcurrent) {
std::vector<std::vector<size_t>> threadWaitTimes(NUM_CONCURRENT_THREADS);
for (size_t i = 0; i < threadWaitTimes.size(); i++) {
@@ -108,4 +152,4 @@
gInstance = argv[2];
return RUN_ALL_TESTS();
-}
\ No newline at end of file
+}