Fix access violation for Broadcast operator when 0-size shape with scalar input#34874
Conversation
|
@mvafin .. Can you please review this PR |
There was a problem hiding this comment.
Pull request overview
This PR addresses an ONNX frontend crash/access violation scenario for Expand/Broadcast when the target shape tensor has zero elements (e.g., shape [0]), particularly impacting cases like scalar/1-element inputs, by converting such initializers into “failsafe” constants so the Expand importer can safely ignore the target shape.
Changes:
- Add an ONNX FE test covering
Expandwith a scalar/1-element input and an empty target-shape constant. - Update ONNX
Tensor::get_ov_constant()to produce a failsafe constant when the tensor has zero elements and an empty/zero-sized shape (including cases whereshape_size(m_shape) == 0).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/frontends/onnx/tests/onnx_import.in.cpp |
Adds a regression test intended to reproduce the empty-shape Expand issue for scalar/1-element input. |
src/frontends/onnx/frontend/src/core/tensor.cpp |
Adjusts constant creation to generate a failsafe constant for zero-element tensors with empty/zero-sized shapes, enabling downstream op handlers (e.g., Expand) to avoid unsafe behavior. |
| } | ||
| } | ||
| if (has_external_data()) { | ||
| if (element_count == 0 && (m_shape.size() == 0 || shape_size(m_shape) == 0)) { |
There was a problem hiding this comment.
mvafin
left a comment
There was a problem hiding this comment.
Looks good.
This code is used in both legacy and delegate paths and the test is added for both cases.
Good to merge if all tests pass.
…e 0-size tensor cases
| if (common::is_failsafe_node(pattern.get_node_shared_ptr())) { | ||
| // in case the "shape" input is connected to a failsafe node created in place of an invalid initializer | ||
| // the target shape should be ignored and this Expand operation should not modify its input tensor | ||
| // the Broadcast created below should be eliminated later on by an appropriate optimization pass | ||
| pattern = v0::Constant::create(ov::element::i64, {0}, {}); | ||
| } |
There was a problem hiding this comment.
[BLOCKER] When the Reshape "shape" input is a failsafe node, replacing it with an empty 1D constant ({0}) will reshape any non-scalar input into a scalar and can lead to runtime validation failures due to element-count mismatch. If the intent is to ignore an invalid initializer and preserve the input tensor, this should instead behave as an identity (e.g., return the input unchanged / use ShapeOf(data) as the pattern) and update the comment (it currently refers to Expand/Broadcast, which is unrelated to Reshape).
| if (names.size() > 0) { | ||
| constant->set_friendly_name(names[0]); | ||
| constant->get_default_output().set_names({names.begin(), names.end()}); | ||
| } |
There was a problem hiding this comment.
[HIGH] create_constant_from_place uses m_tensor_place (member) when setting names, and the unsupported-type path references m_tensor_proto->data_type(). Since Tensor can be constructed with only a TensorONNXPlace (and m_tensor_proto == nullptr), this can become a null dereference if an unexpected element type hits the default branch. Prefer using the tensor_place argument consistently (including for names), and avoid dereferencing m_tensor_proto in this code path.
|
You created lambda functions for place and for proto, this makes it harder to review, but doesn't really help to solve any issue. Please revert that change |
|
build_jenkins |
#34895) ### Details: - This case will make `element_count == 0`, `m_shape.size() == 1`, but `shape_size(m_shape) == 0`. - The `repeats` in `src\plugins\intel_cpu\src\nodes\broadcast.cpp` is `0`. But the `srcDims.size() = 1`. That make the program access `repeats[0]` and cause the access violation. Port the #34874 to 2026.1 ### Tickets: - [CVS-183409](https://jira.devtools.intel.com/browse/CVS-183409) ### AI Assistance: - *AI assistance used: no / yes* - *If yes, summarize how AI was used and what human validation was performed (build/tests/manual checks).* --------- Co-authored-by: Michal Lukaszewski <michal.lukaszewski@intel.com>
#34973) ### Details: - A following PR for #34874 - It fixed the conflict with external 0 size weight. ### Tickets: - [CVS-183409](https://jira.devtools.intel.com/browse/CVS-183409) ### AI Assistance: - *AI assistance used: no - *If yes, summarize how AI was used and what human validation was performed (build/tests/manual checks).*
Details:
element_count == 0,m_shape.size() == 1, butshape_size(m_shape) == 0.repeatsinsrc\plugins\intel_cpu\src\nodes\broadcast.cppis0. But thesrcDims.size() = 1. That make the program accessrepeats[0]and cause the access violation.Tickets:
AI Assistance: