Skip to content

[OpenVINO EP] Fix get/set_float_initializer_data for raw_data backed float initializers - #31138

Open
wangw-1991 wants to merge 3 commits into
microsoft:mainfrom
wangw-1991:fix_float_initializer
Open

[OpenVINO EP] Fix get/set_float_initializer_data for raw_data backed float initializers#31138
wangw-1991 wants to merge 3 commits into
microsoft:mainfrom
wangw-1991:fix_float_initializer

Conversation

@wangw-1991

Copy link
Copy Markdown
Contributor

Description

get_float_initializer_data / set_float_initializer_data assumed a float initializer always stores its value in the typed float_data field. When the value lives in raw_data instead, float_data(0) reads out of bounds and set_float_data(0, ...) writes out of bounds — both undefined behaviour.

Both functions now select the field that actually holds the data: use float_data when it is non-empty, otherwise read/write raw_data (guarded by size checks).

Testing

Adds openvino_ov_protobuf_utils_test.cc covering get/set against both float_data and raw_data backed float scalars. Because the OpenVINO EP is built as a shared-library module with hidden symbols, ov_protobuf_utils.cpp is compiled directly into onnxruntime_provider_test so the tests can link.

@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.

Comment thread cmake/onnxruntime_unittests.cmake
Comment thread onnxruntime/core/providers/openvino/ov_protobuf_utils.cpp
Comment thread onnxruntime/core/providers/openvino/ov_protobuf_utils.cpp Outdated
@wangw-1991
wangw-1991 requested a review from fdwr July 30, 2026 03:52

@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.

👍

@wangw-1991

wangw-1991 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi @ankitm3k, could you help review this change? Thanks.

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.

2 participants