Skip to content

Fix stack_limit handling - #90937

Merged
janvorli merged 4 commits into
dotnet:mainfrom
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged

Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom
janvorli:fix-stack-limit-issue-2

Conversation

@janvorli

@janvorlijanvorli commented Aug 22, 2023

Copy link
Copy Markdown
Member

There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.

But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.

Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.

Close#86265

@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli added this to the 8.0.0 milestone Aug 22, 2023
@janvorli
janvorli requested a review from jkotasAugust 22, 2023 17:35
@janvorlijanvorli self-assigned this Aug 22, 2023
@ghost

ghost commented Aug 22, 2023

Copy link
Copy Markdown

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

Issue Details

There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.

But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.

Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.

Close #86265

Author:janvorli
Assignees:janvorli
Labels:

area-GC-coreclr

Milestone:8.0.0

jkotas
jkotas reviewed Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h Outdated
@janvorli
janvorliforce-pushed the fix-stack-limit-issue-2 branch from 2b29f47 to a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas reviewed Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp Outdated
jkotas
jkotas reviewed Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp Outdated
jkotas
jkotas approved these changes Aug 23, 2023
View reviewed changes

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!

@janvorlijanvorli reopened this Aug 24, 2023
@janvorli
janvorli merged commit 445f01d into dotnet:mainAug 24, 2023
@janvorli

janvorli commented Aug 24, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0

@github-actions

github-actionsBot commented Aug 24, 2023

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487

@github-actionsgithub-actionsBot mentioned this pull request Aug 24, 2023
Merged
@ghostghost locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll

3 participants

@janvorli@jkotas@Maoni0
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Fix stack_limit handling by janvorli · Pull Request #90937 · dotnet/runtime · GitHub
Skip to content
Fix stack_limit handling - #90937
Merged
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged
Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Conversation
@janvorli
@janvorlijanvorli
commented
Aug 22, 2023

Copy link


Copy Markdown

Member
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close#86265
@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli
added this to the 8.0.0 milestone
Aug 22, 2023
@janvorli
janvorli
requested a review
from jkotasAugust 22, 2023 17:35
@janvorlijanvorli
self-assigned this
Aug 22, 2023
@ghost
ghost
commented
Aug 22, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/gc

See info in area-owners.md if you want to be subscribed.
Issue Details
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close #86265
Author:janvorli
Assignees:janvorli
Labels:
area-GC-coreclr
Milestone:8.0.0
jkotas
jkotas
reviewed
Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h
Outdated

@janvorli
janvorliforce-pushed
the
fix-stack-limit-issue-2
branch
from
2b29f47 to
a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
approved these changes
Aug 23, 2023
View reviewed changes
@jkotasjkotas
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM. Thanks!
@janvorlijanvorli
reopened this
Aug 24, 2023
@janvorli
janvorli
merged commit 445f01d
into
dotnet:mainAug 24, 2023
@janvorli
janvorli
commented
Aug 24, 2023

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0
@github-actions
github-actionsBot
commented
Aug 24, 2023

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487
@github-actionsgithub-actionsBot
mentioned this pull request
Aug 24, 2023
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects


None yet


Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll


3 participants

@janvorli@jkotas@Maoni0

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix stack_limit handling by janvorli · Pull Request #90937 · dotnet/runtime · GitHub
Skip to content
Fix stack_limit handling - #90937
Merged
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged
Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Conversation
@janvorli
@janvorlijanvorli
commented
Aug 22, 2023

Copy link


Copy Markdown

Member
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close#86265
@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli
added this to the 8.0.0 milestone
Aug 22, 2023
@janvorli
janvorli
requested a review
from jkotasAugust 22, 2023 17:35
@janvorlijanvorli
self-assigned this
Aug 22, 2023
@ghost
ghost
commented
Aug 22, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/gc

See info in area-owners.md if you want to be subscribed.
Issue Details
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close #86265
Author:janvorli
Assignees:janvorli
Labels:
area-GC-coreclr
Milestone:8.0.0
jkotas
jkotas
reviewed
Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h
Outdated

@janvorli
janvorliforce-pushed
the
fix-stack-limit-issue-2
branch
from
2b29f47 to
a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
approved these changes
Aug 23, 2023
View reviewed changes
@jkotasjkotas
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM. Thanks!
@janvorlijanvorli
reopened this
Aug 24, 2023
@janvorli
janvorli
merged commit 445f01d
into
dotnet:mainAug 24, 2023
@janvorli
janvorli
commented
Aug 24, 2023

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0
@github-actions
github-actionsBot
commented
Aug 24, 2023

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487
@github-actionsgithub-actionsBot
mentioned this pull request
Aug 24, 2023
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects


None yet


Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll


3 participants

@janvorli@jkotas@Maoni0

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix stack_limit handling by janvorli · Pull Request #90937 · dotnet/runtime · GitHub
Skip to content
Fix stack_limit handling - #90937
Merged
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged
Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Conversation
@janvorli
@janvorlijanvorli
commented
Aug 22, 2023

Copy link


Copy Markdown

Member
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close#86265
@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli
added this to the 8.0.0 milestone
Aug 22, 2023
@janvorli
janvorli
requested a review
from jkotasAugust 22, 2023 17:35
@janvorlijanvorli
self-assigned this
Aug 22, 2023
@ghost
ghost
commented
Aug 22, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/gc

See info in area-owners.md if you want to be subscribed.
Issue Details
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close #86265
Author:janvorli
Assignees:janvorli
Labels:
area-GC-coreclr
Milestone:8.0.0
jkotas
jkotas
reviewed
Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h
Outdated

@janvorli
janvorliforce-pushed
the
fix-stack-limit-issue-2
branch
from
2b29f47 to
a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
approved these changes
Aug 23, 2023
View reviewed changes
@jkotasjkotas
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM. Thanks!
@janvorlijanvorli
reopened this
Aug 24, 2023
@janvorli
janvorli
merged commit 445f01d
into
dotnet:mainAug 24, 2023
@janvorli
janvorli
commented
Aug 24, 2023

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0
@github-actions
github-actionsBot
commented
Aug 24, 2023

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487
@github-actionsgithub-actionsBot
mentioned this pull request
Aug 24, 2023
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects


None yet


Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll


3 participants

@janvorli@jkotas@Maoni0

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Fix stack_limit handling by janvorli · Pull Request #90937 · dotnet/runtime · GitHub
Skip to content
Fix stack_limit handling - #90937
Merged
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged
Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Conversation
@janvorli
@janvorlijanvorli
commented
Aug 22, 2023

Copy link


Copy Markdown

Member
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close#86265
@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli
added this to the 8.0.0 milestone
Aug 22, 2023
@janvorli
janvorli
requested a review
from jkotasAugust 22, 2023 17:35
@janvorlijanvorli
self-assigned this
Aug 22, 2023
@ghost
ghost
commented
Aug 22, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/gc

See info in area-owners.md if you want to be subscribed.
Issue Details
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close #86265
Author:janvorli
Assignees:janvorli
Labels:
area-GC-coreclr
Milestone:8.0.0
jkotas
jkotas
reviewed
Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h
Outdated

@janvorli
janvorliforce-pushed
the
fix-stack-limit-issue-2
branch
from
2b29f47 to
a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
approved these changes
Aug 23, 2023
View reviewed changes
@jkotasjkotas
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM. Thanks!
@janvorlijanvorli
reopened this
Aug 24, 2023
@janvorli
janvorli
merged commit 445f01d
into
dotnet:mainAug 24, 2023
@janvorli
janvorli
commented
Aug 24, 2023

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0
@github-actions
github-actionsBot
commented
Aug 24, 2023

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487
@github-actionsgithub-actionsBot
mentioned this pull request
Aug 24, 2023
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects


None yet


Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll


3 participants

@janvorli@jkotas@Maoni0

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix stack_limit handling by janvorli · Pull Request #90937 · dotnet/runtime · GitHub
Skip to content
Fix stack_limit handling - #90937
Merged
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged
Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Conversation
@janvorli
@janvorlijanvorli
commented
Aug 22, 2023

Copy link


Copy Markdown

Member
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close#86265
@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli
added this to the 8.0.0 milestone
Aug 22, 2023
@janvorli
janvorli
requested a review
from jkotasAugust 22, 2023 17:35
@janvorlijanvorli
self-assigned this
Aug 22, 2023
@ghost
ghost
commented
Aug 22, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/gc

See info in area-owners.md if you want to be subscribed.
Issue Details
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close #86265
Author:janvorli
Assignees:janvorli
Labels:
area-GC-coreclr
Milestone:8.0.0
jkotas
jkotas
reviewed
Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h
Outdated

@janvorli
janvorliforce-pushed
the
fix-stack-limit-issue-2
branch
from
2b29f47 to
a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
approved these changes
Aug 23, 2023
View reviewed changes
@jkotasjkotas
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM. Thanks!
@janvorlijanvorli
reopened this
Aug 24, 2023
@janvorli
janvorli
merged commit 445f01d
into
dotnet:mainAug 24, 2023
@janvorli
janvorli
commented
Aug 24, 2023

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0
@github-actions
github-actionsBot
commented
Aug 24, 2023

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487
@github-actionsgithub-actionsBot
mentioned this pull request
Aug 24, 2023
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects


None yet


Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll


3 participants

@janvorli@jkotas@Maoni0

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
})();
Fix stack_limit handling by janvorli · Pull Request #90937 · dotnet/runtime · GitHub
Skip to content
Fix stack_limit handling - #90937
Merged
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Aug 24, 2023
Merged
Fix stack_limit handling#90937
janvorli merged 4 commits into
dotnet:mainfrom 
janvorli:fix-stack-limit-issue-2
Conversation
@janvorli
@janvorlijanvorli
commented
Aug 22, 2023

Copy link


Copy Markdown

Member
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close#86265
@janvorli
Fix stack_limit handling
d9bda2f
There is a problem with computing stack_limit in the presence of
inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that
frame to get the call site SP. But when the inlined call frame is not
active (there is no call to native code in progress), that method
returns random junk.
But even if we checked for an active call before reading the value, it
would not get correct stack limit on x86 windows when there was no
active call with the inlined call frame being the topmost one. That
can happen with hardware exception handling on Windows x86. On other
targets, we always have other explicit frame on top of the explicit
frames stack, but on windows x86, we don't use the FaultingExceptionFrame for
hardware exceptions, so the inactive inlined call frame could be the
topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I
have fixed that by disabling the stack_limit usage for x86 windows.
Closedotnet#86265
@janvorlijanvorli
added this to the 8.0.0 milestone
Aug 22, 2023
@janvorli
janvorli
requested a review
from jkotasAugust 22, 2023 17:35
@janvorlijanvorli
self-assigned this
Aug 22, 2023
@ghost
ghost
commented
Aug 22, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/gc

See info in area-owners.md if you want to be subscribed.
Issue Details
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk.
But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots.
Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows.
Close #86265
Author:janvorli
Assignees:janvorli
Labels:
area-GC-coreclr
Milestone:8.0.0
jkotas
jkotas
reviewed
Aug 22, 2023
View reviewed changes
Comment threadsrc/coreclr/gc/gcinterface.h
Outdated

@janvorli
janvorliforce-pushed
the
fix-stack-limit-issue-2
branch
from
2b29f47 to
a9b1c25CompareAugust 22, 2023 21:21
jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
reviewed
Aug 23, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/gcenv.ee.cpp
Outdated

jkotas
jkotas
approved these changes
Aug 23, 2023
View reviewed changes
@jkotasjkotas
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM. Thanks!
@janvorlijanvorli
reopened this
Aug 24, 2023
@janvorli
janvorli
merged commit 445f01d
into
dotnet:mainAug 24, 2023
@janvorli
janvorli
commented
Aug 24, 2023

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0
@github-actions
github-actionsBot
commented
Aug 24, 2023

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968753487
@github-actionsgithub-actionsBot
mentioned this pull request
Aug 24, 2023
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Sep 24, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@Maoni0Maoni0Maoni0 left review comments
@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovsky

Assignees

@janvorlijanvorli

Labels

area-GC-coreclr

Projects


None yet


Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Test failure: JIT/opt/Vectorization/UnrollEqualsStartsWIth/UnrollEqualsStartsWIth.dll


3 participants

@janvorli@jkotas@Maoni0