druntime commit, revision 459


user: Don Clugston

msg:
Remove dependence on the C stdlib for exception handling. This is a necessary step towards fixing bug 1513.

http://www.dsource.org/projects/druntime/changeset/459

paths changed:
U   trunk/src/rt/deh.d

This one was discussed a while ago in private email and stalled for lack of an executive decision.  I'll paste some pertinent bits of the discussion below (all stuff I said).  The rest of this message is pasted from various old emails.

----------

On Sep 2, 2010, at 3:44 PM, Sean Kelly wrote:

> I spent some time investigating the test suite failures that caused Walter to revert the chaining code and ran into an interesting problem.  Consider this (from test4.d test55):
> 
> void funcA()
> {
>   try {
>       try {
>           throw new Exception( "A" );
>       } finally {
>           funcB();
>       }
>   } catch( Exception e ) {
>       assert( e.msg == "A" );
>   }
> }
> 
> void funcB()
> {
>   try {
>       throw new Exception( "B" );
>   } catch( Exception e ) {
>       assert( e.msg == "B" );
>   }
> }
> 
> In this case, the assertion in funcB() will fail because exception A is already in flight so exception B will be considered collateral damage and be chained to the existing exception A.  In an ideal world, proper behavior would probably be to make exception chaining scope-aware so an exception is only considered collateral if it escapes the top finally block called during stack unwinding.  Practically speaking however, this seems like it could be a very complicated thing to actually do.  What are the alternatives?  As it is, I don't feel comfortable rolling the chaining code back in.

Okay, I think this could be done without too much work, but it would require adding a pointer to the handler table for each stack frame (for the current exception).  I'm unsure whether the chaining behavior is worth it... thoughts?

On Sep 21, 2010, at 3:43 PM, Sean Kelly wrote:
> 
>> On 9/21/10 17:01 CDT, Sean Kelly wrote:
>> 
>>> What's really convinced me that the simple TDPL approach isn't sufficient is that it can violate the nothrow guarantee.  You'd think it should be safe to put a nothrow label on funcB(), right?  But with the previously implemented TDPL model, the exception "thrown" out of funcB() is exception A because exception B is considered collateral damage.  I think you're right that the language in TDPL isn't wrong though.  It's just that a naive implementation of the described TDPL model (as I did before) doesn't work.
> 
> I think doing so will require either a codegen change to add a void* to the DHandlerInfo struct (for saving a reference to the in-flight exception that caused the handler to be called), or a dynamic stack to store this info.

I realized last night that the dynamic stack can be a linked-list of structs on the stack, so no need for DMA.  So store the Throwable reference and a pointer to the finally block being called or perhaps a stack position.  I'll experiment with it today.  The details are still fuzzy but I'm reasonably certain it will work.

O by the way for full TDPL compliance some changes will be necessary for scope(failure) as well.

On Dec 28, 2010, at 10:34 AM, Sean Kelly wrote:

> This one was discussed a while ago in private email and stalled for lack of an executive decision.  I'll paste some pertinent bits of the discussion below (all stuff I said).  The rest of this message is pasted from various old emails.
> 
> ----------
> 
> On Sep 2, 2010, at 3:44 PM, Sean Kelly wrote:
> 
>> I spent some time investigating the test suite failures that caused Walter to revert the chaining code and ran into an interesting problem.  Consider this (from test4.d test55):
>> 
>> void funcA()
>> {
>>  try {
>>      try {
>>          throw new Exception( "A" );
>>      } finally {
>>          funcB();
>>      }
>>  } catch( Exception e ) {
>>      assert( e.msg == "A" );
>>  }
>> }
>> 
>> void funcB()
>> {
>>  try {
>>      throw new Exception( "B" );
>>  } catch( Exception e ) {
>>      assert( e.msg == "B" );
>>  }
>> }
>> 
>> In this case, the assertion in funcB() will fail because exception A is already in flight so exception B will be considered collateral damage and be chained to the existing exception A.  In an ideal world, proper behavior would probably be to make exception chaining scope-aware so an exception is only considered collateral if it escapes the top finally block called during stack unwinding.  Practically speaking however, this seems like it could be a very complicated thing to actually do.  What are the alternatives?  As it is, I don't feel comfortable rolling the chaining code back in.
> 
> Okay, I think this could be done without too much work, but it would require adding a pointer to the handler table for each stack frame (for the current exception).  I'm unsure whether the chaining behavior is worth it... thoughts?
> 
> On Sep 21, 2010, at 3:43 PM, Sean Kelly wrote:
>> 
>>> On 9/21/10 17:01 CDT, Sean Kelly wrote:
>>> 
>>>> What's really convinced me that the simple TDPL approach isn't sufficient is that it can violate the nothrow guarantee.  You'd think it should be safe to put a nothrow label on funcB(), right?  But with the previously implemented TDPL model, the exception "thrown" out of funcB() is exception A because exception B is considered collateral damage.  I think you're right that the language in TDPL isn't wrong though.  It's just that a naive implementation of the described TDPL model (as I did before) doesn't work.
>> 
>> I think doing so will require either a codegen change to add a void* to the DHandlerInfo struct (for saving a reference to the in-flight exception that caused the handler to be called), or a dynamic stack to store this info.
> 
> I realized last night that the dynamic stack can be a linked-list of structs on the stack, so no need for DMA.  So store the Throwable reference and a pointer to the finally block being called or perhaps a stack position.  I'll experiment with it today.  The details are still fuzzy but I'm reasonably certain it will work.
> _______________________________________________
> D-runtime mailing list
> D-runtime at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/d-runtime

On 28 December 2010 19:34, Sean Kelly <sean at invisibleduck.org> wrote:
> This one was discussed a while ago in private email and stalled for lack of an executive decision. ?I'll paste some pertinent bits of the discussion below (all stuff I said). ?The rest of this message is pasted from various old emails.

That's good to know.  So the Linux behaviour is also believed to be incorrect, but in a different way?

Still, none of this SEH stuff seems right to me. localunwind returns a
CollidedUnwind, but it doesn't seem to actually be doing a collided
unwind.  If it does a collided unwind, it should be changing the catch
clause which it's ultimately trying to reach, but continuing to
execute any finally clauses en route. If it does exception chaining,
it should continue to execute any finally clauses en route.
But it does neither of these things; instead, it just silently fails!
(This would seem to be wrong on D1, regardless of what it is supposed
to do on D2).

For something which is so complicated and undocumented by Microsoft,
the lack of comments in the code is terrible. I added a lot of
comments out of frustration. Please fix any which are wrong, and
change the function names where appropriate.
Regardless of when bug 1513 gets fixed, it seemed to me an
embarrassment that a C compiler was required for the D runtime.

>
> ----------
>
> On Sep 2, 2010, at 3:44 PM, Sean Kelly wrote:
>
>> I spent some time investigating the test suite failures that caused Walter to revert the chaining code and ran into an interesting problem. ?Consider this (from test4.d test55):
>>
>> void funcA()
>> {
>> ? try {
>> ? ? ? try {
>> ? ? ? ? ? throw new Exception( "A" );
>> ? ? ? } finally {
>> ? ? ? ? ? funcB();
>> ? ? ? }
>> ? } catch( Exception e ) {
>> ? ? ? assert( e.msg == "A" );
>> ? }
>> }
>>
>> void funcB()
>> {
>> ? try {
>> ? ? ? throw new Exception( "B" );
>> ? } catch( Exception e ) {
>> ? ? ? assert( e.msg == "B" );
>> ? }
>> }
>>
>> In this case, the assertion in funcB() will fail because exception A is already in flight so exception B will be considered collateral damage and be chained to the existing exception A. ?In an ideal world, proper behavior would probably be to make exception chaining scope-aware so an exception is only considered collateral if it escapes the top finally block called during stack unwinding. ?Practically speaking however, this seems like it could be a very complicated thing to actually do. ?What are the alternatives? ?As it is, I don't feel comfortable rolling the chaining code back in.
>
> Okay, I think this could be done without too much work, but it would require adding a pointer to the handler table for each stack frame (for the current exception). ?I'm unsure whether the chaining behavior is worth it... thoughts?
>
> On Sep 21, 2010, at 3:43 PM, Sean Kelly wrote:
>>
>>> On 9/21/10 17:01 CDT, Sean Kelly wrote:
>>>
>>>> What's really convinced me that the simple TDPL approach isn't sufficient is that it can violate the nothrow guarantee. ?You'd think it should be safe to put a nothrow label on funcB(), right? ?But with the previously implemented TDPL model, the exception "thrown" out of funcB() is exception A because exception B is considered collateral damage. ?I think you're right that the language in TDPL isn't wrong though. ?It's just that a naive implementation of the described TDPL model (as I did before) doesn't work.
>>
>> I think doing so will require either a codegen change to add a void* to the DHandlerInfo struct (for saving a reference to the in-flight exception that caused the handler to be called), or a dynamic stack to store this info.
>
> I realized last night that the dynamic stack can be a linked-list of structs on the stack, so no need for DMA. ?So store the Throwable reference and a pointer to the finally block being called or perhaps a stack position. ?I'll experiment with it today. ?The details are still fuzzy but I'm reasonably certain it will work.
> _______________________________________________
> D-runtime mailing list
> D-runtime at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/d-runtime
>

On Dec 28, 2010, at 12:17 PM, Don Clugston wrote:

> On 28 December 2010 19:34, Sean Kelly <sean at invisibleduck.org> wrote:
>> This one was discussed a while ago in private email and stalled for lack of an executive decision.  I'll paste some pertinent bits of the discussion below (all stuff I said).  The rest of this message is pasted from various old emails.
> 
> That's good to know.  So the Linux behaviour is also believed to be incorrect, but in a different way?

Yes.  Basically, having an exception thrown when another is in flight can't replace the in flight exception or chain with it, or things break in different, subtle ways (old D behavior was replacement, new D behavior was to be chaining).

> Still, none of this SEH stuff seems right to me. localunwind returns a
> CollidedUnwind, but it doesn't seem to actually be doing a collided
> unwind.  If it does a collided unwind, it should be changing the catch
> clause which it's ultimately trying to reach, but continuing to
> execute any finally clauses en route. If it does exception chaining,
> it should continue to execute any finally clauses en route.
> But it does neither of these things; instead, it just silently fails!
> (This would seem to be wrong on D1, regardless of what it is supposed
> to do on D2).

You are already better versed with SEH than I am.  I too had trouble finding documentation of SEH and so didn't dig into that code too deeply, and so assumed it was correct and tweaked it to add the chaining behavior.  It sounds like it all needs a closer look.  Thanks for getting the ball rolling!
> 
> For something which is so complicated and undocumented by Microsoft,
> the lack of comments in the code is terrible. I added a lot of
> comments out of frustration. Please fix any which are wrong, and
> change the function names where appropriate.
> Regardless of when bug 1513 gets fixed, it seemed to me an
> embarrassment that a C compiler was required for the D runtime.

You know how these things are... the code (seemingly) worked and so was left alone.  I'm glad you spent the time to convert it though.  I'll have to take care of the mutex stuff soonish as well.  I'll review your changes as well.  Possibly not until next week though.  I'm off this week and that tends to mean that I don't get any D coding done.  Ironic, that.

On 29 December 2010 00:08, Sean Kelly <sean at invisibleduck.org> wrote:
> On Dec 28, 2010, at 12:17 PM, Don Clugston wrote:
>
>> On 28 December 2010 19:34, Sean Kelly <sean at invisibleduck.org> wrote:
>>> This one was discussed a while ago in private email and stalled for lack of an executive decision. ?I'll paste some pertinent bits of the discussion below (all stuff I said). ?The rest of this message is pasted from various old emails.
>>
>> That's good to know. ?So the Linux behaviour is also believed to be incorrect, but in a different way?
>
> Yes. ?Basically, having an exception thrown when another is in flight can't replace the in flight exception or chain with it, or things break in different, subtle ways (old D behavior was replacement, new D behavior was to be chaining).

Now that I've fixed the Windows SEH implementation so that finally
clauses are always called, the TDPL behaviour can be implemented quite
easily.
All that needs to happen is to move the chaining from the catch
handler, into the collision handler.
So, chaining occurs only when two uncaught exceptions have reached the
same finally or catch clause. (It's OK to have two in flight at the
same time, as long as they don't collide). No changes to the compiler
are required, and there's no need to track where the exception is
thrown. I'm actually quite impressed with SEH, it's a very clever
design. I never had any desire to understand Windows SEH, but it's Too
Late Now.

There are only a couple of problems:
*  _d_setUnhandled() isn't much use, because we need the Windows
EXCEPTION_RECORD structure for the exception in flight, not the D
exception.
The semantics of that function need to change anyway, I think, if it's
a user-accessable function.
*  I have no idea how exception handling works on Linux/OSX. So I
don't know how hard the TDPL behaviour will be to implement for them,
and what it will look like. But having a working Windows
implementation might help you.

Should I implement the TDPL behaviour for Windows, and check it in?

-Don.

That would be great!

Sent from my iPhone

On Jan 7, 2011, at 11:29 PM, Don Clugston <dclugston at googlemail.com> wrote:

> On 29 December 2010 00:08, Sean Kelly <sean at invisibleduck.org> wrote:
>> On Dec 28, 2010, at 12:17 PM, Don Clugston wrote:
>> 
>>> On 28 December 2010 19:34, Sean Kelly <sean at invisibleduck.org> wrote:
>>>> This one was discussed a while ago in private email and stalled for lack of an executive decision.  I'll paste some pertinent bits of the discussion below (all stuff I said).  The rest of this message is pasted from various old emails.
>>> 
>>> That's good to know.  So the Linux behaviour is also believed to be incorrect, but in a different way?
>> 
>> Yes.  Basically, having an exception thrown when another is in flight can't replace the in flight exception or chain with it, or things break in different, subtle ways (old D behavior was replacement, new D behavior was to be chaining).
> 
> Now that I've fixed the Windows SEH implementation so that finally
> clauses are always called, the TDPL behaviour can be implemented quite
> easily.
> All that needs to happen is to move the chaining from the catch
> handler, into the collision handler.
> So, chaining occurs only when two uncaught exceptions have reached the
> same finally or catch clause. (It's OK to have two in flight at the
> same time, as long as they don't collide). No changes to the compiler
> are required, and there's no need to track where the exception is
> thrown. I'm actually quite impressed with SEH, it's a very clever
> design. I never had any desire to understand Windows SEH, but it's Too
> Late Now.
> 
> There are only a couple of problems:
> *  _d_setUnhandled() isn't much use, because we need the Windows
> EXCEPTION_RECORD structure for the exception in flight, not the D
> exception.
> The semantics of that function need to change anyway, I think, if it's
> a user-accessable function.
> *  I have no idea how exception handling works on Linux/OSX. So I
> don't know how hard the TDPL behaviour will be to implement for them,
> and what it will look like. But having a working Windows
> implementation might help you.
> 
> Should I implement the TDPL behaviour for Windows, and check it in?
> 
> -Don.
> _______________________________________________
> D-runtime mailing list
> D-runtime at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/d-runtime