Repository navigation
Patch onnx v1.22.0 tensor.h to honour ONNX_NO_EXCEPTIONS - #32593
Closed
Ryan VanderMeulen (rvandermeulen) wants to merge 1 commit into
Closed
Ryan VanderMeulen (rvandermeulen) wants to merge 1 commit into
Ryan VanderMeulen (rvandermeulen) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The vcpkg build path remains affected by the raw throw.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR patches ONNX v1.22.0 tensor error handling to honor ONNX_NO_EXCEPTIONS.
Changes:
- Routes
elem_num()andsize_from_dim()errors throughthrow_tensor_error(). - Leaves the vcpkg patch mirror uncovered.
File summaries
| File | Summary |
|---|---|
cmake/patches/onnx/onnx.patch |
Adds the ONNX tensor exception-handling patch; the equivalent vcpkg hunk is still needed. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Ryan VanderMeulen (rvandermeulen)
force-pushed
the
onnx-patch-tensor-noexcept
branch
from
September 14, 2026 19:01
29ed2e7 to
bdb95f5
Compare
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified in the reviewed patches.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
onnx v1.22.0 rewrote Tensor::elem_num() and Tensor::size_from_dim() around safe_dim_product() with an error callback that does a raw `throw tensor_error(msg)`, bypassing onnx's ONNX_THROW_EX macro. That breaks any onnxruntime build with exceptions disabled as soon as those headers are included. Route both callbacks through the existing [[noreturn]] throw_tensor_error() helper, which honours ONNX_NO_EXCEPTIONS. This is onnx/onnx#8220 (merged upstream, fixes onnx/onnx#8219), carried in cmake/patches/onnx/onnx.patch and its vcpkg mirror cmake/vcpkg-ports/onnx/binskim.patch until the onnx dependency moves to a release that contains it.
Ryan VanderMeulen (rvandermeulen)
force-pushed
the
onnx-patch-tensor-noexcept
branch
from
September 25, 2026 01:01
bdb95f5 to
35052fc
Compare
Contributor
Author
|
This was submitted before onnx v1.23.0 was cut. v1.23.0 includes onnx/onnx#8220, so if you'd rather bump the onnx dependency to v1.23.0 instead, this PR is superseded by that and can be closed. |
Contributor
Author
|
Edward Chen (@edgchen1) This is one of the related PRs from #32597, if you have a chance to take a look. |
Edward Chen (edgchen1)
pushed a commit
that referenced
this pull request
Sep 28, 2026
…-minimal build (#32597) ### Description Convert the raw `try`/`catch`/`throw` statements that a CPU-only, non-minimal build still compiles to the `ORT_TRY` / `ORT_CATCH` / `ORT_HANDLE_EXCEPTION` / `ORT_RETHROW` / `ORT_THROW` macros the rest of the code base uses: `graph_partitioner`, `attention_fusion`, `constant_folding`, `stft_decomposition`, `model_package_context`, `ep_library_plugin`, `provider_bridge_ort`, and the `ORT_ENABLE_STREAM` config reader/writer in `allocation_planner` (whose writer already used `ORT_CATCH` but read the exception outside `ORT_HANDLE_EXCEPTION`). `provider_bridge_ort.cc`'s 18 function-try-blocks have no macro equivalent, so each becomes an `ORT_TRY` over the body followed by a file-local `ORT_CATCH_LOG_RETURN_NULLPTR` that logs and returns `nullptr`, the tail they all shared. `ProviderLibrary::Load`/`Get` keep their unload-and-rethrow via `ORT_RETHROW`. The `allocation_planner` handler that used `EXIT_ON_ERR` is expanded inline so its `return` stays in the function rather than inside the handler lambda. With exceptions enabled this is a spelling change with identical control flow. With `onnxruntime_DISABLE_EXCEPTIONS` the result compiles and links on Linux x64/aarch64, Windows x64/x86/arm64, macOS x64/arm64 and Android arm/arm64/x64 (Firefox's CI, with #32591, #32592, #32593 and #32595 applied alongside). ### Motivation and Context Firefox builds ONNX Runtime with exceptions disabled and carries these conversions as local patches, together with a hack that disables `ORT_ENABLE_STREAM` only because of the `allocation_planner` sites. Together with the other open PRs this leaves the `onnxruntime_DISABLE_EXCEPTIONS` + `onnxruntime_MINIMAL_BUILD` coupling as the only remaining obstacle to a non-minimal no-exceptions build.
Contributor
I think we can just take ONNX 1.23. the ONNX version has been updated in main already with #32359. |
Contributor
|
thanks for the PR. closing as we have already updated ONNX to 1.23 in main. |
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.
Description
Add a hunk to
cmake/patches/onnx/onnx.patchand its vcpkg mirrorcmake/vcpkg-ports/onnx/binskim.patchrouting the error callbacks inTensor::elem_num()andTensor::size_from_dim()through the existing[[noreturn]] throw_tensor_error()helper, which honoursONNX_NO_EXCEPTIONS. This is onnx/onnx#8220, merged upstream but not yet in a release; the hunk can be dropped when the onnx dependency moves past v1.22.0.Motivation and Context
onnx v1.22.0 rewrote those two functions around
safe_dim_product()with a callback that does a rawthrow tensor_error(msg), bypassing onnx'sONNX_THROW_EXmacro. That breaks any onnxruntime build with exceptions disabled as soon as the header is included. Fixes the build failure reported in onnx/onnx#8219 for onnxruntime consumers.