497669 - Standard MSVC longjump cannot be used with Tamarin if Tamarin is compiled with C++ exception support

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P1)

Description

[Lars T Hansen]

The problem is that MSVC longjmp unwinds the stack in the same way as exception handling would, if exception handling is turned on.  This is probably legal but really breaks our locking code: MMGC_LOCK turns into a local variable declaration that acquires a spinlock on entry and releases it when the object is destroyed.  Code that longjmps past such a lock (only the out-of-memory abort at this time) is careful to perform an explicit unlock of the GCHeap lock before jumping - but because the destructor of the locking object is called by longjmp, we get a double release, and we have asserts to catch that.

If we have other acquire/release patterns then they too could be affected.

The fix is to use a setjmp/longjmp implementation that does not do this.

We need to provide such an implementation and we need to implement (well documented) checks for this problem occuring.  It's not clear to me right now what those checks might look like.

Comment 1

[Lars T Hansen]

One correspondent writes:

"You might look into turning off structured exception handling.  That might cause setjmp/longjmp to not call the c++ destructors when unwinding.  Try /EHsc.
 
Link to /EH docs:
http://msdn.microsoft.com/en-us/library/1deeycx5.aspx"

Comment 2

[Lars T Hansen]

This appears to be a problem even if Tamarin is compiled with C++ exceptions disabled, at least on 64-bit Vista.

The situation in the code is pretty messy: we call naked setjmp/longjmp here and there, never VMPI_ versions; the jit appears to call our own _setjmp3 under some circumstances.  None of this seems reliable.

Comment 3

[Lars T Hansen]

(Taking this because it's blocking another P1 bug I have.)

Comment 4

[Lars T Hansen]

Attachment: Preliminary patch, MSVC only

Introduces VMPI_setjmpNoUnwind and VMPI_longjmpNoUnwind and maps them to the correct non-unwinding versions on 64-bit MSVC builds; maps them to setjmp and longjmp on 32-bit MSVC builds because that works correctly.  Implementations for other platforms to follow.

Comment 5

[Lars T Hansen]

Attachment: Patch for all platforms

Tested on Windows and Mac; testing continues.  Please review carefully for impacts to JIT.

Comment 6

[Edwin Smith]

Comment on attachment 408448 [details] [diff] [review]
Patch for all platforms

jit never calls longjmp directly.

confirmed that we're properly taking &::setjmp on mac-32 and mac-64.  if win32 and winmo pass regression testing then i think we're in the clear.

Comment 7

[Lars T Hansen]

redux changeset:   2881:b80e5c6070d6