Validate initializer data length and guard element-count computation in contrib Range shape inference#29265
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the com.microsoft Range operator’s shape inference and the CPU kernel against malformed initializer raw_data and invalid/overflowing element-count computations, ensuring model load and runtime execution fail cleanly instead of reading out-of-bounds or hitting undefined conversions.
Changes:
- Added
raw_datalength validation + aligned reads (viamemcpy) in contrib Range shape inference. - Added non-finite and out-of-
int64_t-range guards for the computed element count in both shape inference and the CPU kernel; clamped empty/backward ranges to dimension 0. - Added contrib Range model-load regression tests covering truncated
raw_data, zero delta, too-large counts, and backward-range inferred zero-dim.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| onnxruntime/core/graph/contrib_ops/range_schema_defs.cc | Adds raw_data length checks/aligned reads and validates/clamps computed output length during shape inference. |
| onnxruntime/core/providers/cpu/generator/range.cc | Adds runtime validation for computed element counts (finite + representable in int64_t) and clamps empty/backward ranges. |
| onnxruntime/test/providers/cpu/generator/range_test.cc | Adds model-load regression tests for contrib Range shape inference failure/success boundary cases. |
…e kernel guards Address automated review feedback on PR microsoft#29265: - range.cc ComputeRange now computes the element count as ceil((double(limit) - double(start)) / double(delta)), byte-identical to the shape-inference path in CalcRangeDim, so integral inputs are promoted to double before the subtraction and cannot overflow in T. - range_schema_defs.cc CalcRangeDim uses the same expression; comments on both sites note they must stay identical. - Added two CPU-EP-pinned kernel-execution tests with runtime (non-constant) inputs so the ComputeRange element-count guards are exercised at execution time: a count that exceeds the int64 range and a non-finite count. Agent-signed-off: Developer (b2fe149b) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e inference GetFirstElement<T> read sizeof(T) bytes from a graph initializer's raw_data guarded only by the presence bit. Add a length check before the cast and fail shape inference cleanly when raw_data is shorter than the element size. Also clamp the computed element count to >= 0 in CalcRangeDim so shape inference matches the CPU kernel for empty/backward ranges. Add model-load regression tests in range_test.cc (guarded by #ifndef DISABLE_CONTRIB_OPS, Status-only assertions for no-exception builds). Agent-signed-off: Developer (b2fe149b) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- GetFirstElement<T>: read raw_data via std::memcpy into an aligned local after the length check, avoiding an unaligned access on strict-alignment targets. - CalcRangeDim<T> and the CPU kernel ComputeRange: guard the element-count cast with std::isfinite so a non-finite computed count fails cleanly instead of an out-of-range cast (fail_shape_inference in the schema, a non-OK Status in the kernel). - range_test.cc: generalize the model-building helper over element type, dimensions and per-input bytes. Truncated raw_data is now modeled as dims=[0] with empty raw_data so the cases are tight regressions for the length check; add coverage for start/limit/delta positions, float and int64 element types, a zero-delta failure, and an exact-size success boundary. Throwing tests are additionally guarded by !defined(ORT_NO_EXCEPTIONS). Agent-signed-off: Developer (b2fe149b) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cast The std::isfinite guard catches inf/NaN but not a finite count larger than the int64 range, where static_cast<int64_t>(count) is out of range. Handle the non-positive case before the cast (so a large-magnitude negative count cannot overflow it) and reject counts at or above 2^63 with a neutral message. Applied identically in the schema CalcRangeDim (fail_shape_inference) and the CPU kernel ComputeRange (non-OK Status), keeping the two sites consistent. Add a guarded regression test (start=0, limit=1e19, delta=1) asserting a clean non-OK status; verified it fails without the new guard. Agent-signed-off: Developer (b2fe149b) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e kernel guards Address automated review feedback on PR microsoft#29265: - range.cc ComputeRange now computes the element count as ceil((double(limit) - double(start)) / double(delta)), byte-identical to the shape-inference path in CalcRangeDim, so integral inputs are promoted to double before the subtraction and cannot overflow in T. - range_schema_defs.cc CalcRangeDim uses the same expression; comments on both sites note they must stay identical. - Added two CPU-EP-pinned kernel-execution tests with runtime (non-constant) inputs so the ComputeRange element-count guards are exercised at execution time: a count that exceeds the int64 range and a non-finite count. Agent-signed-off: Developer (b2fe149b) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a13bb91 to
7fa0558
Compare
|
…the launch path to int64 Mirror the CPU kernel and shape-inference guards in the standard onnx-domain CUDA Range kernel so all three paths stay consistent: - range.cc ComputeRange now rejects a non-finite computed count, handles the non-positive case before the cast, and rejects counts at or above 2^63 (not representable as int64) before converting to int64_t. Messages are identical to the CPU kernel. - Widen the launch path (range_impl.h/.cu) from int to int64_t for the element count and kernel index, compute the block count in int64_t, cap the grid x-dimension to 2^31-1, and use a 64-bit grid-stride loop so valid counts larger than a single launch grid (and larger than INT_MAX) are produced without index truncation or grid-dimension overflow. - Add CUDA-EP tests (guarded by USE_CUDA, skipped when no CUDA provider) exercising the non-finite and >=2^63 count guards at execution time with runtime (non-constant) inputs. Agent-signed-off: Developer (b2fe149b) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…VC 26451 pragma Add get_data<int16_t> in range_schema_defs.cc so a com.microsoft Range with an INT16 initializer stored in the typed int32_data field (the ONNX packing for INT16) resolves shape inference instead of hitting the generic template. int16 is an advertised T type constraint and CalcResultDim already dispatched to CalcRangeDim<int16_t>; only the typed-field accessor was missing. Remove the file-wide MSVC 26451 pragma and its TODO in cpu/generator/range.cc; the double-promoted count computation removed the arithmetic-overflow root cause. Agent-signed-off: Developer (fc32ad9a) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thanks @yuslepukhin for the thorough review — all three points are now addressed in the latest revision:
Appreciate the careful look — the CUDA parity in particular is a good catch. |
Add ContribOp_Int16TypedField_InfersDim: a com.microsoft Range model whose INT16 start/limit/delta initializers are carried in the typed int32_data field (the ONNX packing for INT16) rather than raw_data. This exercises the get_data<int16_t> typed-field path added in the schema fix; without that specialization the generic template throws during shape inference. Extends the RangeInputSpec test harness with an optional int32_data payload and an Int16TypedInput helper. Agent-signed-off: Developer (fc32ad9a) [claude-opus-4.8 via copilot] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
36ef141 to
d1ce9b1
Compare
Summary
The
com.microsoftRange operator's shape-inference helper read a fixed number of bytes from an initializer'sraw_datawithout first checking the buffer length, and the element-count computation could pass non-finite or out-of-range values to anint64cast. This change adds the missing validations and aligns the shape-inference and CPU kernel paths.Changes
GetFirstElementnow checksraw_datalength is at least the element size before reading, and reads viastd::memcpyinto an aligned local.CalcRangeDimand the CPU kernelComputeRangenow reject non-finite computed counts, handle non-positive counts before theint64cast, and reject counts that are not representable asint64(>= 2^63). Both paths use identical messages and semantics.Tests
Added contrib Range model-load regression tests in
range_test.cccovering:raw_dataforstart,limit, anddelta(double, plus float and int64 element types),Tests assert on
Status(safe for no-exception builds) and are guarded by#ifndef DISABLE_CONTRIB_OPS(throwing cases additionally by!defined(ORT_NO_EXCEPTIONS)). 21/21RangeTestcases pass locally.Follow-up
int16inputs are currently supported only viaraw_data(there is noget_data<int16_t>specialization for the non-raw-data path); this is left as a separate follow-up.