Merge "Enforce Treble "wire" ABI by emitting static_asserts for the sizes of" into oc-dev
diff --git a/AST.cpp b/AST.cpp
index fb5d333..4793021 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -117,7 +117,8 @@
}
for (const auto &subFQName : packageInterfaces) {
- AST *ast = mCoordinator->parse(subFQName, &mImportedASTs);
+ // Do not enforce restrictions on imports.
+ AST *ast = mCoordinator->parse(subFQName, &mImportedASTs, false /* enforce */);
if (ast == nullptr) {
return false;
}
@@ -135,7 +136,8 @@
// assume it is an interface, and try to import it.
const FQName interfaceName = fqName.getTopLevelType();
- importAST = mCoordinator->parse(interfaceName, &mImportedASTs);
+ // Do not enforce restrictions on imports.
+ importAST = mCoordinator->parse(interfaceName, &mImportedASTs, false /* enforce */);
if (importAST != nullptr) {
// cases like android.hardware.foo@1.0::IFoo.Internal
@@ -163,7 +165,9 @@
// probably a type in types.hal, like android.hardware.foo@1.0::Abc.Internal
FQName typesFQName = fqName.getTypesForPackage();
- importAST = mCoordinator->parse(typesFQName, &mImportedASTs);
+
+ // Do not enforce restrictions on imports.
+ importAST = mCoordinator->parse(typesFQName, &mImportedASTs, false /* enforce */);
if (importAST != nullptr) {
// Attempt to find Abc.Internal in types.
@@ -522,7 +526,7 @@
void AST::getAllImportedNames(std::set<FQName> *allImportNames) const {
for (const auto& name : mImportedNames) {
allImportNames->insert(name);
- AST *ast = mCoordinator->parse(name);
+ AST *ast = mCoordinator->parse(name, nullptr /* imported */, false /* enforce */);
ast->getAllImportedNames(allImportNames);
}
}
diff --git a/Coordinator.cpp b/Coordinator.cpp
index d50b639..72c8557 100644
--- a/Coordinator.cpp
+++ b/Coordinator.cpp
@@ -53,7 +53,7 @@
// empty
}
-AST *Coordinator::parse(const FQName &fqName, std::set<AST *> *parsedASTs) {
+AST *Coordinator::parse(const FQName &fqName, std::set<AST *> *parsedASTs, bool enforce) {
CHECK(fqName.isFullyQualified());
auto it = mCache.find(fqName);
@@ -75,7 +75,8 @@
if (fqName.name() != "types") {
// Any interface file implicitly imports its package's types.hal.
FQName typesName = fqName.getTypesForPackage();
- typesAST = parse(typesName, parsedASTs);
+ // Do not enforce on imports.
+ typesAST = parse(typesName, parsedASTs, false /* enforce */);
// fall through.
}
@@ -163,13 +164,15 @@
// parse fqName.
mCache[fqName] = ast;
- // For each .hal file that hidl-gen parses, the whole package will be checked.
- err = enforceRestrictionsOnPackage(fqName);
- if (err != OK) {
- mCache[fqName] = nullptr;
- delete ast;
- ast = nullptr;
- return nullptr;
+ if (enforce) {
+ // For each .hal file that hidl-gen parses, the whole package will be checked.
+ err = enforceRestrictionsOnPackage(fqName);
+ if (err != OK) {
+ mCache[fqName] = nullptr;
+ delete ast;
+ ast = nullptr;
+ return nullptr;
+ }
}
return ast;
diff --git a/Coordinator.h b/Coordinator.h
index 0aefb40..ddf5dfa 100644
--- a/Coordinator.h
+++ b/Coordinator.h
@@ -44,7 +44,9 @@
// file if it exists.
// If "parsedASTs" is non-NULL, successfully parsed ASTs are inserted
// into the set.
- AST *parse(const FQName &fqName, std::set<AST *> *parsedASTs = nullptr);
+ // If !enforce, enforceRestrictionsOnPackage won't be run.
+ AST *parse(const FQName &fqName, std::set<AST *> *parsedASTs = nullptr,
+ bool enforce = true);
// Given package-root paths of ["hardware/interfaces",
// "vendor/<something>/interfaces"], package roots of
diff --git a/generateCpp.cpp b/generateCpp.cpp
index 36d205d..c686be2 100644
--- a/generateCpp.cpp
+++ b/generateCpp.cpp
@@ -169,13 +169,42 @@
<< "::android::sp<" << interfaceName << "> " << interfaceName << "::" << functionName << "("
<< "const std::string &serviceName, const bool getStub) ";
out.block([&] {
- out << "::android::sp<" << interfaceName << "> iface = nullptr;\n";
- out << "::android::vintf::Transport transport = ::android::hardware::getTransport("
- << interfaceName << "::descriptor, serviceName);\n";
+ out << "using ::android::hardware::defaultServiceManager;\n";
+ out << "using ::android::hardware::details::waitForHwService;\n";
+ out << "using ::android::hardware::getPassthroughServiceManager;\n";
+ out << "using ::android::hardware::Return;\n";
+ out << "using ::android::sp;\n";
+ out << "using Transport = ::android::hidl::manager::V1_0::IServiceManager::Transport;\n\n";
- out << "const bool vintfHwbinder = (transport == ::android::vintf::Transport::HWBINDER);\n"
- << "const bool vintfPassthru = (transport == ::android::vintf::Transport::PASSTHROUGH);\n"
- << "const bool vintfEmpty = (transport == ::android::vintf::Transport::EMPTY);\n\n";
+ out << "sp<" << interfaceName << "> iface = nullptr;\n";
+
+ out.endl();
+
+ out << "const sp<::android::hidl::manager::V1_0::IServiceManager> sm"
+ << " = defaultServiceManager();\n";
+
+ out.sIf("sm == nullptr", [&] {
+ // hwbinder is not available on this device, so future tries
+ // would also be null. I can only return nullptr.
+ out << "ALOGE(\"getService: defaultServiceManager() is null\");\n"
+ << "return nullptr;\n";
+ }).endl().endl();
+
+ out << "Return<Transport> transportRet = sm->getTransport("
+ << interfaceName << "::descriptor, serviceName);\n\n";
+
+ out.sIf("!transportRet.isOk()", [&] {
+ out << "ALOGE(\"getService: defaultServiceManager()->getTransport returns %s\", "
+ << "transportRet.description().c_str());\n";
+ out << "return nullptr;\n";
+ });
+
+ out.endl();
+
+ out << "Transport transport = transportRet;\n";
+ out << "const bool vintfHwbinder = (transport == Transport::HWBINDER);\n"
+ << "const bool vintfPassthru = (transport == Transport::PASSTHROUGH);\n"
+ << "const bool vintfEmpty = (transport == Transport::EMPTY);\n\n";
// if (getStub) {
// getPassthroughServiceManager()->get only once.
@@ -204,27 +233,15 @@
out << "tried = true;\n";
- out << "const ::android::sp<::android::hidl::manager::V1_0::IServiceManager> sm\n";
- out.indent(2, [&] {
- out << "= ::android::hardware::defaultServiceManager();\n";
- });
- out.sIf("sm == nullptr", [&] {
- // hwbinder is not available on this device, so future tries
- // would also be null. I can only "break" here and
- // (vintfEmpty) try passthrough or (vintfHwbinder) return nullptr.
- out << "ALOGE(\"getService: defaultServiceManager() is null\");\n"
- << "break;\n";
- }).endl();
if (!isTry) {
out.sIf("vintfHwbinder", [&] {
- out << "::android::hardware::details::waitForHwService("
- << interfaceName << "::descriptor" << ", serviceName);\n";
+ out << "waitForHwService("
+ << interfaceName << "::descriptor, serviceName);\n";
}).endl();
}
- out << "::android::hardware::Return<::android::sp<"
- << gIBaseFqName.cppName() << ">> ret = \n";
+ out << "Return<sp<" << gIBaseFqName.cppName() << ">> ret = \n";
out.indent(2, [&] {
out << "sm->get(" << interfaceName << "::descriptor, serviceName);\n";
});
@@ -236,7 +253,7 @@
<< "break;\n";
}).endl();
- out << "::android::sp<" << gIBaseFqName.cppName() << "> base = ret;\n";
+ out << "sp<" << gIBaseFqName.cppName() << "> base = ret;\n";
out.sIf("base == nullptr", [&] {
// race condition. hwservicemanager drops the service
// from waitForHwService to here
@@ -244,7 +261,7 @@
<< (isTry ? "break" : "continue")
<< ";\n";
}).endl();
- out << "::android::hardware::Return<::android::sp<" << interfaceName
+ out << "Return<sp<" << interfaceName
<< ">> castRet = " << interfaceName << "::castFrom(base, true /* emitError */);\n";
out.sIf("!castRet.isOk()", [&] {
out.sIf("castRet.isDeadObject()", [&] {
@@ -272,18 +289,16 @@
}).endl();
out.sIf("getStub || vintfPassthru || vintfEmpty", [&] {
- out << "const ::android::sp<::android::hidl::manager::V1_0::IServiceManager> pm\n";
- out.indent(2, [&] {
- out << "= ::android::hardware::getPassthroughServiceManager();\n";
- });
+ out << "const sp<::android::hidl::manager::V1_0::IServiceManager> pm"
+ << " = getPassthroughServiceManager();\n";
out.sIf("pm != nullptr", [&] () {
- out << "::android::hardware::Return<::android::sp<" << gIBaseFqName.cppName() << ">> ret = \n";
+ out << "Return<sp<" << gIBaseFqName.cppName() << ">> ret = \n";
out.indent(2, [&] {
out << "pm->get(" << interfaceName << "::descriptor" << ", serviceName);\n";
});
out.sIf("ret.isOk()", [&] {
- out << "::android::sp<" << gIBaseFqName.cppName()
+ out << "sp<" << gIBaseFqName.cppName()
<< "> baseInterface = ret;\n";
out.sIf("baseInterface != nullptr", [&]() {
out << "iface = new " << fqName.getInterfacePassthroughName()
@@ -308,6 +323,10 @@
out << "::android::status_t " << interfaceName << "::registerAsService("
<< "const std::string &serviceName) ";
out.block([&] {
+ out << "::android::hardware::details::onRegistration(\""
+ << fqName.getPackageAndVersion().string() << "\", \""
+ << interfaceName
+ << "\", serviceName);\n\n";
out << "const ::android::sp<::android::hidl::manager::V1_0::IServiceManager> sm\n";
out.indent(2, [&] {
out << "= ::android::hardware::defaultServiceManager();\n";
diff --git a/generateJava.cpp b/generateJava.cpp
index 6e5d91b..2db078d 100644
--- a/generateJava.cpp
+++ b/generateJava.cpp
@@ -160,7 +160,7 @@
<< ifaceName
<< "\";\n\n";
- out << "public static "
+ out << "/* package private */ static "
<< ifaceName
<< " asInterface(android.os.IHwBinder binder) {\n";
@@ -188,7 +188,37 @@
out.unindent();
out << "}\n\n";
- out << "return new " << ifaceName << ".Proxy(binder);\n";
+ out << ifaceName << " proxy = new " << ifaceName << ".Proxy(binder);\n\n";
+ out << "try {\n";
+ out.indent();
+ out << "for (String descriptor : proxy.interfaceChain()) {\n";
+ out.indent();
+ out << "if (descriptor.equals(kInterfaceName)) {\n";
+ out.indent();
+ out << "return proxy;\n";
+ out.unindent();
+ out << "}\n";
+ out.unindent();
+ out << "}\n";
+ out.unindent();
+ out << "} catch (android.os.RemoteException e) {\n";
+ out.indent();
+ out.unindent();
+ out << "}\n\n";
+
+ out << "return null;\n";
+
+ out.unindent();
+ out << "}\n\n";
+
+ out << "public static "
+ << ifaceName
+ << " castFrom(android.os.IHwInterface iface) {\n";
+ out.indent();
+
+ out << "return (iface == null) ? null : "
+ << ifaceName
+ << ".asInterface(iface.asBinder());\n";
out.unindent();
out << "}\n\n";
diff --git a/test/AndroidTest.xml b/test/AndroidTest.xml
index da8fe57..ef776c2 100644
--- a/test/AndroidTest.xml
+++ b/test/AndroidTest.xml
@@ -24,24 +24,32 @@
<option name="push" value="DATA/nativetest64/hidl_test_client/hidl_test_client->/data/nativetest64/hidl_test_client/hidl_test_client" />
<option name="push" value="DATA/nativetest64/hidl_test_helper->/data/nativetest64/hidl_test_helper" />
<option name="push" value="DATA/nativetest64/hidl_test->/data/nativetest64/hidl_test" />
- <option name="push" value="DATA/lib/android.hardware.tests.foo@1.0.so->/data/lib/android.hardware.tests.foo@1.0.so" />
- <option name="push" value="DATA/lib64/android.hardware.tests.foo@1.0.so->/data/lib64/android.hardware.tests.foo@1.0.so" />
- <option name="push" value="DATA/lib/android.hardware.tests.bar@1.0.so->/data/lib/android.hardware.tests.bar@1.0.so" />
- <option name="push" value="DATA/lib64/android.hardware.tests.bar@1.0.so->/data/lib64/android.hardware.tests.bar@1.0.so" />
- <option name="push" value="DATA/lib/android.hardware.tests.baz@1.0.so->/data/lib/android.hardware.tests.baz@1.0.so" />
- <option name="push" value="DATA/lib64/android.hardware.tests.baz@1.0.so->/data/lib64/android.hardware.tests.baz@1.0.so" />
- <option name="push" value="DATA/lib/android.hardware.tests.inheritance@1.0.so->/data/lib/android.hardware.tests.inheritance@1.0.so" />
- <option name="push" value="DATA/lib64/android.hardware.tests.inheritance@1.0.so->/data/lib64/android.hardware.tests.inheritance@1.0.so" />
- <option name="push" value="DATA/lib/android.hardware.tests.pointer@1.0.so->/data/lib/android.hardware.tests.pointer@1.0.so" />
- <option name="push" value="DATA/lib64/android.hardware.tests.pointer@1.0.so->/data/lib64/android.hardware.tests.pointer@1.0.so" />
- <option name="push" value="DATA/lib/android.hardware.tests.memory@1.0.so->/data/lib/android.hardware.tests.memory@1.0.so" />
- <option name="push" value="DATA/lib64/android.hardware.tests.memory@1.0.so->/data/lib64/android.hardware.tests.memory@1.0.so" />
+ <option name="push" value="DATA/lib/libfootest.so->/system/lib/libfootest.so" />
+ <option name="push" value="DATA/lib64/libfootest.so->/system/lib64/libfootest.so" />
+ <option name="push" value="DATA/lib/libpointertest.so->/system/lib/libpointertest.so" />
+ <option name="push" value="DATA/lib64/libpointertest.so->/system/lib64/libpointertest.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.foo@1.0.so->/system/lib/android.hardware.tests.foo@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.foo@1.0.so->/system/lib64/android.hardware.tests.foo@1.0.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.bar@1.0.so->/system/lib/android.hardware.tests.bar@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.bar@1.0.so->/system/lib64/android.hardware.tests.bar@1.0.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.baz@1.0.so->/system/lib/android.hardware.tests.baz@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.baz@1.0.so->/system/lib64/android.hardware.tests.baz@1.0.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.hash@1.0.so->/system/lib/android.hardware.tests.hash@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.hash@1.0.so->/system/lib64/android.hardware.tests.hash@1.0.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.inheritance@1.0.so->/system/lib/android.hardware.tests.inheritance@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.inheritance@1.0.so->/system/lib64/android.hardware.tests.inheritance@1.0.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.pointer@1.0.so->/system/lib/android.hardware.tests.pointer@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.pointer@1.0.so->/system/lib64/android.hardware.tests.pointer@1.0.so" />
+ <option name="push" value="DATA/lib/android.hardware.tests.memory@1.0.so->/system/lib/android.hardware.tests.memory@1.0.so" />
+ <option name="push" value="DATA/lib64/android.hardware.tests.memory@1.0.so->/system/lib64/android.hardware.tests.memory@1.0.so" />
<option name="push" value="DATA/vendor/lib/hw/android.hardware.tests.foo@1.0-impl.so->/vendor/lib/hw/android.hardware.tests.foo@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib64/hw/android.hardware.tests.foo@1.0-impl.so->/vendor/lib64/hw/android.hardware.tests.foo@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib/hw/android.hardware.tests.bar@1.0-impl.so->/vendor/lib/hw/android.hardware.tests.bar@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib64/hw/android.hardware.tests.bar@1.0-impl.so->/vendor/lib64/hw/android.hardware.tests.bar@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib/hw/android.hardware.tests.baz@1.0-impl.so->/vendor/lib/hw/android.hardware.tests.baz@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib64/hw/android.hardware.tests.baz@1.0-impl.so->/vendor/lib64/hw/android.hardware.tests.baz@1.0-impl.so" />
+ <option name="push" value="DATA/vendor/lib/hw/android.hardware.tests.hash@1.0-impl.so->/vendor/lib/hw/android.hardware.tests.hash@1.0-impl.so" />
+ <option name="push" value="DATA/vendor/lib64/hw/android.hardware.tests.hash@1.0-impl.so->/vendor/lib64/hw/android.hardware.tests.hash@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib/hw/android.hardware.tests.inheritance@1.0-impl.so->/vendor/lib/hw/android.hardware.tests.inheritance@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib64/hw/android.hardware.tests.inheritance@1.0-impl.so->/vendor/lib64/hw/android.hardware.tests.inheritance@1.0-impl.so" />
<option name="push" value="DATA/vendor/lib/hw/android.hardware.tests.pointer@1.0-impl.so->/vendor/lib/hw/android.hardware.tests.pointer@1.0-impl.so" />
diff --git a/test/hidl_test_client.cpp b/test/hidl_test_client.cpp
index a46368f..e12fae9 100644
--- a/test/hidl_test_client.cpp
+++ b/test/hidl_test_client.cpp
@@ -481,6 +481,17 @@
// statement can be written here.
}
+TEST_F(HidlTest, PassthroughLookupTest) {
+ // IFoo is special because it returns an interface no matter
+ // what instance name is requested. In general, this is BAD!
+ EXPECT_NE(nullptr, IFoo::getService("", true /* getStub */).get());
+ EXPECT_NE(nullptr, IFoo::getService("a", true /* getStub */).get());
+ EXPECT_NE(nullptr, IFoo::getService("asdf", true /* getStub */).get());
+ EXPECT_NE(nullptr, IFoo::getService("::::::::", true /* getStub */).get());
+ EXPECT_NE(nullptr, IFoo::getService("/////", true /* getStub */).get());
+ EXPECT_NE(nullptr, IFoo::getService("\n", true /* getStub */).get());
+}
+
TEST_F(HidlTest, EnumToStringTest) {
using namespace std::string_literals;
using ::android::hardware::tests::foo::V1_0::toString;
diff --git a/test/java_test/src/com/android/commands/hidl_test_java/HidlTestJava.java b/test/java_test/src/com/android/commands/hidl_test_java/HidlTestJava.java
index 87c4676..1d9ef54 100644
--- a/test/java_test/src/com/android/commands/hidl_test_java/HidlTestJava.java
+++ b/test/java_test/src/com/android/commands/hidl_test_java/HidlTestJava.java
@@ -18,6 +18,7 @@
import android.hardware.tests.baz.V1_0.IBase;
import android.hardware.tests.baz.V1_0.IBaz;
+import android.hardware.tests.baz.V1_0.IQuux;
import android.hardware.tests.baz.V1_0.IBaz.NestedStruct;
import android.hardware.tests.baz.V1_0.IBazCallback;
import android.os.HwBinder;
@@ -237,6 +238,14 @@
// Test access through base interface binder.
IBase baseProxy = IBase.getService("baz");
baseProxy.someBaseMethod();
+
+ IBaz bazProxy = IBaz.castFrom(baseProxy);
+ ExpectTrue(bazProxy != null);
+
+ // IQuux is completely unrelated to IBase/IBaz, so the following
+ // should fail, i.e. return null.
+ IQuux quuxProxy = IQuux.castFrom(baseProxy);
+ ExpectTrue(quuxProxy == null);
}
IBaz proxy = IBaz.getService("baz");