Fix new-delete mismatch in DML EP's QuantizeLinear operator#27823
Fix new-delete mismatch in DML EP's QuantizeLinear operator#27823
Conversation
Is this fix hiding any remaining allocation issue which a different E_INVALIDARG might cause? |
fdwr
left a comment
There was a problem hiding this comment.
Glad you found it. 🔍 Approved with some recommendations.
onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorElementWise.cpp
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlOperatorElementWise.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Change looks okay to me (assuming the initial ref count is 1 rather than 0), but I wonder if this location is sufficient (other places use MakeOrThrow), and whether this is actually an issue because the heap block size is stored in the heap itself, meaning a sized delete mismatch seems bad in theory but not in practice (operator delete just calls free which reads the heap block header to read the block size - the same value that _msize function returns).
onnxruntime/core/providers/dml/OperatorAuthorHelper/MLOperatorAuthorHelper.h
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/dml/OperatorAuthorHelper/MLOperatorAuthorHelper.h
Show resolved
Hide resolved
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run ONNX Runtime Web CI Pipeline,Windows GPU CI Pipeline,Linux Android Emulator QNN CI Pipeline,Windows GPU WebGPU CI Pipeline,Windows OpenVINO CI Pipeline |
|
/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,MacOS CI Pipeline,Windows CPU CI Pipeline |
|
/azp run Windows GPU CUDA CI Pipeline,Windows GPU DML CI Pipeline |
|
/azp run Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,Windows x64 QNN CI Pipeline,Big Models |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
No pipelines are associated with this pull request. |
|
/azp run Test Linux CUDA x64 Release,Test Linux TensorRT x64 Release,web_Debug / build_onnxruntime_web,web_Release / build_onnxruntime_web |
|
No pipelines are associated with this pull request. |
|
/azp run Linux QNN CI Pipeline,Win_TRT_Minimal_CUDA_Test_CI,Windows GPU Doc Gen CI Pipeline,Windows ARM64 QNN CI Pipeline |
|
No pipelines are associated with this pull request. |
1 similar comment
|
No pipelines are associated with this pull request. |
|
Azure Pipelines successfully started running 4 pipeline(s). |
### Description <!-- Describe your changes. --> DmlOperatorQuantization21 was missing the tensor reshaping logic that the older DmlOperatorElementwiseQLinear already had. Scalar scale tensors get padded to 4D, but a 5D input stays 5D. DML rejects the dimension mismatch with E_INVALIDARG, and the resulting exception unwind triggers a sized-delete bug in WRL's MakeAllocator which address sanitizer detects. The fix is to port the same logic from the DmlOperatorElementwiseQLinear into this path, so that the dimensions match. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> This is required to ensure the DML EP correctly handles this scenario. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This cherry-picks the following commits for the release: - #26434 [VitisAI]add tensor type bool - #26452 [VitisAI EP] Fix error in graph resolving - #26487 [VitisAI] Enable ort::logger usage in compile_onnx_model_vitisai_ep_v4 - #26519 [VitisAI] Remove unused function body handling in graph fusion - #26627 [VitisAI] Add External EP Loader - #26699 [VitisAI] Add support compiled model compatibility information retrieval and validation - #27295 Remove s_kernel_registry_vitisaiep.reset() in deinitialize_vitisai_ep() - #27356 Add/Update telemetry events - #27626 Add PE version info to onnxruntime_providers_vitisai.dll - #27693 Fix integer division by zero crash in CPU EP Div operator - #27815 Fix overflow in DmlGraphFusionHelper::ProcessInputData - #27823 Fix new-delete mismatch in DML EP's QuantizeLinear operator --------- Co-authored-by: Yueqing Zhang <yuz75@Pitt.edu> Co-authored-by: Yueqing Zhang <yueqingz@amd.com> Co-authored-by: zpye <yezupei92@foxmail.com> Co-authored-by: Chunye Wang@AMD <chunywan@amd.com> Co-authored-by: mingyue <131847423+mingyueliuh@users.noreply.github.com> Co-authored-by: zz002 <zhenzew@amd.com> Co-authored-by: Darshak Bhatti <47045043+dabhattimsft@users.noreply.github.com> Co-authored-by: Darshak Bhatti <dabhatti@micorsoft.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Xiaoxi Han <xiha@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianleiwu <30328909+tianleiwu@users.noreply.github.com> Co-authored-by: skottmckay <979079+skottmckay@users.noreply.github.com> Co-authored-by: Tianlei Wu <tlwu@microsoft.com>
Description
DmlOperatorQuantization21 was missing the tensor reshaping logic that the older DmlOperatorElementwiseQLinear already had.
Scalar scale tensors get padded to 4D, but a 5D input stays 5D. DML rejects the dimension mismatch with E_INVALIDARG, and the resulting exception unwind triggers a sized-delete bug in WRL's MakeAllocator which address sanitizer detects. The fix is to port the same logic from the DmlOperatorElementwiseQLinear into this path, so that the dimensions match.
Motivation and Context
This is required to ensure the DML EP correctly handles this scenario.