Skip to content

Commit a6fe4b4

Browse files
committed
Add validate_tensor_layout to check a layout without aborting
make_tensor_ptr checks sizes, dim order and strides with ET_CHECK, so an invalid layout aborts the process: std::vector<float> data(16); auto t = make_tensor_ptr({2, 2}, data.data(), {}, {8, 2}); // invalid strides for dim 0: 8!= 2 while its size is 2 != 1 That is fine for a layout the program builds itself. But a program that gets the layout from outside, for example from a file or over the network, has no ExecuTorch function to reject a bad one and keep running. ExecuTorch has no exceptions, so it needs a check that returns an Error. (PyTorch accepts these strides, because its tensors do not have to be dense.) Add validate_tensor_layout(sizes, dim_order, strides). It returns Error::InvalidArgument for every layout that the make_tensor_ptr overloads taking sizes and data abort on, and also for sizes that are negative or whose product overflows: if (validate_tensor_layout(sizes, {}, strides) != Error::Ok) { return Error::InvalidArgument; } It does not look at the data, so the caller still has to check that the data size matches the sizes. The tensor guide shows both checks. The layout checks move into one internal function that make_tensor_ptr and validate_tensor_layout both call. It also computes the strides with an overflow check. Before, a stride that does not fit StridesType was computed with signed overflow, which is undefined behavior. For example, in portable mode make_tensor_ptr({0, 65536, 65536}, std::vector<float>{}) built a tensor with strides {0, 65536, 1}. Such a layout now aborts make_tensor_ptr, the way PyTorch throws "Stride calculation overflowed" when its own strides overflow. In portable mode this also covers layouts that used to pass even though a stride overflowed: - On 64-bit targets, every layout whose element count overflows. The raw pointer overload accepted these with a wrong element count. - Tensors with data whose only stride that does not fit belongs to a size-1 dim, which is never used to address the data. For example, make_tensor_ptr with sizes {1, 16384, 131072} (2^31 elements), or a clone of such a tensor, now aborts, while sizes {16384, 131072} with the same data still work. Every other layout is accepted or rejected as before. Only the abort message changes, to "Invalid tensor layout.", with the reason logged on the line before it. The WebAssembly bindings kept their own copy of these checks, so that Tensor.fromArray and Tensor.fromIter throw a JS error instead of aborting. They now call validate_tensor_layout, so an empty tensor whose strides overflow, such as sizes [0, 65536, 65536], now throws instead of being built. All their layout errors now throw "Invalid tensor layout.", also for negative or overflowing sizes, which used to throw "Invalid tensor sizes: numel computation failed". The reason is logged only in builds with logging enabled, which in CMake are Debug builds by default.
1 parent 01d6403 commit a6fe4b4

5 files changed

Lines changed: 184 additions & 61 deletions

File tree

‎docs/source/extension-tensor.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,25 @@ auto tensor = make_tensor_ptr(
160160

161161
The `TensorPtr` will call the custom deleter when it is destroyed, i.e., when the smart pointer is reset and no more references to the underlying `Tensor` exist.
162162

163+
#### Checking a Layout Without Aborting
164+
165+
`make_tensor_ptr` aborts the process if the sizes, dim order and strides do not describe a valid layout, or if the data does not match the sizes. When they come from outside the program, for example from a file or over the network, check them first:
166+
167+
```cpp
168+
if (validate_tensor_layout(sizes, dim_order, strides) != Error::Ok) {
169+
return Error::InvalidArgument; // reject the input instead of aborting
170+
}
171+
// After the check above, this product cannot overflow.
172+
const auto numel = std::accumulate(
173+
sizes.begin(), sizes.end(), size_t(1), std::multiplies<size_t>());
174+
if (data.size() != numel) {
175+
return Error::InvalidArgument;
176+
}
177+
auto tensor = make_tensor_ptr(sizes, std::move(data), dim_order, strides);
178+
```
179+
180+
`validate_tensor_layout` does not look at the data, so check its size yourself, as above. For a `std::vector<uint8_t>` of raw bytes, the expected size is `numel * elementSize(type)` bytes. Compute it with an overflow check, because this product can overflow `size_t` even when `numel` fits.
181+
163182
#### Sharing Existing Tensor
164183

165184
Since `TensorPtr` is a `std::shared_ptr<Tensor>`, you can easily create a `TensorPtr` that shares an existing `Tensor`. Any changes made to the shared data are reflected across all instances sharing the same data.

‎extension/tensor/tensor_ptr.cpp‎

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,23 +65,29 @@ struct Storage final {
6565
}
6666
};
6767
#endif // USE_ATEN_LIB
68-
} // namespace
6968

70-
TensorPtr make_tensor_ptr(
71-
std::vector<executorch::aten::SizesType> sizes,
72-
void* data,
73-
std::vector<executorch::aten::DimOrderType> dim_order,
74-
std::vector<executorch::aten::StridesType> strides,
75-
executorch::aten::ScalarType type,
76-
executorch::aten::Device device,
77-
executorch::aten::TensorShapeDynamism dynamism,
78-
std::function<void(void*)> deleter) {
69+
// Fills an empty dim_order and computes the strides. Shared by make_tensor_ptr
70+
// and validate_tensor_layout so the check rejects every layout the factory
71+
// aborts on.
72+
runtime::Error resolve_layout(
73+
const std::vector<executorch::aten::SizesType>& sizes,
74+
std::vector<executorch::aten::DimOrderType>& dim_order,
75+
const std::vector<executorch::aten::StridesType>& strides,
76+
std::vector<executorch::aten::StridesType>& computed_strides) {
7977
const auto dim = sizes.size();
80-
ET_CHECK_MSG(
78+
ET_CHECK_OR_RETURN_ERROR(
79+
dim <= runtime::kTensorDimensionLimit,
80+
InvalidArgument,
81+
"Tensor has %zu dims, more than the limit of %zu",
82+
dim,
83+
runtime::kTensorDimensionLimit);
84+
ET_CHECK_OR_RETURN_ERROR(
8185
dim_order.empty() || dim_order.size() == dim,
86+
InvalidArgument,
8287
"dim_order size must match sizes or be empty.");
83-
ET_CHECK_MSG(
88+
ET_CHECK_OR_RETURN_ERROR(
8489
strides.empty() || strides.size() == dim,
90+
InvalidArgument,
8591
"strides size must match sizes or be empty.");
8692

8793
if (dim_order.empty()) {
@@ -93,16 +99,31 @@ TensorPtr make_tensor_ptr(
9399
});
94100
}
95101
}
96-
std::vector<executorch::aten::StridesType> computed_strides(dim);
97-
98-
auto error = runtime::dim_order_to_stride(
99-
sizes.data(), dim_order.data(), dim, computed_strides.data());
100-
ET_CHECK_MSG(error == runtime::Error::Ok, "Failed to compute strides.");
102+
ET_CHECK_OR_RETURN_ERROR(
103+
runtime::validate_dim_order(dim_order.data(), dim),
104+
InvalidArgument,
105+
"Invalid dim order: values must be a permutation of [0, %zu)",
106+
dim);
107+
// Same strides as runtime::dim_order_to_stride, but an overflow is an error.
108+
computed_strides.assign(dim, 1);
109+
for (size_t i = dim; i > 1; --i) {
110+
const auto inner = dim_order[i - 1];
111+
const auto outer = dim_order[i - 2];
112+
const executorch::aten::StridesType size =
113+
sizes[inner] == 0 ? 1 : sizes[inner];
114+
ET_CHECK_OR_RETURN_ERROR(
115+
!c10::mul_overflows(
116+
computed_strides[inner], size, &computed_strides[outer]),
117+
InvalidArgument,
118+
"Stride of dim %zu overflows",
119+
static_cast<size_t>(outer));
120+
}
101121

102122
if (!strides.empty()) {
103123
for (size_t i = 0; i < dim; i++) {
104-
ET_CHECK_MSG(
124+
ET_CHECK_OR_RETURN_ERROR(
105125
strides[i] == computed_strides[i] || sizes[i] == 1,
126+
InvalidArgument,
106127
"invalid strides for dim %zu: %" ET_PRI_SIZES_AND_STRIDES
107128
"!= %" ET_PRI_SIZES_AND_STRIDES
108129
" while its size is %" ET_PRI_SIZES_AND_STRIDES " != 1",
@@ -112,10 +133,27 @@ TensorPtr make_tensor_ptr(
112133
sizes[i]);
113134
}
114135
}
136+
return runtime::Error::Ok;
137+
}
138+
} // namespace
115139

140+
TensorPtr make_tensor_ptr(
141+
std::vector<executorch::aten::SizesType> sizes,
142+
void* data,
143+
std::vector<executorch::aten::DimOrderType> dim_order,
144+
std::vector<executorch::aten::StridesType> strides,
145+
executorch::aten::ScalarType type,
146+
executorch::aten::Device device,
147+
executorch::aten::TensorShapeDynamism dynamism,
148+
std::function<void(void*)> deleter) {
149+
std::vector<executorch::aten::StridesType> computed_strides;
150+
const auto error =
151+
resolve_layout(sizes, dim_order, strides, computed_strides);
152+
ET_CHECK_MSG(error == runtime::Error::Ok, "Invalid tensor layout.");
116153
strides = std::move(computed_strides);
117154

118155
#ifndef USE_ATEN_LIB
156+
const auto dim = sizes.size();
119157
executorch::aten::TensorImpl tensor_impl(
120158
type,
121159
dim,
@@ -296,6 +334,17 @@ runtime::Error resize_tensor_ptr(
296334
sizes.data(), sizes.size()));
297335
}
298336

337+
runtime::Error validate_tensor_layout(
338+
const std::vector<executorch::aten::SizesType>& sizes,
339+
const std::vector<executorch::aten::DimOrderType>& dim_order,
340+
const std::vector<executorch::aten::StridesType>& strides) {
341+
ET_CHECK_OK_OR_RETURN_ERROR(
342+
executorch::aten::safe_numel(sizes.data(), sizes.size()).error());
343+
auto mutable_dim_order = dim_order;
344+
std::vector<executorch::aten::StridesType> computed_strides;
345+
return resolve_layout(sizes, mutable_dim_order, strides, computed_strides);
346+
}
347+
299348
// ---- Device tensor helper ----
300349
//
301350
// This helper relies on the ExecuTorch DeviceAllocator and the portable tensor

‎extension/tensor/tensor_ptr.h‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ using TensorPtr = std::shared_ptr<executorch::aten::Tensor>;
3838
* the runtime environment and pass it in. To copy CPU data to a device, use
3939
* `clone_tensor_ptr_to` instead.
4040
*
41+
* Aborts if `dim_order` or `strides` are invalid for `sizes`.
42+
* To reject a layout that comes from outside the program instead, check it
43+
* with `validate_tensor_layout` first.
44+
*
4145
* @param sizes A vector specifying the size of each dimension.
4246
* @param data A pointer to the data buffer (CPU or device, see device).
4347
* @param dim_order A vector specifying the order of dimensions.
@@ -622,6 +626,36 @@ runtime::Error resize_tensor_ptr(
622626
TensorPtr& tensor,
623627
const std::vector<executorch::aten::SizesType>& sizes);
624628

629+
/**
630+
* Checks whether `make_tensor_ptr` accepts the given layout, without creating
631+
* a tensor. It returns an error for every layout that the `make_tensor_ptr`
632+
* overloads taking sizes and data abort on, and also for sizes that are
633+
* negative or whose product overflows.
634+
*
635+
* A layout has at most `kTensorDimensionLimit` (16) dimensions. The dim
636+
* order and strides must each be empty or have one entry per dimension. A
637+
* non-empty dim order must be a permutation of the dimensions. An empty dim
638+
* order follows the strides from largest to smallest, or is the default order
639+
* when the strides are empty too. Non-empty strides must be the dense strides
640+
* for that dim order, except in dimensions of size 1. The dense strides must
641+
* fit in `StridesType` (`int32_t`, or `int64_t` in ATen mode).
642+
*
643+
* The data is not checked. The vector overloads of `make_tensor_ptr` also
644+
* need exactly as many elements, or bytes for raw bytes, as the sizes and
645+
* scalar type describe.
646+
*
647+
* @param sizes A vector specifying the size of each dimension.
648+
* @param dim_order A vector specifying the order of dimensions, or empty.
649+
* @param strides A vector specifying the strides of each dimension, or empty.
650+
* @return Error::Ok if the layout is valid, or Error::InvalidArgument
651+
* otherwise.
652+
*/
653+
ET_NODISCARD
654+
runtime::Error validate_tensor_layout(
655+
const std::vector<executorch::aten::SizesType>& sizes,
656+
const std::vector<executorch::aten::DimOrderType>& dim_order,
657+
const std::vector<executorch::aten::StridesType>& strides);
658+
625659
/**
626660
* Clones a TensorPtr's data onto the given target device, allocating and
627661
* copying as needed.

‎extension/tensor/test/tensor_ptr_test.cpp‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,14 @@ TEST_F(TensorPtrTest, TensorOwningEmptyData) {
332332
EXPECT_EQ(tensor->scalar_type(), executorch::aten::ScalarType::Float);
333333
}
334334

335+
TEST_F(TensorPtrTest, TensorWithInnerZeroSize) {
336+
// A size-0 dim counts as size 1 for the strides outside it, as in PyTorch.
337+
auto tensor = make_tensor_ptr({2, 0, 3}, std::vector<float>{});
338+
EXPECT_EQ(tensor->strides()[0], 3);
339+
EXPECT_EQ(tensor->strides()[1], 3);
340+
EXPECT_EQ(tensor->strides()[2], 1);
341+
}
342+
335343
TEST_F(TensorPtrTest, TensorDataOnly) {
336344
auto tensor = make_tensor_ptr({1.0f, 2.0f, 3.0f, 4.0f});
337345

@@ -1093,6 +1101,55 @@ TEST_F(TensorPtrTest, TensorInvalidDimOrder) {
10931101
"");
10941102
}
10951103

1104+
TEST_F(TensorPtrTest, ValidateTensorLayoutRejectsInvalidLayouts) {
1105+
EXPECT_EQ(validate_tensor_layout({2, 2}, {}, {8, 2}), Error::InvalidArgument);
1106+
EXPECT_EQ(
1107+
validate_tensor_layout({3, 4}, {1, 0}, {1, 4}), Error::InvalidArgument);
1108+
EXPECT_EQ(validate_tensor_layout({4, 5}, {2, 1}, {}), Error::InvalidArgument);
1109+
EXPECT_EQ(
1110+
validate_tensor_layout({3, 4}, {}, {4, 1, 1}), Error::InvalidArgument);
1111+
EXPECT_EQ(
1112+
validate_tensor_layout({3, 4}, {0, 1, 2}, {}), Error::InvalidArgument);
1113+
EXPECT_EQ(validate_tensor_layout({-1, 4}, {}, {}), Error::InvalidArgument);
1114+
EXPECT_EQ(
1115+
validate_tensor_layout(
1116+
std::vector<executorch::aten::SizesType>(17, 1), {}, {}),
1117+
Error::InvalidArgument);
1118+
const executorch::aten::SizesType large = 1 << 30;
1119+
EXPECT_EQ(
1120+
validate_tensor_layout({large, large, large}, {}, {}),
1121+
Error::InvalidArgument);
1122+
// The square of this size overflows StridesType.
1123+
const auto huge = executorch::aten::SizesType(1)
1124+
<< (4 * sizeof(executorch::aten::StridesType));
1125+
EXPECT_EQ(
1126+
validate_tensor_layout({0, huge, huge}, {}, {}), Error::InvalidArgument);
1127+
EXPECT_EQ(
1128+
validate_tensor_layout({2, huge, huge}, {}, {}), Error::InvalidArgument);
1129+
}
1130+
1131+
TEST_F(TensorPtrTest, ValidateTensorLayoutAcceptsValidLayouts) {
1132+
EXPECT_EQ(validate_tensor_layout({}, {}, {}), Error::Ok);
1133+
EXPECT_EQ(validate_tensor_layout({2, 3}, {}, {}), Error::Ok);
1134+
EXPECT_EQ(validate_tensor_layout({3, 4}, {1, 0}, {1, 3}), Error::Ok);
1135+
EXPECT_EQ(validate_tensor_layout({3, 4}, {}, {1, 3}), Error::Ok);
1136+
EXPECT_EQ(validate_tensor_layout({1, 4}, {}, {100, 1}), Error::Ok);
1137+
}
1138+
1139+
TEST_F(TensorPtrTest, TensorWithMaxDims) {
1140+
const std::vector<executorch::aten::SizesType> sizes(16, 1);
1141+
EXPECT_EQ(validate_tensor_layout(sizes, {}, {}), Error::Ok);
1142+
EXPECT_EQ(make_tensor_ptr(sizes, {1.f})->dim(), 16);
1143+
}
1144+
1145+
TEST_F(TensorPtrTest, TensorOverflowingStrides) {
1146+
// The square of this size overflows StridesType.
1147+
const auto huge = executorch::aten::SizesType(1)
1148+
<< (4 * sizeof(executorch::aten::StridesType));
1149+
ET_EXPECT_DEATH(
1150+
{ auto _ = make_tensor_ptr({0, huge, huge}, std::vector<float>{}); }, "");
1151+
}
1152+
10961153
TEST_F(TensorPtrTest, TensorCustomDeleter) {
10971154
float data[20] = {4};
10981155
auto tensor = make_tensor_ptr({4, 5}, data);

‎extension/wasm/wasm_bindings.cpp‎

Lines changed: 7 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
#include <executorch/extension/data_loader/buffer_data_loader.h>
1212
#include <executorch/extension/module/module.h>
1313
#include <executorch/extension/tensor/tensor.h>
14-
#include <executorch/runtime/core/exec_aten/util/tensor_util.h>
15-
#include <numeric>
1614

1715
#ifdef EXECUTORCH_ENABLE_EVENT_TRACER
1816
#include <executorch/devtools/etdump/etdump_flatcc.h>
@@ -136,49 +134,15 @@ std::vector<T> convertJSGeneratorToNumberVector(val generator) {
136134
return data;
137135
}
138136

139-
// make_tensor_ptr() assertions will abort the program if they fail.
140-
// These checks will throw a JS error instead.
137+
// make_tensor_ptr() aborts the program on an invalid layout. This throws a JS
138+
// error instead.
141139
void assert_dim_order_and_strides_valid(
142140
const std::vector<executorch::aten::SizesType>& sizes,
143-
std::vector<executorch::aten::DimOrderType>& dim_order,
144-
std::vector<executorch::aten::StridesType>& strides) {
145-
THROW_IF_FALSE(
146-
dim_order.size() == 0 || dim_order.size() == sizes.size(),
147-
"dim_order size must match sizes or be empty.");
148-
THROW_IF_FALSE(
149-
strides.size() == 0 || strides.size() == sizes.size(),
150-
"strides size must match sizes or be empty.");
151-
152-
if (dim_order.empty()) {
153-
dim_order.resize(sizes.size());
154-
std::iota(dim_order.begin(), dim_order.end(), 0);
155-
if (!strides.empty()) {
156-
std::sort(dim_order.begin(), dim_order.end(), [&](size_t a, size_t b) {
157-
return strides[a] > strides[b];
158-
});
159-
}
160-
}
161-
std::vector<executorch::aten::StridesType> computed_strides(sizes.size());
162-
163-
auto error = runtime::dim_order_to_stride(
164-
sizes.data(), dim_order.data(), sizes.size(), computed_strides.data());
165-
THROW_IF_ERROR(error, "Failed to compute strides.");
166-
167-
if (!strides.empty()) {
168-
for (size_t i = 0; i < sizes.size(); i++) {
169-
THROW_IF_FALSE(
170-
strides[i] == computed_strides[i] || sizes[i] == 1,
171-
"invalid strides for dim %zu: %" ET_PRI_SIZES_AND_STRIDES
172-
"!= %" ET_PRI_SIZES_AND_STRIDES
173-
" while its size is %" ET_PRI_SIZES_AND_STRIDES " != 1",
174-
i,
175-
strides[i],
176-
computed_strides[i],
177-
sizes[i]);
178-
}
179-
}
180-
181-
strides = std::move(computed_strides);
141+
const std::vector<executorch::aten::DimOrderType>& dim_order,
142+
const std::vector<executorch::aten::StridesType>& strides) {
143+
THROW_IF_ERROR(
144+
validate_tensor_layout(sizes, dim_order, strides),
145+
"Invalid tensor layout.");
182146
}
183147

184148
/**

0 commit comments

Comments
 (0)