Validate ORT format edge slots - #32200
Open
Akshay Sonawane (apsonawane) wants to merge 1 commit into
Open
Conversation
Akshay Sonawane (apsonawane)
requested review from
Tianlei Wu (tianleiwu)
and
a balanced review from Copilot
August 21, 2026 17:15
Akshay Sonawane (apsonawane)
enabled auto-merge (squash)
August 21, 2026 17:15
Copilot started reviewing on behalf of
Akshay Sonawane (apsonawane)
August 21, 2026 17:15
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens ORT-format graph loading by validating and canonicalizing serialized edge relationships.
Changes:
- Validates edge slots, NodeArgs, control edges, and producer conflicts.
- Canonicalizes one-sided and reciprocal edge records.
- Adds focused malformed-model tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
onnxruntime/core/graph/graph.cc |
Implements edge validation and canonicalization. |
include/onnxruntime/core/graph/graph.h |
Allows edge loading to update graph relationships. |
onnxruntime/test/framework/ort_model_only_test.cc |
Adds edge-loading validation tests. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+915
to
+918
| const bool is_control_edge = src_arg_index == INT_MAX && dst_arg_index == INT_MAX; | ||
| ORT_RETURN_IF((src_arg_index == INT_MAX) != (dst_arg_index == INT_MAX), | ||
| "Node::LoadEdgesFromOrtFormat, ", edge_description, | ||
| " has an invalid control-edge slot pair. Invalid ORT format model."); |
| auto add_edges = [&graph](const flatbuffers::Vector<const onnxruntime::fbs::EdgeEnd*>* fbs_edges, | ||
| EdgeSet& edge_set, const std::string& dst_name) -> Status { | ||
| auto add_edges = [this, &graph](const flatbuffers::Vector<const onnxruntime::fbs::EdgeEnd*>* fbs_edges, | ||
| const std::string& edge_description, bool input_edges) -> Status { |
Contributor
There was a problem hiding this comment.
nit: the edge description can be inferred from the new bool parameter. also, the edge description value is plural (e.g., "input edges") which doesn't read well with many of the error messages.
Comment on lines
+920
to
+922
| src_node.relationships_.output_edges.emplace(dst_node); | ||
| dst_node.relationships_.input_edges.emplace(src_node); | ||
| dst_node.relationships_.control_inputs.insert(src_node.Name()); |
Contributor
There was a problem hiding this comment.
this is a copy of the logic in Graph::AddControlEdge(). maybe refactor into a shared helper function like void AddControlEdgeBetweenNodes(Node& src_node, Node& dst_node)?
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.
This pull request enhances the robustness and correctness of ONNX Runtime's ORT format model loading, particularly around edge validation in computational graphs. It introduces stricter checks for edge consistency, improves error handling, and significantly expands test coverage to prevent invalid graph structures from being loaded.
Edge validation and canonicalization improvements:
Node::LoadEdgesFromOrtFormatmethod was refactored to enforce stricter validation of edge indices, ensure control edges are handled correctly, and guarantee that edges only connect matchingNodeArgs. This includes checks for out-of-range indices, mismatched arguments, and correct handling of control edges. (onnxruntime/core/graph/graph.cc,include/onnxruntime/core/graph/graph.h) [1] [2]onnxruntime/core/graph/graph.cc)Test coverage expansion:
onnxruntime/test/framework/ort_model_only_test.cc) [1] [2]These changes collectively ensure that only valid and well-formed graphs are accepted from ORT format models, reducing the risk of subtle bugs or crashes due to malformed edge definitions.