Skip to content

[OpenVINO EP] Fix constant output mis-routing for names containing '/' - #29882

Merged
fdwr merged 2 commits into
microsoft:mainfrom
wangw-1991:fix_tensor_name_match
Aug 3, 2026
Merged

[OpenVINO EP] Fix constant output mis-routing for names containing '/'#29882
fdwr merged 2 commits into
microsoft:mainfrom
wangw-1991:fix_tensor_name_match

Conversation

@wangw-1991

Copy link
Copy Markdown
Contributor

Description

GetOutputTensor in the OpenVINO EP resolved a constant-folded output's OpenVINO friendly name back to an ONNX output by truncating at the first / and doing an exact-string lookup. OpenVINO appends a /sink_port_0 suffix to the friendly name, and ONNX output names may themselves contain /, so a name like D/x/sink_port_0 was truncated to D and matched different output named D. The constant was then written into the wrong output's slot; with a pre-bound fixed-size output buffer this is an out-of-bounds write.

This PR:

  • Matches the full friendly name first, and only strips the single trailing /-suffix if that fails, so names containing / resolve correctly.
  • Adds a byte-size bounds check in FillOutputHelper before the std::copy, so any residual size mismatch throws instead of overrunning the buffer.
  • Adds regression tests covering the same-shape mis-route and the mismatched-shape overrun.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@wangw-1991

Copy link
Copy Markdown
Contributor Author

@fdwr Can you help review this? Thanks.

@fdwr

fdwr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

I'll review, but do you also have any contacts on the OpenVINO side to review?

fdwr
fdwr previously approved these changes Jul 27, 2026

@fdwr fdwr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread onnxruntime/test/providers/openvino/openvino_ep_context_test.cc Outdated
@wangw-1991

Copy link
Copy Markdown
Contributor Author

I'll review, but do you also have any contacts on the OpenVINO side to review?

Hi @javier-intel, could you help review this change or find a suitable reviewer? Thanks.

@fdwr fdwr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fdwr

fdwr commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Tests passed ✅.

@wangw-1991

Copy link
Copy Markdown
Contributor Author

Tests passed ✅.

Thanks, We need another approval before merging this, right?

@javier-intel

Copy link
Copy Markdown
Contributor

It looks good to me but I don't work in that code enough to feel confident. @ankitm3k can you please also take a look?

@fdwr

fdwr commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

We need another approval before merging this, right?

@wangw-1991: Well ideally we'd have an OpenVINO expert review it too, but if ankitm3k doesn't have the time to approve, I'm also content to merge it, as we have two positive reviews by nonexperts :b.

@wangw-1991

Copy link
Copy Markdown
Contributor Author

We need another approval before merging this, right?

@wangw-1991: Well ideally we'd have an OpenVINO expert review it too, but if ankitm3k doesn't have the time to approve, I'm also content to merge it, as we have two positive reviews by nonexperts :b.

Thanks, I've already pinged him; let's wait and see.

@wangw-1991

Copy link
Copy Markdown
Contributor Author

Hi, @fdwr. @ankitm3k hasn't responded, can you help merge this? Thanks.

@fdwr
fdwr merged commit 8d187b0 into microsoft:main Aug 3, 2026
86 checks passed
@ankitm3k

ankitm3k commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

The openvino EP aka Legacy EP in this repo will soon reach end of dev cycle. To use the new ABI EP use the WinML APIs from builds available now at -

wheel - https://pypi.org/project/onnxruntime-ep-openvino/

Nuget - http://nuget.org/packages/Intel.ML.OnnxRuntime.EP.OpenVINO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants