Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, '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" + '
OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, '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('^' + ".*" + ' OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, '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('^' + ".*" + ' OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, '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" + ' OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, '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('^' + ".*" + ' OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, '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); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); OSX unwind workaround by christianscheuer · Pull Request #2166 · dotnet/corert · GitHub
Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

OSX unwind workaround - #2166

Merged
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround
Nov 26, 2016
Merged

OSX unwind workaround#2166
jkotas merged 11 commits into
dotnet:masterfrom
christianscheuer:osx-unwind-workaround

Conversation

@christianscheuer

Copy link
Copy Markdown
Contributor

Work in progress.

I have done a workaround for the missing OSX unwind as you suggested, @jkotas (see #1867).
I might be in a little deep here though, so forgive me if I have misunderstood anything.

Now when I run a real-world OSX app on this, it stops in RhpCallCatchFunclet:
frame #0: 0x000000010005fad2 cptsbackendRhpCallCatchFunclet + 63`

At this point:

 0x10005fad2 <+63>: movq (%rax), %rbx
0x10005fad5 <+66>: movq 0x20(%rdx), %rax
0x10005fad9 <+70>: movq (%rax), %rbp
0x10005fadc <+73>: movq 0x58(%rdx), %rax

Where the value of rax is 0x0.

It seems to be this code in RhpCallCatchFunclet

 mov rax, [r8 + OFFSETOF__REGDISPLAY__pRbx]
mov rbx, [rax]

Where the RegDisplay's pRbx is 0x0.

I don't know if that happens due to a misalignment of offsets, if I did something wrong in other parts of the code, or if this is related to the original todo-item.

@dnfclas

Copy link
Copy Markdown

Hi @christianscheuer, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas

Copy link
Copy Markdown

@christianscheuer, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@jkotas

Copy link
Copy Markdown
Member

Where the value of rax is 0x0.

@janvorli Apparently, there is number of places where we expect to have pointers to registers. Could you please take a look and see whether it is worth it to try to fix all of them as a workaround, instead of just fixing the unwinder?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas Let me know if there is anything I can do to help push this. Right now this issue is blocking a lot of progress for me personally, hence my interest in seeking to resolve it before the March milestone. I'll be happy to invest considerable time in it, but I would need some guidance to get started :)

@janvorli

Copy link
Copy Markdown
Member

@jkotas, @christianscheuer what could work as a temporary workaround for now is to revert the change that @christianscheuer made and then:

  • stick PAL_LIMITED_CONTEXT into the REGDISPLAY for OSX only
  • update all necessary offset constants
  • in the UnixContext.cpp, in UnwindCursorToRegDisplay function, set all the pointers in REGDISPLAY to point to the corresponding members in the PAL_LIMITED_CONTEXT and add for OSX unw_get_reg for all the registers that have pointers in the REGDISPLAY.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

Great, thank you @janvorli. I'll give that a shot asap.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli PTLA.
I'm unsure if this is working as expected. I'm now getting an "Abort trap: 6" fatal error, with this backtrace:

frame #0: 0x00007fff91eda286 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff8b10b9f9 libsystem_pthread.dylib`pthread_kill + 90
frame #2: 0x00007fff8f8219ab libsystem_c.dylib`abort + 129
frame #3: 0x000000010005a279 cptsbackend`RaiseFailFastException + 9
frame #4: 0x000000010006808c cptsbackend`System_Private_CoreLib_System_Runtime_EH__FailFastViaClasslib + 48
frame #5: 0x000000010006ef2a cptsbackend`System_Private_CoreLib_System_Runtime_EH__UnhandledExceptionFailFastViaClasslib + 98

@janvorli

Copy link
Copy Markdown
Member

@christianscheuer - does this mean you were not getting the stack trace? The abort is expected to happen at the end, since unhandled exception is supposed to fail fast.
@jkotas should we be getting stack trace with your change?
In case we should be getting the stack trace, from the call stack shown above, it seems that in the UnhandledExceptionFailFastViaClasslib, the pFailFastFunction extracted using the RhpGetClasslibFunction was IntPtr.Zero, so maybe that would be the next problem here.
@christianscheuer can you set a breakpoint to RhpGetClasslibFunction and see whether we haven't found a code manager or whether the code manager hasn't found the classlib function?

@jkotas

Copy link
Copy Markdown
Member

I'm now getting an "Abort trap: 6" fatal error, with this backtrace

This is expected. You should also see message with exception type and message on the console. The stacktrace printing on unhandled exceptions does not work yet.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@janvorli Yes this is an exception that when compiled in CoreCLR is caught by a catch block and logged to console so the program can continue. With these bits in CoreRT it aborts the program. So it seems the catch handler is not called, making the exception bubble up.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas All I get in the console is:

Abort trap: 6


#else // !UNIX_AMD64_ABI

#if defined(__APPLE__)

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.

Nit: Could you please change the define to something like UNWIND_WORKAROUND that is defined for __APPLE__ so that all places related to this are easy to find?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes! Which header file would be the appropriate place to make this define?

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.

src\Native\CMakeLists.txt may be a good place

@janvorli

Copy link
Copy Markdown
Member

Hmm, we may need to set the register pointers in the UnwindCursorToRegDisplay little bit differently. There seem to be cases when the pointers were actually pointing to some location and we want to preserve that. E.g. the pRBP is set in the StackFrameIterator::InternalInit to point to a member in the TransitionFrame or the others pointing to the initial PAL_LIMITED_CONTEXT. Or we set them to values from thisFuncletPtrs at some point.
So, it seems that we should also ifdef-out the following piece of code in the UnwindCursorToRegDisplay for the UNWIND_WORAROUND and change the setting of the pointers so that we set them only if they were NULL.

#defineGET_CONTEXT_POINTER(unwReg, rdReg) GetContextPointer(cursor, unwContext, unwReg, &regDisplay->p##rdReg);
GET_CONTEXT_POINTERS
#undef GET_CONTEXT_POINTER

Hmm, actually, now that I think about it, I've just realized that the context pointers are always initialized at the beginning of the stack walk to point inside the initial PAL_LIMITED_CONTEXT (or the TransitionFrame).
So it seems that all we actually need to do is to ifdef out the above mentioned block. No other changes would be necessary. I am sorry for not realizing this earlier.

@jkotas

Copy link
Copy Markdown
Member

Also, you can try setting breaking at RhpThrowEx and RhpRethrow, and check whether you see good stacktrace under debugger. There is a bug in generation of unwind info on Unix: #1461 - the exception handling won't work through methods affected by this bug.

Comment threadsrc/Native/CMakeLists.txt Outdated
add_compile_options(-fPIC)
add_compile_options(-fvisibility=hidden)

if(CLR_CMAKE_PLATFORM_DARWIN)

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.

If the workaround is limited to a single place now, it is not very valueable to have it controlled via special define...

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Is this still WIP? Are the crashes that you have been running into before fixed now?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

@jkotas I am currently getting a new error which relates to this code using an await statement in a try block. I am trying to isolate it to see if this works as expected when not combined with the Task async/await pattern.

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I removed the Task pattern complexity from my code. It seems now that the stack's backtrace show correctly, but that it somehow resumes from the wrong address?

I am now getting an error here:

 * frame #0: 0x0000000100001d5f cptsbackend`__VirtualCall_src_Creatix_PTShortcuts_AxNodes_AxElement__get_UIElement + 3
frame #1: 0x000000010016d6b5 cptsbackend`src_Creatix_PTShortcuts_Actions_ClickButtonAction__Execute + 129
frame #2: 0x0000000100137138 cptsbackend`src_Creatix_PTShortcuts_Actions_AutoAction__Run + 80
...

This virtualcall is the argument Element.UIElement (a property call) in the following code:

result.Require(Element.UIElement, "ClickButtonAction requires UIElement");

result.Require(...) is the throwing method.
So we should not end up in the UIElement's virtualcall after the exception.

Any ideas?

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

This new issue I was encountering seems to be the same you are hitting here, @jkotas: #2254

@christianscheuer

Copy link
Copy Markdown
ContributorAuthor

I can now confirm that @jkotas' fix for #2254, #2271, solves the related problem I was encountering. So this workaround now should be good to go. I have simplified the ifdef again.

@jkotas

Copy link
Copy Markdown
Member

@christianscheuer Thank you for the confirmation.

@janvorli is working on the full fix for this problem (statically link a custom version of the unwinder with the runtime), but it does not hurt to have this workaround in master until that comes online.

@jkotasjkotas changed the title [WIP] OSX unwind workaroundOSX unwind workaroundNov 26, 2016
@jkotas
jkotas merged commit 0d054ad into dotnet:masterNov 26, 2016
@christianscheuer
christianscheuer deleted the osx-unwind-workaround branch November 27, 2016 16:22
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@christianscheuer@dnfclas@jkotas@janvorli