Fix Type-confusion: raw-pointer bind_input causing OOB write - #28781
Closed
selenayang888 wants to merge 6 commits into
Closed
Fix Type-confusion: raw-pointer bind_input causing OOB write#28781selenayang888 wants to merge 6 commits into
selenayang888 wants to merge 6 commits into
Conversation
Member
|
You should add a unit test. |
tianleiwu
previously approved these changes
Jun 4, 2026
yuslepukhin
reviewed
Jun 4, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Python SessionIOBinding.bind_input raw-pointer overloads to prevent binding STRING tensors backed by a non-owning byte buffer, which can lead to type confusion and unsafe reads/writes through unconstructed std::string storage.
Changes:
- Reject
onnx::TensorProto::STRINGin thebind_input(..., int32_t element_type, ..., int64_t data_ptr)overload. - Reject
std::string(NPY_UNICODE/NPY_STRING/NPY_OBJECT/NPY_VOIDmapping) in thebind_input(..., py::object& element_type, ..., int64_t data_ptr)overload.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
in your repo run one time: |
…dentity Co-authored-by: Copilot <copilot@github.com>
…tf_models_to_pytorch.py The test was failing during collection in the CI build environment because the module path calculation used a hardcoded `parents[4]` offset that assumed the test file was in the source tree at `onnxruntime/test/python/transformers/`. In CI, test files are copied to the build output directory where the parent-traversal path points to the runner's temp directory rather than the workspace source root. Fixes: 1. Update path resolution in test_convert_tf_models_to_pytorch.py to first look in the same directory as the test file (works after CMake copies the source alongside tests), then fall back to the source-tree location using the correct `parents[2]` offset. 2. Add a CMake copy command in onnxruntime_python.cmake to copy convert_tf_models_to_pytorch.py from onnxruntime/python/tools/transformers/ to the transformers/ build output directory alongside the test files.
Copilot
AI
changed the title
Fix Type-confusion: raw-pointer bind_input causing OOB write
Fix Windows GPU CUDA CI failure: test_convert_tf_models_to_pytorch.py collection error
Jun 5, 2026
Contributor
|
See #28798 |
selenayang888
changed the title
yuslepukhin
pushed a commit
that referenced
this pull request
Jun 8, 2026
### Description > **NOTE: Replacement of #28781. The original PR was accidentally affected by an unexpected agentic AI commit.** Fix a pytest collection error in the Windows GPU CUDA CI pipeline caused by `test_convert_tf_models_to_pytorch.py` failing to locate `convert_tf_models_to_pytorch.py` at module load time. Two changes were made: - **`onnxruntime/test/python/transformers/test_convert_tf_models_to_pytorch.py`**: Updated the path resolution logic to first check if `convert_tf_models_to_pytorch.py` is in the same directory as the test file (the case after CMake copies it into the build output), then fall back to the correct source-tree path using `parents[2]` (resolving to the `onnxruntime/` subdirectory, three levels up from `test/python/transformers/`). - **`cmake/onnxruntime_python.cmake`**: Added a `cmake -E copy` command to deploy `convert_tf_models_to_pytorch.py` from `onnxruntime/python/tools/transformers/` into the `transformers/` build output directory alongside the existing test files, so the first-choice path resolution works in CI. ### Motivation and Context The CI job `Windows GPU CUDA CI Pipeline Test Job` was failing during pytest collection with: ``` FileNotFoundError: 'D:\\a\\_work\\_temp\\onnxruntime\\python\\tools\\transformers\\convert_tf_models_to_pytorch.py' ``` The test used a hardcoded `parents[4]` offset assuming the test file resided at `onnxruntime/test/python/transformers/` in the source tree (where `parents[4]` = repo root). In CI, pytest test files are copied to the build output directory (e.g. `$runner_temp/build/RelWithDebInfo/RelWithDebInfo/transformers/`), where `parents[4]` resolves to the runner's temp directory rather than the workspace root, so the source file was never found and collection aborted with exit code 2. --------- Co-authored-by: Copilot <copilot@github.com>
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
Fix a pytest collection error in the Windows GPU CUDA CI pipeline caused by
test_convert_tf_models_to_pytorch.pyfailing to locateconvert_tf_models_to_pytorch.pyat module load time.Two changes were made:
onnxruntime/test/python/transformers/test_convert_tf_models_to_pytorch.py: Updated the path resolution logic to first check ifconvert_tf_models_to_pytorch.pyis in the same directory as the test file (the case after CMake copies it into the build output), then fall back to the correct source-tree path usingparents[2](resolving to theonnxruntime/subdirectory, three levels up fromtest/python/transformers/).cmake/onnxruntime_python.cmake: Added acmake -E copycommand to deployconvert_tf_models_to_pytorch.pyfromonnxruntime/python/tools/transformers/into thetransformers/build output directory alongside the existing test files, so the first-choice path resolution works in CI.Motivation and Context
The CI job
Windows GPU CUDA CI Pipeline Test Jobwas failing during pytest collection with:The test used a hardcoded
parents[4]offset assuming the test file resided atonnxruntime/test/python/transformers/in the source tree (whereparents[4]= repo root). In CI, pytest test files are copied to the build output directory (e.g.$runner_temp/build/RelWithDebInfo/RelWithDebInfo/transformers/), whereparents[4]resolves to the runner's temp directory rather than the workspace root, so the source file was never found and collection aborted with exit code 2.