Skip to content

Conversation

@mikem8361
Copy link
Contributor

There are memory regions > 4GB in size that overflowed a 32bit size value. Changed to 64bit.

Fixes issue: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1277488?src=WorkItemMention&src-action=artifact_link

This also may be the cause of issue dotnet/diagnostics#1780

@mikem8361 mikem8361 requested a review from sdmaclea March 11, 2021 01:35
@mikem8361 mikem8361 self-assigned this Mar 11, 2021
@ghost
Copy link

ghost commented Mar 11, 2021

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

There are memory regions > 4GB in size that overflowed a 32bit size value. Changed to 64bit.

Fixes issue: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1277488?src=WorkItemMention&src-action=artifact_link

This also may be the cause of issue dotnet/diagnostics#1780

Author: mikem8361
Assignees: mikem8361
Labels:

area-Diagnostics-coreclr

Milestone: -

Copy link
Contributor

@sdmaclea sdmaclea left a comment

Choose a reason for hiding this comment

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

LGTM
z --> size_t
Learn something new every day

Copy link
Contributor

@sdmaclea sdmaclea Mar 11, 2021

Choose a reason for hiding this comment

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

size should probably be declared above as size_t ... Otherwise

Suggested change
size_t bytesToRead = std::min(size, sizeof(m_tempBuffer));
size_t bytesToRead = std::min((size_t)size, sizeof(m_tempBuffer));
2021-03-11T02:11:41.5645060Z /Users/runner/work/1/s/src/coreclr/debug/createdump/dumpwriter.cpp:212:38: error: no matching function for call to 'min'
2021-03-11T02:11:41.5744730Z                 size_t bytesToRead = std::min(size, sizeof(m_tempBuffer));
2021-03-11T02:11:41.5845290Z                                      ^~~~~~~~
2021-03-11T02:11:41.5947840Z /Applications/Xcode_12.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2560:1: note: candidate template ignored: deduced conflicting types for parameter '_Tp' ('unsigned long long' vs. 'unsigned long')
2021-03-11T02:11:41.6050380Z min(const _Tp& __a, const _Tp& __b)
2021-03-11T02:11:41.6152150Z ^
2021-03-11T02:11:41.6255530Z /Applications/Xcode_12.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2571:1: note: candidate template ignored: could not match 'initializer_list<type-parameter-0-0>' against 'unsigned long long'
2021-03-11T02:11:41.6357920Z min(initializer_list<_Tp> __t, _Compare __comp)
2021-03-11T02:11:41.6460080Z ^
2021-03-11T02:11:41.6561170Z /Applications/Xcode_12.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2551:1: note: candidate function template not viable: requires 3 arguments, but 2 were provided
2021-03-11T02:11:41.6662240Z min(const _Tp& __a, const _Tp& __b, _Compare __comp)
2021-03-11T02:11:41.6761820Z ^
2021-03-11T02:11:41.6863770Z /Applications/Xcode_12.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/algorithm:2580:1: note: candidate function template not viable: requires single argument '__t', but 2 arguments were provided
2021-03-11T02:11:41.6993010Z min(initializer_list<_Tp> __t)
2021-03-11T02:11:41.7195180Z ^
2021-03-11T02:11:42.0217020Z 1 error generated.

@ghost
Copy link

ghost commented Mar 11, 2021

Hello @mikem8361!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit d3e69e0 into dotnet:main Mar 11, 2021
@mikem8361 mikem8361 deleted the fixcreatedump branch March 11, 2021 21:13
mikem8361 pushed a commit to mikem8361/runtime that referenced this pull request Mar 16, 2021
The core dump generated for a app that has large GC heaps (>4GB) are don't contain all the memory needed in process. This is because of a 32bit size value overflow; changed to size_t.

Multiple customers have reported this problem in 3.1 and 5.0.

Issue: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1277488?src=WorkItemMention&src-action=artifact_link and dotnet/diagnostics#1780
Anipik pushed a commit that referenced this pull request Apr 6, 2021
)

The core dump generated for a app that has large GC heaps (>4GB) are don't contain all the memory needed in process. This is because of a 32bit size value overflow; changed to size_t.

Multiple customers have reported this problem in 3.1 and 5.0.

Issue: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1277488?src=WorkItemMention&src-action=artifact_link and dotnet/diagnostics#1780
@ghost ghost locked as resolved and limited conversation to collaborators Apr 10, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants