Hoist enforceInterface and escape onTransact
This change adds as check for user-defined transactions in order to call
enforceInterface before checking for different case in onTransact. It
also adds break statements to onTransact.
'system/tools/aidl/tests/golden_test.sh update' was used to update the
reference outputs.
Test: - builds and boots
- atest aidl_unittests CtsNdkBinderTestCases -m -c
- cd out/target/product/redfin/symbols/system/framework/arm64;
nm --size-sort -Sr boot-framework.oat | grep onTransact
- cd out/target/product/redfin/system/framework;
./apkanalyzer dex packages framework.jar | grep 'onTransact' |
sort -r -g -k 5,5 | awk '{$1=$2=$3=$4=""; print $0}')
Bug: 194712114
Change-Id: Ib13b0f694bd3bf151677abe98cdd3368c873beaf
diff --git a/generate_java_binder.cpp b/generate_java_binder.cpp
index fa1c14a..04887fc 100644
--- a/generate_java_binder.cpp
+++ b/generate_java_binder.cpp
@@ -207,8 +207,23 @@
std::vector<std::shared_ptr<Expression>>{this->transact_code, this->transact_data,
this->transact_reply, this->transact_flags});
default_case->statements->Add(std::make_shared<ReturnStatement>(superCall));
+
+ auto case_count = transact_switch_user->cases.size();
transact_switch_user->cases.push_back(default_case);
+ // Interface token validation is done for user-defined transactions.
+ if (case_count > 0) {
+ auto ifStatement = std::make_shared<IfStatement>();
+ ifStatement->expression = std::make_shared<LiteralExpression>(
+ "code >= android.os.IBinder.FIRST_CALL_TRANSACTION && "
+ "code <= android.os.IBinder.LAST_CALL_TRANSACTION");
+ ifStatement->statements = std::make_shared<StatementBlock>();
+ ifStatement->statements->Add(std::make_shared<MethodCall>(
+ this->transact_data, "enforceInterface",
+ std::vector<std::shared_ptr<Expression>>{this->get_transact_descriptor(nullptr)}));
+ transact_statements->Add(ifStatement);
+ }
+
// Meta transactions are looked up prior to user-defined transactions.
transact_statements->Add(this->transact_switch_meta);
transact_statements->Add(this->transact_switch_user);
@@ -222,6 +237,12 @@
code_switch_default_case->statements->Add(std::make_shared<ReturnStatement>(NULL_VALUE));
this->code_to_method_name_switch->cases.push_back(code_switch_default_case);
}
+
+ // There will be at least one statement for the default, but if we emit a
+ // return true after that default, it will be unreachable.
+ if (case_count > 0) {
+ transact_statements->Add(std::make_shared<ReturnStatement>(TRUE_VALUE));
+ }
}
// The the expression for the interface's descriptor to be used when
@@ -425,7 +446,7 @@
std::shared_ptr<Variable> transact_reply,
const AidlTypenames& typenames,
std::shared_ptr<StatementBlock> statement_block,
- std::shared_ptr<StubClass> stubClass, const Options& options) {
+ const Options& options) {
// try and finally
auto tryStatement = std::make_shared<TryStatement>();
auto finallyStatement = std::make_shared<FinallyStatement>();
@@ -449,11 +470,6 @@
auto realCall = std::make_shared<MethodCall>(THIS_VALUE, method.GetName());
- // interface token validation is the very first thing we do
- statements->Add(std::make_shared<MethodCall>(
- transact_data, "enforceInterface",
- std::vector<std::shared_ptr<Expression>>{stubClass->get_transact_descriptor(&method)}));
-
// args
VariableFactory stubArgs("_arg");
{
@@ -525,9 +541,6 @@
generate_write_to_parcel(arg->GetType(), statements, v, transact_reply, true, typenames);
}
}
-
- // return true
- statements->Add(std::make_shared<ReturnStatement>(TRUE_VALUE));
}
static void generate_stub_case(const AidlInterface& iface, const AidlMethod& method,
@@ -537,7 +550,8 @@
auto c = std::make_shared<Case>(transactCodeName);
generate_stub_code(iface, method, oneway, stubClass->transact_data, stubClass->transact_reply,
- typenames, c->statements, stubClass, options);
+ typenames, c->statements, options);
+ c->statements->Add(std::make_shared<BreakStatement>());
stubClass->transact_switch_user->cases.push_back(c);
}
@@ -562,7 +576,8 @@
stubClass->elements.push_back(onTransact_case);
generate_stub_code(iface, method, oneway, transact_data, transact_reply, typenames,
- onTransact_case->statements, stubClass, options);
+ onTransact_case->statements, options);
+ onTransact_case->statements->Add(std::make_shared<ReturnStatement>(TRUE_VALUE));
}
// Generate the case dispatch.
@@ -828,8 +843,7 @@
if (method.GetName() == kGetInterfaceVersion && options.Version() > 0) {
auto c = std::make_shared<Case>(transactCodeName);
std::ostringstream code;
- code << "data.enforceInterface(descriptor);\n"
- << "reply.writeNoException();\n"
+ code << "reply.writeNoException();\n"
<< "reply.writeInt(" << kGetInterfaceVersion << "());\n"
<< "return true;\n";
c->statements->Add(std::make_shared<LiteralStatement>(code.str()));
@@ -838,8 +852,7 @@
if (method.GetName() == kGetInterfaceHash && !options.Hash().empty()) {
auto c = std::make_shared<Case>(transactCodeName);
std::ostringstream code;
- code << "data.enforceInterface(descriptor);\n"
- << "reply.writeNoException();\n"
+ code << "reply.writeNoException();\n"
<< "reply.writeString(" << kGetInterfaceHash << "());\n"
<< "return true;\n";
c->statements->Add(std::make_shared<LiteralStatement>(code.str()));