Skip to content

Commit eb4744e

Browse files
dbortfacebook-github-bot
authored andcommitted
Avoid fatal checks in populate_operator_name (#1511)
Summary: Pull Request resolved: #1511 Make `populate_operator_name()` return an error so we can avoid crashing when there's a problem. Also ensure that the `overload` field is present before using it. Reviewed By: larryliu0820 Differential Revision: D52451742 fbshipit-source-id: 85a5beb5e98db67534883fef36b9103d02a7b852
1 parent e089910 commit eb4744e

File tree

1 file changed

+21
-11
lines changed

1 file changed

+21
-11
lines changed

runtime/executor/method.cpp

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -413,37 +413,44 @@ Error Method::parse_values() {
413413
return Error::Ok;
414414
}
415415

416+
namespace {
416417
/**
417418
* Private/helper method for populating operator_name from the Operator.
418419
* operator_name is a char pointer that is already allocated. The size of
419420
* of this buffer is of size operator_name_size.
420421
*/
421-
static void populateOperatorName(
422+
Error populate_operator_name(
422423
const executorch_flatbuffer::Operator* const& op,
423424
const size_t operator_name_size,
424425
char* operator_name) {
425-
int cx;
426-
const bool has_overload = (op->overload()->size() > 0);
427-
// Don't append any overload if the overload string is empty.
428-
cx = snprintf(
426+
const bool has_overload =
427+
op->overload() != nullptr && op->overload()->size() > 0;
428+
429+
ET_CHECK_OR_RETURN_ERROR(
430+
op->name() != nullptr, InvalidProgram, "Missing operator name");
431+
int cx = snprintf(
429432
operator_name,
430433
operator_name_size,
431434
"%s%s%s",
432435
op->name()->c_str(),
436+
// Don't append any overload if the overload string is empty.
433437
has_overload ? "." : "",
434438
has_overload ? op->overload()->c_str() : "");
435-
436-
ET_CHECK_MSG(cx >= 0, "String encoding error occured.");
437-
ET_CHECK_MSG(
439+
ET_CHECK_OR_RETURN_ERROR(cx >= 0, Internal, "snprintf failed: %d", cx);
440+
ET_CHECK_OR_RETURN_ERROR(
438441
cx < operator_name_size,
439-
"Aborting. Operator name %s%s%s with length %d "
442+
Internal,
443+
"Operator name %s%s%s with length %d "
440444
"truncated to %zu due to internal buffer limit.",
441445
op->name()->c_str(),
442446
has_overload ? "." : "",
443447
has_overload ? op->overload()->c_str() : "",
444448
cx,
445449
operator_name_size);
450+
451+
return Error::Ok;
446452
}
453+
} // namespace
447454

448455
Error Method::resolve_operator(
449456
int32_t op_index,
@@ -465,7 +472,10 @@ Error Method::resolve_operator(
465472
op_index);
466473
const auto& op = ops->Get(op_index);
467474

468-
populateOperatorName(op, kTempBufferSizeForName, operator_name);
475+
Error err = populate_operator_name(op, kTempBufferSizeForName, operator_name);
476+
if (err != Error::Ok) {
477+
return err;
478+
}
469479

470480
// resolve tensor meta
471481
auto method_allocator = memory_manager_->method_allocator();
@@ -481,7 +491,7 @@ Error Method::resolve_operator(
481491
exec_aten::DimOrderType* dim_order_ptr = ET_ALLOCATE_LIST_OR_RETURN_ERROR(
482492
method_allocator, exec_aten::DimOrderType, tensor.dim());
483493
size_t size = tensor.dim();
484-
Error err = get_dim_order(tensor, dim_order_ptr, size);
494+
err = get_dim_order(tensor, dim_order_ptr, size);
485495
ET_CHECK_OR_RETURN_ERROR(
486496
err == Error::Ok,
487497
InvalidArgument,

0 commit comments

Comments
 (0)