Merge "Add HAL-level validation for CF operands of unknown size" into rvc-dev am: 4bc8563265

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/ml/+/11913564

Change-Id: I7df54cc5913831e4aba03270fe8fe0acc9fd2955

7 files changed, 54 insertions, 21 deletions

diff --git a/nn/common/Utils.cpp b/nn/common/Utils.cpp
index 188436f11..1694c9c23 100644
--- a/nn/common/Utils.cpp
+++ b/nn/common/Utils.cpp
@@ -762,6 +762,15 @@ static bool validateIfOperation(uint32_t inputCount, const uint32_t* inputs, uin
     return true;
 }
 
+static bool validateControlFlowOperandUnknownSize(const SubgraphValidationHelper& helper,
+                                                  const Operand& operand) {
+    if (!helper.allowControlFlowOperationWithOperandOfUnknownSize &&
+        !isExtensionOperandType(operand.type)) {
+        NN_RET_CHECK_NE(nonExtensionOperandSizeOfData(operand.type, operand.dimensions), 0u);
+    }
+    return true;
+}
+
 static bool validateWhileOperation(uint32_t inputCount, const uint32_t* inputs,
                                    uint32_t outputCount, const uint32_t* outputs,
                                    const std::vector<Operand>& operands,
@@ -789,6 +798,8 @@ static bool validateWhileOperation(uint32_t inputCount, const uint32_t* inputs,
             const Operand& innerOperand = *helper.getSubgraphInputOperand(condModelOperand, i);
             const Operand& outerOperand = operands[inputs[op::kFirstInput + i]];
             NN_RET_CHECK(compatible(innerOperand, outerOperand));
+            NN_RET_CHECK(validateControlFlowOperandUnknownSize(helper, innerOperand));
+            NN_RET_CHECK(validateControlFlowOperandUnknownSize(helper, outerOperand));
         }
         NN_RET_CHECK(
                 validateConditionOperand(*helper.getSubgraphOutputOperand(condModelOperand, 0)));
@@ -809,16 +820,20 @@ static bool validateWhileOperation(uint32_t inputCount, const uint32_t* inputs,
             const Operand& innerOperand = *helper.getSubgraphInputOperand(bodyModelOperand, i);
             const Operand& outerOperand = operands[inputs[op::kFirstInput + i]];
             NN_RET_CHECK(compatible(innerOperand, outerOperand));
+            NN_RET_CHECK(validateControlFlowOperandUnknownSize(helper, innerOperand));
+            NN_RET_CHECK(validateControlFlowOperandUnknownSize(helper, outerOperand));
         }
         for (uint32_t i = 0; i < inputOutputCount; ++i) {
             const Operand& innerOperand = *helper.getSubgraphOutputOperand(bodyModelOperand, i);
             const Operand& outerOperand = operands[outputs[i]];
             NN_RET_CHECK(compatible(innerOperand, outerOperand));
+            NN_RET_CHECK(validateControlFlowOperandUnknownSize(helper, outerOperand));
         }
         for (uint32_t i = 0, n = inputOutputCount + stateOnlyCount; i < n; ++i) {
             const Operand& inputOperand = *helper.getSubgraphInputOperand(bodyModelOperand, i);
             const Operand& outputOperand = *helper.getSubgraphOutputOperand(bodyModelOperand, i);
             NN_RET_CHECK(compatible(inputOperand, outputOperand));
+            NN_RET_CHECK(validateControlFlowOperandUnknownSize(helper, outputOperand));
         }
         return true;
     };
diff --git a/nn/common/ValidateHal.cpp b/nn/common/ValidateHal.cpp
index 5668ff5ab..e5b9b12a8 100644
--- a/nn/common/ValidateHal.cpp
+++ b/nn/common/ValidateHal.cpp
@@ -448,7 +448,7 @@ static HalVersion getHalVersion(const V1_3::Operation&) {
 template <typename VersionedOperation>
 static bool validateOperations(const hidl_vec<VersionedOperation>& operations,
                                const hidl_vec<Operand>& operands,
-                               const hidl_vec<Subgraph>& subgraphs) {
+                               const hidl_vec<Subgraph>& subgraphs, ValidationMode mode) {
     auto isValidSubgraphReference = [&subgraphs](const Operand& modelOperand) -> bool {
         NN_RET_CHECK(modelOperand.type == OperandType::SUBGRAPH)
                 << "Unexpected operand type: " << toString(modelOperand.type);
@@ -490,7 +490,11 @@ static bool validateOperations(const hidl_vec<VersionedOperation>& operations,
                  .getSubgraphInputCount = getInputCount,
                  .getSubgraphOutputCount = getOutputCount,
                  .getSubgraphInputOperand = getInputOperand,
-                 .getSubgraphOutputOperand = getOutputOperand});
+                 .getSubgraphOutputOperand = getOutputOperand,
+                 // 1.3 HAL does not support CF operations with operands of
+                 // unknown size. See http://b/132458982#comment63.
+                 .allowControlFlowOperationWithOperandOfUnknownSize =
+                         mode == ValidationMode::RUNTIME});
         if (error != ANEURALNETWORKS_NO_ERROR) {
             LOG(ERROR) << "Invalid operation " << toString(op.type);
             return false;
@@ -687,7 +691,7 @@ static bool checkNoReferenceCycles(const V1_3::Model& model) {
 }
 
 template <class T_Model>
-bool validateModel(const T_Model& model) {
+bool validateModel(const T_Model& model, ValidationMode mode) {
     NNTRACE_FULL(NNTRACE_LAYER_UTILITY, NNTRACE_PHASE_UNSPECIFIED, "validateModel");
     HalVersion version = ModelToHalVersion<T_Model>::version;
     if (model.operations.size() == 0 || model.operands.size() == 0) {
@@ -699,7 +703,7 @@ bool validateModel(const T_Model& model) {
     const hidl_vec<Operand> latestVersionOperands = convertToV1_3(model.operands);
     return (validateOperands(model.operands, model.operandValues, model.pools, /*subgraphs=*/{},
                              /*allowUnspecifiedRank=*/version >= HalVersion::V1_2) &&
-            validateOperations(model.operations, latestVersionOperands, /*subgraphs=*/{}) &&
+            validateOperations(model.operations, latestVersionOperands, /*subgraphs=*/{}, mode) &&
             validateModelInputOutputs(model.inputIndexes, latestVersionOperands,
                                       OperandLifeTime::SUBGRAPH_INPUT) &&
             validateModelInputOutputs(model.outputIndexes, latestVersionOperands,
@@ -707,21 +711,22 @@ bool validateModel(const T_Model& model) {
             validatePools(model.pools, version) && validateGraph(model));
 }
 
-template bool validateModel<V1_0::Model>(const V1_0::Model& model);
-template bool validateModel<V1_1::Model>(const V1_1::Model& model);
-template bool validateModel<V1_2::Model>(const V1_2::Model& model);
+template bool validateModel<V1_0::Model>(const V1_0::Model& model, ValidationMode mode);
+template bool validateModel<V1_1::Model>(const V1_1::Model& model, ValidationMode mode);
+template bool validateModel<V1_2::Model>(const V1_2::Model& model, ValidationMode mode);
 
 template <>
-bool validateModel(const V1_3::Model& model) {
+bool validateModel(const V1_3::Model& model, ValidationMode mode) {
     NNTRACE_FULL(NNTRACE_LAYER_UTILITY, NNTRACE_PHASE_UNSPECIFIED, "validateModel");
     if (model.main.operations.size() == 0 || model.main.operands.size() == 0) {
         LOG(ERROR) << "Invalid empty model.";
         return false;
     }
-    auto validateSubgraph = [&model](const Subgraph& subgraph) -> bool {
+    auto validateSubgraph = [&model, mode](const Subgraph& subgraph) -> bool {
         return (validateOperands(subgraph.operands, model.operandValues, model.pools,
                                  model.referenced, /*allowUnspecifiedRank=*/true) &&
-                validateOperations(subgraph.operations, subgraph.operands, model.referenced) &&
+                validateOperations(subgraph.operations, subgraph.operands, model.referenced,
+                                   mode) &&
                 validateModelInputOutputs(subgraph.inputIndexes, subgraph.operands,
                                           OperandLifeTime::SUBGRAPH_INPUT) &&
                 validateModelInputOutputs(subgraph.outputIndexes, subgraph.operands,
diff --git a/nn/common/include/Utils.h b/nn/common/include/Utils.h
index ca11c5ebc..4bbbf8215 100644
--- a/nn/common/include/Utils.h
+++ b/nn/common/include/Utils.h
@@ -391,6 +391,9 @@ struct SubgraphValidationHelper {
     std::function<const hal::Operand*(const hal::Operand&, uint32_t)> getSubgraphInputOperand;
     // Gets the specified output operand of a subgraph referenced by a given operand.
     std::function<const hal::Operand*(const hal::Operand&, uint32_t)> getSubgraphOutputOperand;
+    // Whether control flow operations with inner or outer input or output
+    // operands of unknown size are allowed.
+    bool allowControlFlowOperationWithOperandOfUnknownSize;
 };
 
 // Returns ANEURALNETWORKS_NO_ERROR if the corresponding operation is defined and can handle the
diff --git a/nn/common/include/ValidateHal.h b/nn/common/include/ValidateHal.h
index 7b097fd20..32d7662ed 100644
--- a/nn/common/include/ValidateHal.h
+++ b/nn/common/include/ValidateHal.h
@@ -37,6 +37,10 @@ enum class HalVersion : int32_t {
 enum class IOType { INPUT, OUTPUT };
 using PreparedModelRole = std::tuple<const hal::IPreparedModel*, IOType, uint32_t>;
 
+// 1.3 HAL does not support control flow operations with operands of unknown size.
+// See http://b/132458982#comment63.
+enum class ValidationMode { DRIVER, RUNTIME };
+
 // Verifies that the model is valid, i.e. it is consistent, takes
 // only acceptable values, the constants don't extend outside the memory
 // regions they are part of, etc.
@@ -44,7 +48,7 @@ using PreparedModelRole = std::tuple<const hal::IPreparedModel*, IOType, uint32_
 // are correctly defined, as these are specific to each implementation.
 // Each driver should do their own validation of OEM types.
 template <class T_Model>
-bool validateModel(const T_Model& model);
+bool validateModel(const T_Model& model, ValidationMode mode = ValidationMode::DRIVER);
 
 // Verifies that the request for the given model is valid.
 // IMPORTANT: This function cannot validate that OEM operation and operands
diff --git a/nn/runtime/ExecutionPlan.cpp b/nn/runtime/ExecutionPlan.cpp
index 1f540805f..1797475ac 100644
--- a/nn/runtime/ExecutionPlan.cpp
+++ b/nn/runtime/ExecutionPlan.cpp
@@ -555,6 +555,11 @@ int ExecutionStep::finishStepModel(const ModelBuilder* mainModel, bool* hasOutpu
                    [](auto& e) { return e.second; });
     NN_RETURN_IF_ERROR(mStepModel.identifyInputsAndOutputs(inputs.size(), inputs.data(),
                                                            outputs.size(), outputs.data()));
+    // TODO: Model::finish() should use ValidationMode::RUNTIME when sending the
+    // step model to CpuDevice. Right now, this is harmless because the only
+    // difference in validation occurs with control flow operations and inputs
+    // or outputs of unknown size and we never send control flow operations to
+    // CpuDevice. We need to address this if this behavior changes (b/151634976).
     NN_RETURN_IF_ERROR(mStepModel.finish());
 
     // TODO: Move compilation elsewhere?
diff --git a/nn/runtime/Manager.cpp b/nn/runtime/Manager.cpp
index 634cd2aec..50d118000 100644
--- a/nn/runtime/Manager.cpp
+++ b/nn/runtime/Manager.cpp
@@ -657,8 +657,8 @@ std::pair<int, std::shared_ptr<PreparedModel>> CpuDevice::prepareModel(
             << "Should never call prepareModel with cache information on CpuDevice";
 
     const Model model = makeModel();
-    if (!validateModel(model) || !validateExecutionPreference(preference) ||
-        !validatePriority(priority)) {
+    if (!validateModel(model, ValidationMode::RUNTIME) ||
+        !validateExecutionPreference(preference) || !validatePriority(priority)) {
         return {ANEURALNETWORKS_OP_FAILED, nullptr};
     }
     if (hasDeadlinePassed(deadline)) {
diff --git a/nn/runtime/ModelBuilder.cpp b/nn/runtime/ModelBuilder.cpp
index 8669203de..73ec1af8c 100644
--- a/nn/runtime/ModelBuilder.cpp
+++ b/nn/runtime/ModelBuilder.cpp
@@ -388,13 +388,14 @@ int ModelBuilder::addOperation(ANeuralNetworksOperationType type, uint32_t input
     auto getOutputOperand = [this](const Operand& modelOperand, uint32_t index) -> const Operand* {
         return &getReferencedModel(modelOperand)->getOutputOperand(index);
     };
-    NN_RETURN_IF_ERROR(validateOperation(type, inputCount, inputs, outputCount, outputs, mOperands,
-                                         HalVersion::LATEST,
-                                         {.isValidSubgraphReference = isValidSubgraphReference,
-                                          .getSubgraphInputCount = getInputCount,
-                                          .getSubgraphOutputCount = getOutputCount,
-                                          .getSubgraphInputOperand = getInputOperand,
-                                          .getSubgraphOutputOperand = getOutputOperand}));
+    NN_RETURN_IF_ERROR(validateOperation(
+            type, inputCount, inputs, outputCount, outputs, mOperands, HalVersion::LATEST,
+            {.isValidSubgraphReference = isValidSubgraphReference,
+             .getSubgraphInputCount = getInputCount,
+             .getSubgraphOutputCount = getOutputCount,
+             .getSubgraphInputOperand = getInputOperand,
+             .getSubgraphOutputOperand = getOutputOperand,
+             .allowControlFlowOperationWithOperandOfUnknownSize = true}));
 
     uint32_t operationIndex = operationCount();
     if (operationIndex >= MAX_NUMBER_OF_OPERATIONS) {
@@ -523,7 +524,7 @@ int ModelBuilder::finish() {
     //       a CONSTANT_REFERENCE operand will not have correct .poolIndex, and
     //       validation will not work properly.
     const Model modelForValidation = makeHidlModel();
-    if (!validateModel(modelForValidation)) {
+    if (!validateModel(modelForValidation, ValidationMode::RUNTIME)) {
         LOG(ERROR) << "ANeuralNetworksModel_finish called on invalid model";
         mInvalidModel = true;
         return ANEURALNETWORKS_BAD_DATA;