diff options
author | Slava Shklyaev <slavash@google.com> | 2020-06-18 18:47:06 +0100 |
---|---|---|
committer | Slava Shklyaev <slavash@google.com> | 2020-06-25 16:37:52 +0100 |
commit | 8088ca02e388d6df7f75975062ddd2a6bdebc07c (patch) | |
tree | ac725b265133be895e28034bb97b7ff3d524b48b /nn | |
parent | eb1372a96a73149d63a7cac3fe0ebd2f3276876e (diff) | |
download | ml-8088ca02e388d6df7f75975062ddd2a6bdebc07c.tar.gz |
Add HAL-level validation for CF operands of unknown size
At the NDK level, we allow IF and WHILE operations where an inner or
outer input or output operand has a type that is not fully specified.
However, this is not allowed At the HAL level. This CL adds HAL-level
validation.
See http://b/132458982#comment63
Bug: 132458982
Test: NNT_static
Change-Id: I54754d6241a1f8eb99717899ffd4f0ace4750060
Merged-In: I54754d6241a1f8eb99717899ffd4f0ace4750060
(cherry picked from commit 0fda6bb19a6fa83fe512dd6889b71e3ab37709bc)
Diffstat (limited to 'nn')
-rw-r--r-- | nn/common/Utils.cpp | 15 | ||||
-rw-r--r-- | nn/common/ValidateHal.cpp | 25 | ||||
-rw-r--r-- | nn/common/include/Utils.h | 3 | ||||
-rw-r--r-- | nn/common/include/ValidateHal.h | 6 | ||||
-rw-r--r-- | nn/runtime/ExecutionPlan.cpp | 5 | ||||
-rw-r--r-- | nn/runtime/Manager.cpp | 4 | ||||
-rw-r--r-- | nn/runtime/ModelBuilder.cpp | 17 |
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 75604da2b..045aa8ea4 100644 --- a/nn/common/include/Utils.h +++ b/nn/common/include/Utils.h @@ -395,6 +395,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 39af8aefc..da0c003f2 100644 --- a/nn/runtime/ExecutionPlan.cpp +++ b/nn/runtime/ExecutionPlan.cpp @@ -554,6 +554,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 2bc73f465..6b80d208d 100644 --- a/nn/runtime/Manager.cpp +++ b/nn/runtime/Manager.cpp @@ -656,8 +656,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; |