Skip to content

Fix memleakdbg call stack output. - #24542

Merged
Edward Chen (edgchen1) merged 5 commits into
mainfrom
edgchen1/fix_debug_alloc_callstack
Apr 25, 2025
Merged

Fix memleakdbg call stack output.#24542
Edward Chen (edgchen1) merged 5 commits into
mainfrom
edgchen1/fix_debug_alloc_callstack

Conversation

@edgchen1

@edgchen1 Edward Chen (edgchen1) commented Apr 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Fix memleakdbg call stack output.

The call stack output was getting clobbered:
C:\dev\onnxruntime\build\Debug\_deps\googletest-src\googletest\include\gtest\internal\gtest-port.h(1631): l\gtest-port.h(1631): eadLocal<testing::Sequence *>::GetOrCreateValue

I think the issue is that this aliasing of buffer and symbol:

onnxruntime/onnxruntime/core/platform/windows/debug_alloc.cc

Lines 97 to 100 in 173a11a

alignas(SYMBOL_INFO) char buffer[kInitialBufferSize] = {0};
SYMBOL_INFO* symbol = reinterpret_cast<SYMBOL_INFO*>(buffer);
symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
symbol->MaxNameLen = MAX_SYM_NAME;

does not play nicely with a call to _snprintf_s like this:

onnxruntime/onnxruntime/core/platform/windows/debug_alloc.cc

Line 115 in 173a11a

_snprintf_s(buffer, _TRUNCATE, "%s(%d): %s", line.FileName, static_cast<int>(line.LineNumber), symbol->Name);

The clobbered output does not match the predefined, ignored patterns, so we see spurious mem leak check output.

This change updates the memleakdbg output generation to use C++ ostreams and instead of fixed size buffers and _snprintf_s.

Motivation and Context

Fix spurious mem leak check output.
Fix #24535.

edgchen1
Edward Chen (edgchen1) commented Apr 25, 2025
View reviewed changes
Comment thread onnxruntime/core/platform/windows/debug_alloc.cc Outdated
RyanUnderhill
Ryan Hill (RyanUnderhill) reviewed Apr 25, 2025
View reviewed changes
Comment thread onnxruntime/core/platform/windows/debug_alloc.cc Outdated
@edgchen1
Edward Chen (edgchen1) merged commit d4c9b92 into main Apr 25, 2025
@edgchen1
Edward Chen (edgchen1) deleted the edgchen1/fix_debug_alloc_callstack branch April 25, 2025 23:32
vraspar pushed a commit that referenced this pull request Apr 29, 2025
@edgchen1 @vraspar
Fix memleakdbg call stack output. (#24542)
9307071 
### Description
<!-- Describe your changes. -->

Fix memleakdbg call stack output.

The call stack output was getting clobbered:

`C:\dev\onnxruntime\build\Debug\_deps\googletest-src\googletest\include\gtest\internal\gtest-port.h(1631):
l\gtest-port.h(1631): eadLocal<testing::Sequence *>::GetOrCreateValue`

I think the issue is that this aliasing of `buffer` and `symbol`:

https://github.com/microsoft/onnxruntime/blob/173a11a4e7a2f7a360c9db6abbe601a06a16f004/onnxruntime/core/platform/windows/debug_alloc.cc#L97-L100

does not play nicely with a call to `_snprintf_s` like this:

https://github.com/microsoft/onnxruntime/blob/173a11a4e7a2f7a360c9db6abbe601a06a16f004/onnxruntime/core/platform/windows/debug_alloc.cc#L115

The clobbered output does not match the predefined, ignored patterns, so
we see spurious mem leak check output.

This change updates the memleakdbg output generation to use C++ ostreams
and instead of fixed size buffers and `_snprintf_s`.

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

Fix spurious mem leak check output.
Fix #24535.
@vraspar vraspar mentioned this pull request Apr 29, 2025
Merged
George Wu (jywu-mysoft) pushed a commit that referenced this pull request Apr 30, 2025
@vraspar @adrianlizarraga
@minfhong-quic minfhong-quic @justinchuby @prathikr @edgchen1 @ankan-ban @gedoensmax @gaugarg-nv @ishwar-raut1 @hrishikeshm @skottmckay @qjia7 @kunal-vaishnavi @xhcao
Cherry-picks into rel-1.22.0 (#24580)
ef546e9 
### Description

Cherry pick the following into
[rel-1.22.0](https://github.com/microsoft/onnxruntime/tree/rel-1.22.0)


- (#24487)
- (#24466)
- (#24493)
- (#24484)
- (#24494)
- (#24489)
- (#24504)
- (#24510)
- (#24456)
- (#24537)
- (#24501)
- (#24519)
- (#24513)
- (#24539)
- (#24514)
- (#24542)
- (#24585)

Not added:

Planning to cherry pick Cuda Matmulnbits PRs once the fix for failing
cuda pipeline is ready
- (#24491)
- (#24509)
- (#24564)

---------

Co-authored-by: Adrian Lizarraga <adlizarraga@microsoft.com>
Co-authored-by: minfhong-quic <quic_minfhong@quicinc.com>
Co-authored-by: minfhong-quic <minfhong-quic@quicinc.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Prathik Rao <prathik.rao@gmail.com>
Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com>
Co-authored-by: Ankan Banerjee <ankan.ban@gmail.com>
Co-authored-by: Maximilian Müller <maximilianm@nvidia.com>
Co-authored-by: Gaurav Garg <gaugarg@nvidia.com>
Co-authored-by: iraut <iraut@nvidia.com>
Co-authored-by: Hrishikesh Manohar <hrishikeshm@nvidia.com>
Co-authored-by: Maximilian Müller <44298237+gedoensmax@users.noreply.github.com>
Co-authored-by: Scott McKay <skottmckay@gmail.com>
Co-authored-by: Jiajia Qin <jiajiaqin@microsoft.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: xhcao <xinghua.cao@intel.com>
Jatin Wadhwa (jatinwadhwa921) pushed a commit to intel/onnxruntime that referenced this pull request Apr 30, 2025
@ankitm3k @vraspar
@adrianlizarraga @minfhong-quic minfhong-quic @justinchuby @prathikr @edgchen1 @ankan-ban @gedoensmax @gaugarg-nv @ishwar-raut1 @hrishikeshm @skottmckay @qjia7 @kunal-vaishnavi @xhcao
Cherry-picks into rel-1.22.0 (microsoft#24580) (#680)
c531a61 
### Description

Cherry pick the following into
[rel-1.22.0](https://github.com/microsoft/onnxruntime/tree/rel-1.22.0)


- (microsoft#24487)
- (microsoft#24466)
- (microsoft#24493)
- (microsoft#24484)
- (microsoft#24494)
- (microsoft#24489)
- (microsoft#24504)
- (microsoft#24510)
- (microsoft#24456)
- (microsoft#24537)
- (microsoft#24501)
- (microsoft#24519)
- (microsoft#24513)
- (microsoft#24539)
- (microsoft#24514)
- (microsoft#24542)
- (microsoft#24585)

Not added:

Planning to cherry pick Cuda Matmulnbits PRs once the fix for failing
cuda pipeline is ready
- (microsoft#24491)
- (microsoft#24509)
- (microsoft#24564)

---------

Co-authored-by: vraspar <vrajang@outlook.com>
Co-authored-by: Adrian Lizarraga <adlizarraga@microsoft.com>
Co-authored-by: minfhong-quic <quic_minfhong@quicinc.com>
Co-authored-by: minfhong-quic <minfhong-quic@quicinc.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Prathik Rao <prathik.rao@gmail.com>
Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com>
Co-authored-by: Ankan Banerjee <ankan.ban@gmail.com>
Co-authored-by: Maximilian Müller <maximilianm@nvidia.com>
Co-authored-by: Gaurav Garg <gaugarg@nvidia.com>
Co-authored-by: iraut <iraut@nvidia.com>
Co-authored-by: Hrishikesh Manohar <hrishikeshm@nvidia.com>
Co-authored-by: Maximilian Müller <44298237+gedoensmax@users.noreply.github.com>
Co-authored-by: Scott McKay <skottmckay@gmail.com>
Co-authored-by: Jiajia Qin <jiajiaqin@microsoft.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: xhcao <xinghua.cao@intel.com>
Ankit Maheshkar (ankitm3k) pushed a commit to intel/onnxruntime that referenced this pull request May 12, 2025
@edgchen1 @ankitm3k
Fix memleakdbg call stack output. (microsoft#24542)
39ab256 
### Description
<!-- Describe your changes. -->

Fix memleakdbg call stack output.

The call stack output was getting clobbered:

`C:\dev\onnxruntime\build\Debug\_deps\googletest-src\googletest\include\gtest\internal\gtest-port.h(1631):
l\gtest-port.h(1631): eadLocal<testing::Sequence *>::GetOrCreateValue`

I think the issue is that this aliasing of `buffer` and `symbol`:

https://github.com/microsoft/onnxruntime/blob/173a11a4e7a2f7a360c9db6abbe601a06a16f004/onnxruntime/core/platform/windows/debug_alloc.cc#L97-L100

does not play nicely with a call to `_snprintf_s` like this:

https://github.com/microsoft/onnxruntime/blob/173a11a4e7a2f7a360c9db6abbe601a06a16f004/onnxruntime/core/platform/windows/debug_alloc.cc#L115

The clobbered output does not match the predefined, ignored patterns, so
we see spurious mem leak check output.

This change updates the memleakdbg output generation to use C++ ostreams
and instead of fixed size buffers and `_snprintf_s`.

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

Fix spurious mem leak check output.
Fix microsoft#24535.
Sam Webster (samwebster) added a commit that referenced this pull request Jun 10, 2025
@samwebster
Revert "Fix memleakdbg call stack output. (#24542)"
30c87bd 
This reverts commit d4c9b92.
@samwebster Sam Webster (samwebster) mentioned this pull request Jun 10, 2025
Closed
@snnn

Changming Sun (snnn) commented Sep 5, 2025

Copy link
Copy Markdown
Contributor

This PR has been included in the rel-1.22.0 branch. Removing the release:1.22.0 label.

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

Reviewers

@yuslepukhin Dmitri Smirnov (yuslepukhin) Awaiting requested review from yuslepukhin
2 more reviewers
@RyanUnderhill Ryan Hill (RyanUnderhill) RyanUnderhill left review comments
@snnn Changming Sun (snnn) snnn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ORT Debug Build Fails to Infer for Every Model

5 participants

@edgchen1 @snnn @yuslepukhin @RyanUnderhill @vraspar