Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[x86/Linux] Emit proper frame for ArrayOpStub #9688

Closed
wants to merge 2 commits into from

Conversation

parjong
Copy link

@parjong parjong commented Feb 21, 2017

In x86/Linux, C++ unwinder failed to unwind the frame by ArrayOpStub as the current implementation does not emit a proper frame (discussed in #9687).

This commit revises ArrayOpStub to have a ebp frame to fix #9687.

@parjong
Copy link
Author

parjong commented Feb 21, 2017

\CC @wateret @seanshpark

@parjong
Copy link
Author

parjong commented Feb 21, 2017

@dotnet-bot test Windows_NT x86 Checked Build and Test please

@parjong parjong changed the title [x86/Linux] Emit proper frame for ArrayOpStub [x86/Linux] Emit proper frame for ArrayOpStub (WIP) Feb 21, 2017
@parjong parjong changed the title [x86/Linux] Emit proper frame for ArrayOpStub (WIP) [x86/Linux] Emit proper frame for ArrayOpStub Feb 21, 2017
@janvorli
Copy link
Member

@parjong what was the problem in the unwind? Was it just unable to get past the frame of the asm helper (getting wrong EIP, ESP or both)? If that's the case, we should be able to fix it by adding CFI instructions to the asm helpers instead to teach the unwinder how to unwind from these.
I think that adding .cfi_def_cfa_offset 12 as the first line of the ArrayOpStubXXX functions would work without adding the ebp pushing / popping.

@parjong
Copy link
Author

parjong commented Feb 22, 2017

@janvorli An exception is throw inside ArrayStoreCheck called from generated stub (not one of asm helpers).

Here is the stack trace when hitting the issue:

(gdb) bt
#0  0xf7fd8be9 in __kernel_vsyscall ()
#1  0xf7c38ea9 in __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#2  0xf7c3a407 in __GI_abort () at abort.c:89
#3  0xf7ea7d35 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#4  0xf7ea5833 in ?? () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#5  0xf7ea58ad in std::terminate() () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#6  0xf7ea5b70 in __cxa_throw () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#7  0xf7137776 in DispatchManagedException (ex=..., isHardwareException=false)
    at /home/parjong/projects/dotnet/coreclr/src/vm/exceptionhandling.cpp:4667
#8  0xf6fadb8f in ArrayStoreCheck (pArray=0xffffc5ac, pElement=0xffffc5c0) at /home/parjong/projects/dotnet/coreclr/src/vm/jithelpers.cpp:3368
#9  0xf631b669 in ?? ()

@janvorli
Copy link
Member

@parjong oh, thanks for the explanation. But then the fixed worries me. The thing is that we unwind the stub that's generated as asm as native code using the libunwind. But that stub has no DWARF unwind info and it seems that the fact that libunwind can unwind through it is kind of luck (it probably has a fallback to use ebp if it doesn't find DWARF unwind info).
I would feel much better if we could fix it by switching x86 on Linux to FEATURE_ARRAYSTUB_AS_IL. @jkotas, do you think it would be feasible or are there some pitfalls?

@jkotas
Copy link
Member

jkotas commented Feb 22, 2017

switching x86 on Linux to FEATURE_ARRAYSTUB_AS_IL

I do not see any problems with it

@parjong
Copy link
Author

parjong commented Feb 22, 2017

@janvorli As you pointed out, it seems that libunwind runs in fallback mode (based on minimal ebp frame).

Non-volatile register fixup using helper frame (in 2nd pass) seems to mitigate the issues from incorrect register values (May this explain why this patch works), but I agree with you that it would be better to enable FEATURE_ARRAY_STUBS_AS_IL.

I'll take a look.

@parjong parjong closed this Feb 22, 2017
@parjong parjong deleted the fix/emit_ArrayOpStub_frame branch February 23, 2017 23:59
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants