Validate initializer data length and guard element-count computation in contrib Range shape inference#29265
Open
titaiwangms wants to merge 4 commits into
Open
Conversation
…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>
Contributor
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.