Dwarf EH: enable it #5324
Dwarf EH: enable it #5324
Conversation
WalterBright
force-pushed
the
enable-dwarfeh
branch
2 times, most recently
from
f7569d0 to
7471d47
Compare
|
Needs dlang/druntime#1454 |
|
I am having trouble reproducing this (though obviously the autotester doesn't). I am having a hard time figuring out which module is failing the unit test - anyone have a clue? |
|
Finally tracked it down to etc.linux.memoryerror |
We still use that module? |
WalterBright
force-pushed
the
enable-dwarfeh
branch
from
7471d47 to
2442010
Compare
|
I tracked it down to the use of How does gdc handle this? |
WalterBright
force-pushed
the
enable-dwarfeh
branch
from
2442010 to
8ae7ce4
Compare
|
Sigh, I see how gcc handles the eh_frame data for inline assembler: |
|
It's working. Woo-hoo! |
| dwarf_CFA_set_loc(I64 ? 4 : 3); | ||
| dwarf_CFA_set_reg_offset(BP, off); | ||
| } | ||
|
|
There was a problem hiding this comment.
Don't think EH can be supported for naked functions, please don't introduce broken hacks just to make our testsuite work.
c++ - Using Try/Catch with naked functions - Stack Overflow
The main point of naked functions is to not setup a stack frame, so assuming a normal stack frame is pointless.
There was a problem hiding this comment.
The existing stack unwinding mechanism already assumes a normal stack frame. This does not make things worse, it brings it up to the current state by emitting eh_frame info that says it's a normal stack frame.
We can do better by extending the inline assembler to support CFA pseudo-ops, but that would break existing code, and is a non-trivial project itself.
The stack frame info is necessary only if the inline assembler does not call any functions that throw. The etc.linux.memoryerror does do this. I considered removing memoryerror, as it converts seg faults into thrown exceptions, something that doesn't belong in a systems programming language, but decided to simply make it work for now and leave that issue for a separate day.
There was a problem hiding this comment.
The existing stack unwinding mechanism already assumes a normal stack frame.
OK, fine.
The stack frame info is necessary only if the inline assembler does not call any functions that throw. The etc.linux.memoryerror does do this. I considered removing memoryerror, as it converts seg faults into thrown exceptions, something that doesn't belong in a systems programming language, but decided to simply make it work for now and leave that issue for a separate day.
I tried hard to prevent memoryerror from happening but couldn't achieve more than moving it to etc. In any case people are now relying on it to get tracebacks, e.g. for server apps, so w/o an alternative it would be very rude to remove it.
|
Auto-merge toggled on |
| @@ -110,7 +112,6 @@ void out_config_init( | |||
| config.flags |= CFGalwaysframe; // PIC needs a frame for TLS fixups | |||
| } | |||
| config.objfmt = OBJ_ELF; | |||
| config.ehmethod = dwarfeh ? EH_DWARF : EH_DM; | |||
There was a problem hiding this comment.
I hope this means we can easily disable the feature if it doesn't survive the beta testing.
There was a problem hiding this comment.
That's exactly what it means, and one reason I did it that way. Another reason is it makes it much easier to test, because you cannot build druntime with a non-functional EH system.
|
Nice! Shouldn't the command line option be removed, too? |
Is there a switch to toggle back to the old EH handling? That would be helpful during the beta and should stay in for a little while. |
|
@MartinNowak: I didn't closely follow the backend implementation, but it seems as if this commit hard-codes the distinction again. But I doubt it would be very useful anyway. You'd need to rebuild druntime/Phobos to use the correct EH method anyway every time you switch. At that point, you can keep two DMD executables around just as well. |
|
Apparently it did broke a test case that links against C++. |
|
@MartinNowak: That run is for your FreeBSD-related change, not this PR. |
|
As @JackStouffer pointed out, according to @CyberShadow's http://digger.k3.1azy.net/trend/ this caused a 85KiB increase in the hello world binary!? |
|
Yeah, well, the dwarf exception handling tables are rather large. Anyhow, commenting on a PR that was closed several months ago is likely to be overlooked. It would be better to start a thread in the n.g. on it, or open a new bug report. |
Thanks for the advice - I created a bug for reference: |
|
Seems this PR caused a bug (or so it is claimed): https://issues.dlang.org/show_bug.cgi?id=15625 |
|
... and another one: https://issues.dlang.org/show_bug.cgi?id=16699 |
No description provided.