Skip to content

Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
janvorli merged 2 commits into from
Sep 9, 2021
Merged

Fix x86 EH for UnmanagedCallersOnly methods #58796

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a943ad5
Fix x86 EH for UnmanagedCallersOnly methods
janvorli Sep 8, 2021
36861ee
Reflect PR feedback, use GCInfo instead of parsing the attribute
janvorli Sep 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/coreclr/vm/stackwalk.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
hdrInfo gcHdrInfo;
DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);

@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?

@janvorlijanvorliSep 8, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.

bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

ProcessIp(GetControlPC(m_crawl.pRD));

// if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
m_frameState = SFITER_NATIVE_MARKER_FRAME;
m_crawl.isNativeMarker = true;
}
#ifdef TARGET_X86
else if (hasReversePInvoke)
{
// The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
// frame, that means that the method was called from managed code without any native frames in between.
// On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
// for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
// So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
_ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
m_crawl.isFrameless = false;
}
#endif
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down
, '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 x86 EH for UnmanagedCallersOnly methods by janvorli · Pull Request #58796 · dotnet/runtime · GitHub
Skip to content
Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

janvorli
merged 2 commits into

from

Sep 9, 2021
 Merged


Fix x86 EH for UnmanagedCallersOnly methods
#58796
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a943ad5

Fix x86 EH for UnmanagedCallersOnly methods


janvorli Sep 8, 2021
36861ee

Reflect PR feedback, use GCInfo instead of parsing the attribute


janvorli Sep 8, 2021

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


20 changes: 20 additions & 0 deletions
src/coreclr/vm/stackwalk.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
 hdrInfo gcHdrInfo;
 DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);
@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?
@janvorlijanvorliSep 8, 2021

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.
 bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

 ProcessIp(GetControlPC(m_crawl.pRD));

 // if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_frameState = SFITER_NATIVE_MARKER_FRAME;
 m_crawl.isNativeMarker = true;
 }
#ifdef TARGET_X86
 else if (hasReversePInvoke)
 {
 // The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
 // frame, that means that the method was called from managed code without any native frames in between. 
 // On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
 // for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
 // So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
 _ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
 m_crawl.isFrameless = false;
 }
#endif 
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down

, '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 x86 EH for UnmanagedCallersOnly methods by janvorli · Pull Request #58796 · dotnet/runtime · GitHub
Skip to content
Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

janvorli
merged 2 commits into

from

Sep 9, 2021
 Merged


Fix x86 EH for UnmanagedCallersOnly methods
#58796
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a943ad5

Fix x86 EH for UnmanagedCallersOnly methods


janvorli Sep 8, 2021
36861ee

Reflect PR feedback, use GCInfo instead of parsing the attribute


janvorli Sep 8, 2021

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


20 changes: 20 additions & 0 deletions
src/coreclr/vm/stackwalk.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
 hdrInfo gcHdrInfo;
 DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);
@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?
@janvorlijanvorliSep 8, 2021

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.
 bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

 ProcessIp(GetControlPC(m_crawl.pRD));

 // if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_frameState = SFITER_NATIVE_MARKER_FRAME;
 m_crawl.isNativeMarker = true;
 }
#ifdef TARGET_X86
 else if (hasReversePInvoke)
 {
 // The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
 // frame, that means that the method was called from managed code without any native frames in between. 
 // On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
 // for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
 // So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
 _ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
 m_crawl.isFrameless = false;
 }
#endif 
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down

, '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 x86 EH for UnmanagedCallersOnly methods by janvorli · Pull Request #58796 · dotnet/runtime · GitHub
Skip to content
Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

janvorli
merged 2 commits into

from

Sep 9, 2021
 Merged


Fix x86 EH for UnmanagedCallersOnly methods
#58796
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a943ad5

Fix x86 EH for UnmanagedCallersOnly methods


janvorli Sep 8, 2021
36861ee

Reflect PR feedback, use GCInfo instead of parsing the attribute


janvorli Sep 8, 2021

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


20 changes: 20 additions & 0 deletions
src/coreclr/vm/stackwalk.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
 hdrInfo gcHdrInfo;
 DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);
@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?
@janvorlijanvorliSep 8, 2021

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.
 bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

 ProcessIp(GetControlPC(m_crawl.pRD));

 // if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_frameState = SFITER_NATIVE_MARKER_FRAME;
 m_crawl.isNativeMarker = true;
 }
#ifdef TARGET_X86
 else if (hasReversePInvoke)
 {
 // The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
 // frame, that means that the method was called from managed code without any native frames in between. 
 // On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
 // for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
 // So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
 _ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
 m_crawl.isFrameless = false;
 }
#endif 
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down

, '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 x86 EH for UnmanagedCallersOnly methods by janvorli · Pull Request #58796 · dotnet/runtime · GitHub
Skip to content
Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

janvorli
merged 2 commits into

from

Sep 9, 2021
 Merged


Fix x86 EH for UnmanagedCallersOnly methods
#58796
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a943ad5

Fix x86 EH for UnmanagedCallersOnly methods


janvorli Sep 8, 2021
36861ee

Reflect PR feedback, use GCInfo instead of parsing the attribute


janvorli Sep 8, 2021

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


20 changes: 20 additions & 0 deletions
src/coreclr/vm/stackwalk.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
 hdrInfo gcHdrInfo;
 DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);
@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?
@janvorlijanvorliSep 8, 2021

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.
 bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

 ProcessIp(GetControlPC(m_crawl.pRD));

 // if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_frameState = SFITER_NATIVE_MARKER_FRAME;
 m_crawl.isNativeMarker = true;
 }
#ifdef TARGET_X86
 else if (hasReversePInvoke)
 {
 // The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
 // frame, that means that the method was called from managed code without any native frames in between. 
 // On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
 // for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
 // So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
 _ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
 m_crawl.isFrameless = false;
 }
#endif 
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down

, '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 x86 EH for UnmanagedCallersOnly methods by janvorli · Pull Request #58796 · dotnet/runtime · GitHub
Skip to content
Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

janvorli
merged 2 commits into

from

Sep 9, 2021
 Merged


Fix x86 EH for UnmanagedCallersOnly methods
#58796
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a943ad5

Fix x86 EH for UnmanagedCallersOnly methods


janvorli Sep 8, 2021
36861ee

Reflect PR feedback, use GCInfo instead of parsing the attribute


janvorli Sep 8, 2021

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


20 changes: 20 additions & 0 deletions
src/coreclr/vm/stackwalk.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
 hdrInfo gcHdrInfo;
 DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);
@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?
@janvorlijanvorliSep 8, 2021

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.
 bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

 ProcessIp(GetControlPC(m_crawl.pRD));

 // if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_frameState = SFITER_NATIVE_MARKER_FRAME;
 m_crawl.isNativeMarker = true;
 }
#ifdef TARGET_X86
 else if (hasReversePInvoke)
 {
 // The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
 // frame, that means that the method was called from managed code without any native frames in between. 
 // On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
 // for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
 // So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
 _ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
 m_crawl.isFrameless = false;
 }
#endif 
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down

, '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 x86 EH for UnmanagedCallersOnly methods by janvorli · Pull Request #58796 · dotnet/runtime · GitHub
Skip to content
Fix x86 EH for UnmanagedCallersOnly methods#58796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

janvorli
merged 2 commits into

from

Sep 9, 2021
 Merged


Fix x86 EH for UnmanagedCallersOnly methods
#58796
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a943ad5

Fix x86 EH for UnmanagedCallersOnly methods


janvorli Sep 8, 2021
36861ee

Reflect PR feedback, use GCInfo instead of parsing the attribute


janvorli Sep 8, 2021

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


20 changes: 20 additions & 0 deletions
src/coreclr/vm/stackwalk.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,8 @@
#include "interpreter.h"
#endif // FEATURE_INTERPRETER

#include "gcinfodecoder.h"

#ifdef FEATURE_EH_FUNCLETS
#define PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME
#endif
Expand DownExpand Up@@ -3135,6 +3137,12 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_exInfoWalk.WalkToPosition(GetRegdisplaySP(m_crawl.pRD), (m_flags & POPFRAMES));
#endif // ELIMINATE_FEF

#ifdef TARGET_X86
 hdrInfo gcHdrInfo;
 DecodeGCHdrInfo(m_crawl.codeInfo.GetGCInfoToken(), 0, &gcHdrInfo);
@jkotasjkotasSep 8, 2021

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.

All other places that decode GCInfo in this file go through m_crawl.GetCodeManager(). Is it worth following the pattern?
@janvorlijanvorliSep 8, 2021

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Good idea, but let me do that as a separate cleanup so that this change is just a fix for the bug. I will need to add a new method to the EECodeManager and then I'd use it in the exceptionhandling.cpp too where we currently also have this raw decoding.
 bool hasReversePInvoke = gcHdrInfo.revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // TARGET_X86

 ProcessIp(GetControlPC(m_crawl.pRD));

 // if we have unwound to a native stack frame, stop and set the frame state accordingly
Expand All@@ -3143,6 +3151,18 @@ void StackFrameIterator::PostProcessingForManagedFrames(void)
 m_frameState = SFITER_NATIVE_MARKER_FRAME;
 m_crawl.isNativeMarker = true;
 }
#ifdef TARGET_X86
 else if (hasReversePInvoke)
 {
 // The managed frame we've unwound from had reverse PInvoke frame. Since we are on a frameless
 // frame, that means that the method was called from managed code without any native frames in between. 
 // On x86, the InlinedCallFrame of the pinvoke would get skipped as we've just unwound to the pinvoke IL stub and
 // for this architecture, the inlined call frames are supposed to be processed before the managed frame they are stored in.
 // So we force the stack frame iterator to process the InlinedCallFrame before the IL stub.
 _ASSERTE(InlinedCallFrame::FrameHasActiveCall(m_crawl.pFrame));
 m_crawl.isFrameless = false;
 }
#endif 
} // StackFrameIterator::PostProcessingForManagedFrames()

//---------------------------------------------------------------------------------------
Expand Down