Skip to content

Commit 2d5303b

Browse files
GregoryComerfacebook-github-bot
authored andcommitted
Check for null outputs in method (#12559)
Summary: When non-memory-planned outputs are unset, method execution will crash when writing to the output tensor. This manifests as a native crash with a deep stack trace that is both unrecoverable and unclear to the user. This PR adds a check in method.execute to detect null output tensor data pointers, and, if found, log a message and return an error code. Differential Revision: D78438716
1 parent 1333b36 commit 2d5303b

File tree

2 files changed

+55
-0
lines changed

2 files changed

+55
-0
lines changed

runtime/executor/method.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,6 +1540,28 @@ Error Method::execute() {
15401540
"Cannot execute until method has been initialized.");
15411541
ET_LOG(Debug, "Executing method: %s.", method_meta().name());
15421542

1543+
// Validate that outputs are set.
1544+
for (size_t i = 0; i < method_meta().num_outputs(); i++) {
1545+
auto& output = mutable_value(get_output_index(i));
1546+
if (!output.isTensor()) {
1547+
continue;
1548+
}
1549+
1550+
auto tensor_meta = method_meta().output_tensor_meta(i);
1551+
if (tensor_meta->is_memory_planned()) {
1552+
continue;
1553+
}
1554+
1555+
auto& t = output.toTensor();
1556+
if (t.const_data_ptr() == nullptr) {
1557+
ET_LOG(
1558+
Error,
1559+
"Output tensor at index %" ET_PRIsize_t " is not set.",
1560+
i);
1561+
return Error::InvalidState;
1562+
}
1563+
}
1564+
15431565
// Chains are executed sequentially today, but future async designs may
15441566
// branch and run many in parallel or out of order.
15451567
for (step_state_.chain_idx = 0; step_state_.chain_idx < n_chains_;

runtime/executor/test/method_test.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,3 +390,36 @@ TEST_F(MethodTest, OptionalTensorListDeserialization) {
390390
EXPECT_EQ(outputs.toTensor().size(2), 10);
391391
}
392392
*/
393+
394+
TEST_F(MethodTest, UnsetOutputTest) {
395+
// Validate that methods with non-memory planned outputs return an error when
396+
// the output data pointer is not set. It should not crash.
397+
398+
ManagedMemoryManager mmm(kDefaultNonConstMemBytes, kDefaultRuntimeMemBytes);
399+
Result<Method> method = programs_["cat"]->load_method("forward", &mmm.get());
400+
ASSERT_EQ(method.error(), Error::Ok);
401+
402+
constexpr int buffer_size = 8;
403+
float buffer[buffer_size];
404+
for (int i = 0; i < buffer_size; ++i) {
405+
buffer[i] = 0.f;
406+
}
407+
int32_t sizes[2] = {2, 4};
408+
uint8_t dim_order[2] = {0, 1};
409+
int32_t strides[2] = {4, 1};
410+
executorch::aten::TensorImpl impl(
411+
executorch::aten::ScalarType::Float,
412+
2,
413+
sizes,
414+
buffer,
415+
dim_order,
416+
strides);
417+
418+
auto input_err =
419+
method->set_input(EValue(executorch::aten::Tensor(&impl)), 0);
420+
ASSERT_EQ(input_err, Error::Ok);
421+
422+
// Execute the method once. It should return an error (not crash).
423+
auto execute_error = method->execute();
424+
ASSERT_EQ(execute_error, Error::InvalidState);
425+
}

0 commit comments

Comments
 (0)