This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Windows thread naming suggestion. Don't apply. (sdl2_win_thread_naming_suggestion-do_not_apply.diff, text/plain, 2013-09-11 16:48:58 +0000, 1996 bytes)
• Removes the inline asm and adds vectored exception handling and the __try/__except extensions. (sdl2_thread_names_exception_handling.diff, text/plain, 2013-10-02 09:20:19 +0000, 3636 bytes)

Reported in version: HG 2.0
Reported for operating system, platform: Windows 7, x86_64

Comments on the original bug report:

On 2013-09-06 14:40:18 +0000, Leander wrote:

After this commit: http://hg.libsdl.org/SDL/rev/54c06b0f65d9

SDL\src\thread\windows\SDL_systhread.c, line 184:
error C4235: nonstandard extension used : '__asm' keyword not supported on this architecture.

On 2013-09-07 18:15:42 +0000, Ryan C. Gordon wrote:

I disabled this code on Win64 for now, so we're back where we were before that commit. We'll come up with a better solution, so I'll leave this bug open for now.

--ryan.

On 2013-09-11 14:14:46 +0000, Leander wrote:

More bad news: rev 7735 causes an exception anyway in the 32-bit DLL.

Reverting to rev 7498 fixes it.

The exception happens when calling SDL_Init( SDL_INIT_TIMER ).

Note that to see the exception of the 32-bit build of my program I have to run it from the command-line, not from the VS2012EE debugger (or run it from VS with Ctrl-F5). Probably the way exceptions are handled is different with the debugger attached.

Not having the VS debugger to understand where the problem lied, I used gdb under cygwin:

[New Thread 5240.0xb8c]
[New Thread 5240.0xf3c]
gdb: unknown target exception 0x406d1388 at 0x7557c41f
Program received signal ?, Unknown signal.
[Switching to Thread 5240.0xf3c]
0x7557c41f in RaiseException () from /cygdrive/c/Windows/syswow64/KERNELBASE.dll
(gdb) bt

0 0x7557c41f in RaiseException () from /cygdrive/c/Windows/syswow64/KERNELBASE.dll

1 0x51a44560 in SDL_SemWaitTimeout () from SDL2.dll

2 0x51a44c1f in SDL_GetThreadName () from SDL2.dll

3 0x51a44409 in SDL_SemWaitTimeout () from SDL2.dll

4 0x51a4442b in SDL_SemWaitTimeout () from SDL2.dll

5 0x7585336a in KERNEL32!BaseThreadInitThunk () from kernel32.dll

6 0x77ab9f72 in ntdll!RtlInitializeExceptionChain () from ntdll.dll

7 0x77ab9f45 in ntdll!RtlInitializeExceptionChain () from ntdll.dll

8 0x00000000 in ?? ()

indeed it's the SDL_GetThreadName call...

On 2013-09-11 15:03:31 +0000, Leander wrote:

Actually ignore the SDL_GetThreadName line.

Anyway it appears an unhandled 0x406d1388 exception is causing my C++ program to terminate when run from the command line with the new rev, whether the old rev works.

On 2013-09-11 16:48:58 +0000, wrote:

Created attachment 1328
Windows thread naming suggestion. Don't apply.

On 2013-09-11 16:49:11 +0000, wrote:

The RaiseException also terminates the application here. When the debugger handles the exception, it works fine.

For some reason, the SDL handler doesn't get called. The asm seems to push the address correctly into the TIB, but I don't get why Windows doesn't use it. Is it incomplete or is something else overriding it?

Perhaps it could check with IsDebuggerPresent if a debugger is attached and only then fire the exception? Though I don't know how reliable that is.

I was about to suggest using SetUnhandledExceptionFilter to wrap that RaiseException. The upside is that it is clean C and works with x86-64, but that function has its own painful history. It only works if no debugger is attached. It is a global setting and other libraries or the application itself might battle over who may set its handler. Not really a problem since this only involves one tiny part and the previous handler will reset immediately, however, some people actively patch the code in memory at runtime to prevent the function from working.

On 2013-09-11 18:57:27 +0000, wrote:

Never mind that SetUnhandledExceptionFilter idea. It's exceptionally thread-unsafe. ;)

On 2013-09-16 05:20:59 +0000, wrote:

Oh, I missed that safe SEH is enabled by default. Looks like there's only one clean way to add the handler to the safe handler list and that's with some assembly. That's a bit disappointing.

Removing the static keyword from the handler function and throwing this at the Microsoft assembler makes it work properly if linked with safe SEH on x86.

.386
.model flat
_ignore_exception proto
.safeseh _ignore_exception
end

On 2013-10-02 09:20:19 +0000, wrote:

Created attachment 1352
Removes the inline asm and adds vectored exception handling and the __try/__except extensions.

I could not get x64 exception handling to work with assembly. The handler just doesn't get called for the RaiseException exception.

I looked at vectored exception handling instead which isn't really pretty since I have to add the handler at run-time and it will be called for all exception, giving an overhead on all of them. Again, the upside is that it works with x64 and MinGW.

I also added the __try/__except extensions when building with MSVC and HAVE_LIBC.

On 2013-10-04 07:22:01 +0000, Mitchell Keith Bloch wrote:

As pointed out several weeks ago, this causes crashing on 32-bit Windows. Is there any reason for this code to be enabled when compiling for a Release target? Why not just wrap the RaiseException in an #ifndef NDEBUG statement?:

diff -r 3023b0270c94 src/thread/windows/SDL_systhread.c
--- a/src/thread/windows/SDL_systhread.c Thu Oct 03 21:41:09 2013 -0700
+++ b/src/thread/windows/SDL_systhread.c Fri Oct 04 03:20:53 2013 -0400
@@ -187,8 +187,10 @@
mov fs:[0],esp
}

+#ifndef NDEBUG
/* The program itself should ignore this bogus exception. /
RaiseException(0x406D1388, 0, sizeof(inf)/sizeof(DWORD), (DWORD)&inf);
+#endif

     __asm {  /* tear down SEH. */
         mov eax,[esp]

On 2013-10-21 03:09:57 +0000, Ryan C. Gordon wrote:

I've disabled this code again in http://hg.libsdl.org/SDL/rev/6d79888998f5 ... so we're back where we were for SDL 2.0.0.

Leaving this bug open until I fix this better.

--ryan.

On 2016-02-21 22:24:54 +0000, Ryan C. Gordon wrote:

This should be fixed in a way that makes everyone happy (debugger or not, win32 or win64, built with any compiler)...

https://hg.libsdl.org/SDL/rev/96f7be2c885b

Let me know if this is still broken for you, though!

--ryan.

On 2016-10-08 01:57:03 +0000, Ryan C. Gordon wrote:

Apparently GPU debuggers, the .NET debugger, etc, will make IsDebuggerAvailable() report true, but then will panic when it sees the exception raised.

So...reopening the bug...again...

--ryan.

On 2018-08-07 15:51:47 +0000, Ryan C. Gordon wrote:

This was fixed in https://hg.libsdl.org/SDL/rev/d2843a5e3d3a and https://hg.libsdl.org/SDL/rev/abf45a095845 ... it'll use the official Windows 10 API for naming threads if available, and only set the name with the magic Visual Studio debugger way if a hint explicitly allows it.

So now this is safe everywhere, and as tools and Windows installs get upgraded, eventually this works just about everywhere, too.

--ryan.