Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/core/runtime.d
Original file line number Diff line number Diff line change
Expand Up @@ -760,11 +760,17 @@ Throwable.TraceInfo defaultTraceHandler( void* ptr = null )
{
version (Win64)
{
static enum FIRSTFRAME = 4;
version (LDC)
static enum FIRSTFRAME = 1;
else
static enum FIRSTFRAME = 4;
}
else version (Win32)
{
static enum FIRSTFRAME = 0;
version (LDC)
static enum FIRSTFRAME = 1;
else
static enum FIRSTFRAME = 0;
}
import core.sys.windows.windows : CONTEXT;
auto s = new StackTrace(FIRSTFRAME, cast(CONTEXT*)ptr);
Expand Down
40 changes: 23 additions & 17 deletions src/ldc/eh/msvc.d
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ struct CxxExceptionInfo
version(Win64) void* ImgBase;
}

extern(C) Throwable.TraceInfo _d_traceContext(void* ptr = null);

extern(C) void _d_throw_exception(Object e)
{
if (e is null)
Expand All @@ -109,12 +111,16 @@ extern(C) void _d_throw_exception(Object e)
if (!old_terminate_handler)
old_terminate_handler = set_terminate(&msvc_eh_terminate);
}
auto t = cast(Throwable) e;
exceptionStack.push(t);

auto throwable = cast(Throwable) e;
exceptionStack.push(throwable);

if (throwable.info is null && cast(byte*)throwable !is typeid(throwable).init.ptr)
throwable.info = _d_traceContext();
Copy link

Choose a reason for hiding this comment

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

Even though this is the behaviour used on other platfroms or in dmd, I don't think it should be encouraged: it can make exceptions unusable slow, even if only used in rare cases. The trace should only be created with the catch statement that actually wants to print it. That's quite a bit more difficult to implement, though...

Copy link
Member

Choose a reason for hiding this comment

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

Interesting info, thanks. I'd rather have Windows streamlined with the other platforms though (and create an enhancement issue wrt. performance and your idea how to improve on it), don't you agree?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe need a option to let user choose not to generate the stack traceback, like this:
throw Exception("", stackTrace=false)

Copy link
Member

@kinke kinke Jan 13, 2017

Choose a reason for hiding this comment

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

When constructing the exception, the user generally doesn't have any idea about where it's going to unwind to and whether there are catch handlers interested in the trace. throw -> _d_throw_exception() could check for registered handlers at runtime though, but it would also require compiler support to register the handlers while in a try scope.

Copy link

Choose a reason for hiding this comment

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

don't you agree?

I'm ok with streamlining for now. Disabling the stack trace generation could be done through a -DRT-option for exception heavy applications.

Copy link

Choose a reason for hiding this comment

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

your idea how to improve on it

I lost a branch of druntime in a SSD crash that had that for dmd's Win32 exception handling: The type of the d_run_main catch handler had a specific type that was detected during the search phase of the exception unwinding when the stack of the throw was still available. I guess this is not so easily integrated into the MS C++ runtime without being able to modify it...

Copy link
Member

@kinke kinke Jan 13, 2017

Choose a reason for hiding this comment

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

I lost a branch of druntime in a SSD crash

Sorry to hear that :/ - you got me slightly worried about my SSD and my reluctance to backups now. ;)

I'm ok with streamlining for now.

Perfect.

Disabling the stack trace generation could be done through a -DRT-option for exception heavy applications.

You mean as CMake option to be forwarded as -d-version=... when compiling your own speedy-Gonzalez druntime? Or more fine-grained by extending _d_throw_exception() by a compile-time bool depending on LDC switch, to allow compiling specific modules with no stack traces attached to thrown exceptions?
Edit: In that case, we could also introduce a pragma/UDA controlling the stack trace generation.

Copy link
Member

Choose a reason for hiding this comment

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

--DRT- refers to process command line arguments implicitly picked up by druntime.

Copy link
Member

@kinke kinke Jan 13, 2017

Choose a reason for hiding this comment

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

Oh I see, thanks, never heard of this before. Disabling it per process sounds very reasonable.

Copy link

Choose a reason for hiding this comment

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

--DRT options can also be embedded in the executable or picked up from the environment. See https://dlang.org/spec/garbage.html#gc_config


CxxExceptionInfo info;
info.Magic = EH_MAGIC_NUMBER1;
info.pThrowable = &t;
info.pThrowable = &throwable;
info.ThrowInfo = getThrowInfo(ti).toPointer;
version(Win64) info.ImgBase = ehHeap.base;

Expand Down Expand Up @@ -377,14 +383,14 @@ void msvc_eh_terminate() nothrow
mov RBX, 0xFFFFFF;
and RDX, RBX;
cmp RDX, 0xC48348; // add ESP,nn (debug UCRT libs)
je L_addESP_found;
je L_addESP_found;
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR, I'll test it over the weekend. All changes starting with this one seem to be different line endings only. Could you please fix that (for the whole file)?

Copy link
Author

Choose a reason for hiding this comment

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

I just edit the file on the github (using the chrome browser), I really do not know why the line endings changed?

Copy link

Choose a reason for hiding this comment

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

The mixture of line endings is probably my fault. Even with the editorconfig plugin VS seems to produce these quite often. I suppose the chrome editor has converted everything to unix-style (which is ok).

cmp DL, 0x90; // nop; (release libs)
jne L_term;

L_release_ucrt:
mov RDX,[RSP+8];
cmp word ptr[RDX-2], 0xD3FF; // call ebx?
sete BL; // if not, it's UCRT 10.0.14393.0
jne L_term;
L_release_ucrt:
mov RDX,[RSP+8];
cmp word ptr[RDX-2], 0xD3FF; // call ebx?
sete BL; // if not, it's UCRT 10.0.14393.0
movzx RBX,BL;
mov RDX, 0x28; // release build of vcruntimelib
jmp L_retTerminate;
Expand All @@ -394,13 +400,13 @@ void msvc_eh_terminate() nothrow

cmp byte ptr [RAX+4], 0xC3; // ret?
jne L_term;

L_retTerminate:
lea RDX,[RSP+RDX+0x10]; // RSP before returning from terminate()

mov RAX,[RDX]; // return address inside __FrameUnwindHandler

or RDX,RBX; // RDX aligned, save RBX == 0 for UCRT 10.0.14393.0, 1 otherwise
or RDX,RBX; // RDX aligned, save RBX == 0 for UCRT 10.0.14393.0, 1 otherwise

cmp byte ptr [RAX-19], 0xEB; // skip back to default jump inside "switch" (libvcruntimed.lib)
je L_switchFound;
Expand All @@ -424,7 +430,7 @@ void msvc_eh_terminate() nothrow

L_switchFound2:
dec RAX;
L_switchFound:
L_switchFound:
movsx RBX, byte ptr [RAX-18]; // follow jump
lea RAX, [RAX+RBX-17];

Expand All @@ -433,12 +439,12 @@ void msvc_eh_terminate() nothrow

add RAX,2;
L_xorSkipped:
mov RBX, RDX; // extract UCRT marker from EDX
and RDX, ~1;
and RBX, 1;
mov RBX, RDX; // extract UCRT marker from EDX
and RDX, ~1;
and RBX, 1;

cmovnz RBX,[RDX-8]; // restore RBX (pushed inside terminate())
cmovz RBX,[RSP]; // RBX not changed in terminate inside UCRT 10.0.14393.0
cmovz RBX,[RSP]; // RBX not changed in terminate inside UCRT 10.0.14393.0

lea RSP,[RDX+8];
push RAX; // new return after setting return value in __frameUnwindHandler
Expand Down