Tk Source Code

Building tk with a recent gcc version (>= 15) on mingw64 with -fstack-protector-strong -O2 causes a crash on exit. For reproducing, just execute wish and exit.

The problem is the assembler code in win/tkWin32Dll.c DllMain.

The call to TkFinalize via assembler causes all non-caller-saved registers to be potentially clobbered, but the clobber list of the __asm__ block does not mention them all (currently they are quite a few and the list grows as new registers are introduced by extensions such as AVX*, etc.)

When compiling with the options mentioned above, the compiler introduces extra code for checking against stack smashing and that uses some of the registers clobbered by the assembler call to TkFinalize.

Please note that any other change to DllMain (in source code or build options) might introduce this problem as well.

We could add all non-caller-saved registers to the list, but the most robust, future-proof approach is to not call TkFinalize from assembler, letting the compiler figure out register handling. The patch below splits the assembler code in two chunks surrounding the C call to TkFinalize. It also adjusts the list of clobbered registers to reflect those that are actually modified by the corresponding assembler block.

diff --git a/win/tkWin32Dll.c b/win/tkWin32Dll.c
index 5161333da..fd15e5206 100644
--- a/win/tkWin32Dll.c
+++ b/win/tkWin32Dll.c
@@ -148,13 +148,21 @@ DllMain(
 
 	    "movq	%%rdx,		%%gs:0"		"\n\t"
 
-	    /*
-	     * Call TkFinalize
-	     */
+	    :
+	    /* No outputs */
+	    :
+	    [registration]	"m"	(registration),
+	    [error]		"i"	(TCL_ERROR)
+	    :
+	    "%rax", "%rdx", "memory"
+	);
 
-	    "movq	$0x0,		0x0(%%rsp)"		"\n\t"
-	    "call	TkFinalize"			"\n\t"
+        /* Just do a regular C call so we don't need to worry about following
+         * the calling convention, specially the registers the function may
+         * clobber: */
+        TkFinalize(NULL);
 
+	__asm__ __volatile__ (
 	    /*
 	     * Come here on a normal exit. Recover the TCLEXCEPTION_REGISTRATION
 	     * and store a TCL_OK status
@@ -188,11 +196,9 @@ DllMain(
 	    :
 	    /* No outputs */
 	    :
-	    [registration]	"m"	(registration),
-	    [ok]		"i"	(TCL_OK),
-	    [error]		"i"	(TCL_ERROR)
+	    [ok]		"i"	(TCL_OK)
 	    :
-	    "%rax", "%rbx", "%rcx", "%rdx", "%rsi", "%rdi", "memory"
+	    "%rax", "%rdx", "%rbp", "memory"
 	);
 
 #   else
@@ -218,12 +224,18 @@ DllMain(
 
 	    "movl	%%edx,		%%fs:0"		"\n\t"
 
-	    /*
-	     * Call TkFinalize
-	     */
+	    :
+	    /* No outputs */
+	    :
+	    [registration]	"m"	(registration),
+	    [error]		"i"	(TCL_ERROR)
+	    :
+	    "%eax", "%ebx", "%edx", "memory"
+	);
+
+        TkFinalize(NULL);
 
-	    "movl	$0x0,		0x0(%%esp)"		"\n\t"
-	    "call	_TkFinalize"			"\n\t"
+	__asm__ __volatile__ (
 
 	    /*
 	     * Come here on a normal exit. Recover the TCLEXCEPTION_REGISTRATION
@@ -259,11 +271,9 @@ DllMain(
 	    :
 	    /* No outputs */
 	    :
-	    [registration]	"m"	(registration),
-	    [ok]		"i"	(TCL_OK),
-	    [error]		"i"	(TCL_ERROR)
+	    [ok]		"i"	(TCL_OK)
 	    :
-	    "%eax", "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"
+	    "%eax", "%ebx", "%edx", "%ebp", "memory"
 	);
 
 #   endif

> What about back-porting to 9.0 and 8.6 ?

Is already done (the initial part only, not the removal of all SEH code)

Great ! What about back-porting to 9.0 and 8.6 ? This sounds quite serious (crash)? THanks, Harald

Closing, because the SEH code removal is in Tcl/Tk 9.1 now.

Please re-open if any new unexpected problem shows up. I'm quit confident that won't happen.

> Those socket tests, do they load Tk? Is an exception thrown from TkFinalize? If yes, that would be very interesting by itself, see the discussion here.

Those socket failures are in Tcl only (without Tk), and they were there already before we made the SEH changes. So, this ticket - for sure - didn't cause it.

@dkf:

Please note that this ticket was reporting an "always crashing" scenario. See the original report.

The current asm split in two block was for fixing that crash. The only thing that can be objected wrt the previous code is that the first block references a label defined on the second block. I tested that the generated address is correct, but that was just one configuration.

For solving the label cross-reference we could use "asm goto". However, that doesn't make the general situation any better. The asm code does terrible things, like assigning the stack registers. Oh, and the call to TkFinalize on the previous version, for amd64, was writing the stack (not "push %0x0", but "movq $0x0, 0x0(%%rsp)" !) instead of passing the parameter on a register, as the ABI requires.

In general, you can't expect to balance the stack on the exception code path because you we don't know certain things that only the compiler knows.

Those socket tests, do they load Tk? Is an exception thrown from TkFinalize? If yes, that would be very interesting by itself, see the discussion here.

Finally, the SEH hacks in Tcl are superfluous, as the WIN API functions they wrap do not throw exceptions (unless, for one of then, when the program is running under a debugger, but then you don't want to hide those exceptions.)

No objections, just me being annoyingly pedantic:

> This is high-risk: A long-lasting existing clean-up bug in user code could lead to an exception withthis change, which worked fine before.

Please note that Windows API functions don't throw exceptions (except a few of them when running in a debugger.)  Other sources of exceptions, such as divide-by-zero, are unlikely on this scenario. So the most probable cause for an exception in TkFinalize is an access violation (a.k.a. SIGSEGV) and that is anything but "working fine". As mentioned elsewhere, if those problems exist, we want to fix them.

Please CC me if you see reports related to this. Thank you.

> My POV is that an exception in TkFinalize would be a bug on Tk and we should fix it

Not necessary: TkFinalize() also runs exit handlers, which could do anything. There could be a bug there causing an exception.

Still I agree with you: We are not responsible for user-inserted bugs, so - still - in that case we we would want to see the exception. This is high-risk: A long-lasting existing clean-up bug in user code could lead to an exception withthis change, which worked fine before. Therefore I intend to backport the partial fix to 9.0, and fully remove all SEH handlers in Tcl/Tk 9.1a1: then, if an issue is found during beta-testing (which I don't expect) we still can (partially) revert the change.

Any objections?

Proposed [df7d241d4372112b|change] is in effect on trunk now. It looks like it works fine.

> The reason for this code is unknown.

What transpires from the bug report and the commit logs is that sometimes an exception was thrown from TkFinalize and, instead of investigating its cause and fixing it, the SEH handling code was added to hide the exception.

Maybe fixing the source of the exception was hard and the consequences of a half-done TkFinalize were considered as mild.

My POV is that an exception in TkFinalize would be a bug on Tk and we should fix it. Sadly, now we are on a scenario were there is a suspicion of something being wrong in TkFinalize but not observing the problem is no proof of its nonexistence.

I also think that SEH was used without fully understanding its implications. See the occurrences in Tcl, where they make no sense in two cases and very little sense on the third, as I mentioned on a previous comment. All those can be removed with confidence.

> Although I'm afraid that that combination is not very frequently used

Well, let's clarify. It's not a combination, it's either one. So it involves Windows on _all_ ARM64 platforms, and also on all other platforms (x86 and x86_64) in debug mode.

As Harald pointed out, we want to remove it fully in Tcl 9.1a1. That way, if any problems show up, we can modify it again in Tcl 9.1a2, depending on the lessons learned.

So, thanks for your suggestion! You are being listened at ;-)

We discussed it in the biweekly telco.

The reason for this code is unknown.
To find out, if full removal of SEH catch is ok, it was proposed to remove it and test in the alpha phase of Tk9.1.
It may be re-integrated if there are any issues in the field.

Harald

> A discussion about going even further is 

I think you left the phrase incomplete.

By the way, if there are reports about crashes on arm/windows/debug causud by the call to TkFinalize, I'll appreciate if you ping me. Although I'm afraid that that combination is not very frequently used.

Thanks!

The follow-up proposal is here: https://core.tcl-lang.org/tk/timeline?r=bug-44b34c6152 It is just merged to trunk

At least in debug mode and on arm64 we won't use the SEH handler any more. A discussion about going even further is

> Any objections to the revised patch posted on 2025-11-12 10:40:35 ?

No objections, therefore it was released with 9.0.3 already ;-)

Any objections to the revised patch posted on 2025-11-12 10:40:35 ? The only difference is that it removes rbp/ebp from the "clobbers" list.

I vill patch my development build to call TkFinalize without the SEH hack. Even causing SEH exceptions from within TkFinalize. To see what happens.

In the meantime, can you apply the revised patch that I posted a few hours ago? Let's try at least to not crash the application in the normal execution path :-)

So, make steps on getting rid of it, in small steps.

The first step could be, call TkFinalize(NULL) on arm64 and in debug mode on x86. Let's see how far this brings us. At least programming errors in TkFinalize could be detected then.

> If an exception is thrown _inside_ TkFinalize, we don't want to stop it from doing further steps to unload the dll. The worst that could happen is a resource leak.

The worst that could happen is corrupting memory owned by the embedding application (which includes the stack.)

I embed Tcl/Tk in my applications. Some of them do delicate stuff. If TkFinalize crashes, for whatever reason, I *absolutely* want to know. My applications implement exception handling for bailing out on a specific way when an exception happens, differentiating by type of exception. This hack works against me.

Currently TkFinalize could be throwing exceptions more often than not due to a bug in Tk's code without we knowing about it.

Also, it is implemented on the proper way (__try ... __catch) for one platform, as a hack (__asm__) for some other platforms, and not implemented at all for the rest (which now is ARM.)

Finally, the way the __asm__ hack is implemented looks very fragile to me. I have doubts about how well it works in case an exception is thrown. It depends on many factors.

Looking into the history of this use of SEH, it begins here:

Date:   Sun Dec 21 23:50:13 2003 +0000
[...]
            * win/winMain.c:      DllMain's DLL_PROCESS_DETACH now protected with
                                  SEH as DeleteWindowsExitProc is causing an
                                  exception of its own under some teardown
                                  conditions.  AT&T assembly syntax has not been
                                  added for MinGW yet.  [Tcl Patch 858493]

Digging on the referenced patch:

https://sourceforge.net/p/tcl/patches/311/

it was motivated by a problem terminating Tk from a  __try ... __finally SEH handler. But gcc has no SEH, so why bother about faking it in DllMain at all?

The author expressed concerns similar to mine:

"I sure hope this won't mask
programming errors other developers might create in their
journeys of embedding."

Thanks, let's see: [f3c74805]

> but I suggest to consider what I mentioned yesterday about the sanity of hiding exceptions from the embedding application

Understood, but I also kind of understand why this was done: If an exception is thrown _inside_ TkFinalize, we don't want to stop it from doing further steps to unload the dll. The worst that could happen is a resource leak. But most application which load Tk only unload Tk when the application ends.

That's why I'm reluctant to change that.

Ok, that's because the build uses %rbp/%ebp for the frame pointer and my patch added that register to the __asm__ clobbers section.

So for "fixing" this we need to remove %rbp/%ebp from the clobbers list, when actually it is written to on the __asm__ block, i.e. lie to the compiler. Another hint about how horrible this "fake SEH" hack is.

That's a trivial "fix" (see below), but I suggest to consider what I mentioned yesterday about the sanity of hiding exceptions from the embedding application.

Adapted patch:

diff --git a/win/tkWin32Dll.c b/win/tkWin32Dll.c
index 5161333da..fd15e5206 100644
--- a/win/tkWin32Dll.c
+++ b/win/tkWin32Dll.c
@@ -148,13 +148,21 @@ DllMain(
 
 	    "movq	%%rdx,		%%gs:0"		"\n\t"
 
-	    /*
-	     * Call TkFinalize
-	     */
+	    :
+	    /* No outputs */
+	    :
+	    [registration]	"m"	(registration),
+	    [error]		"i"	(TCL_ERROR)
+	    :
+	    "%rax", "%rdx", "memory"
+	);
 
-	    "movq	$0x0,		0x0(%%rsp)"		"\n\t"
-	    "call	TkFinalize"			"\n\t"
+        /* Just do a regular C call so we don't need to worry about following
+         * the calling convention, specially the registers the function may
+         * clobber: */
+        TkFinalize(NULL);
 
+	__asm__ __volatile__ (
 	    /*
 	     * Come here on a normal exit. Recover the TCLEXCEPTION_REGISTRATION
 	     * and store a TCL_OK status
@@ -188,11 +196,9 @@ DllMain(
 	    :
 	    /* No outputs */
 	    :
-	    [registration]	"m"	(registration),
-	    [ok]		"i"	(TCL_OK),
-	    [error]		"i"	(TCL_ERROR)
+	    [ok]		"i"	(TCL_OK)
 	    :
-	    "%rax", "%rbx", "%rcx", "%rdx", "%rsi", "%rdi", "memory"
+	    "%rax", "%rdx", "memory"
 	);
 
 #   else
@@ -218,12 +224,18 @@ DllMain(
 
 	    "movl	%%edx,		%%fs:0"		"\n\t"
 
-	    /*
-	     * Call TkFinalize
-	     */
+	    :
+	    /* No outputs */
+	    :
+	    [registration]	"m"	(registration),
+	    [error]		"i"	(TCL_ERROR)
+	    :
+	    "%eax", "%ebx", "%edx", "memory"
+	);
+
+        TkFinalize(NULL);
 
-	    "movl	$0x0,		0x0(%%esp)"		"\n\t"
-	    "call	_TkFinalize"			"\n\t"
+	__asm__ __volatile__ (
 
 	    /*
 	     * Come here on a normal exit. Recover the TCLEXCEPTION_REGISTRATION
@@ -259,11 +271,9 @@ DllMain(
 	    :
 	    /* No outputs */
 	    :
-	    [registration]	"m"	(registration),
-	    [ok]		"i"	(TCL_OK),
-	    [error]		"i"	(TCL_ERROR)
+	    [ok]		"i"	(TCL_OK)
 	    :
-	    "%eax", "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"
+	    "%eax", "%ebx", "%edx", "memory"
 	);
 
 #   endif

Unfortunately, the build breaks for the --enable-symbols build:

https://github.com/tcltk/tk/actions/runs/19290295988/job/55159219445

Sorry, I have to revert this patch for now.

About those labels in the asm blocks in DllMain...

It is possible to move enough code to C so not to use labels within the asm blocks at all (using the GNU extension for taking the address of a label).

But that trickery of eating up the exception and pretending that nothing happened by just restoring the rbp/rsp registers to the values they had before the call to TkFinalize... it looks gross to me.

Do we really want to hide those exceptions? TkFinalize might corrupt memory and the embedding application would be none the wiser.

So this is about an application that embeds Tk, unloads it, some horrible thing happens in TkFinalize... and we want for the application to go its merry way. Is this realistic? Is it the right thing to do?

What if we delegate to the embedding application the handling of those exceptions?

Where does Tcl has such assembler? In tclWin32Dll.c there is an assembler block, but it does something different.

I searched the Tcl source code for __asm__ and, indeed, there are a few places for emulating SEH with __asm__. However:

1. Those are for 32 bit Windows only.

2. They wrap system calls (CloseHandle, CopyFile, MoveFile). None of them throws exceptions under normal circumstances. Only CloseHandle

"If the application is running under a debugger, the function will throw an exception if it receives either a handle value that is not valid or a pseudo-handle value."

https://learn.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle

IMO wrapping those calls with __try .. __catch (and the equivalent __asm__ hacks) is unnecessary.

OTOH, something in the comments of those __asm__ blocks catched my eye:

/* ... Note that this needs
     * to be one block of asm, to avoid stack imbalance; also, it is illegal
     * for one asm block to contain a jump to another.
     */

My patch does not jump from one block to another, but in the first block it takes the address of a label defined in the second block. Before sending the patch I checked that the address is correct by looking into the generated assembler code with gdb. The binary was compiled with -O2. Gcc has "asm goto" for dealing with labels defined outside of the current asm block. I'll investigate this when I have some spare time.

Just a question: Tcl has such assembler-code as well. Does Tcl have the same problem?

Fixed [9b98d74b754b9d75|here]. Thanks for the patch!

Forgot to mention that the original code was wrong on this too:

	    "movq	$0x0,		0x0(%%rsp)"		"\n\t"
	    "call	TkFinalize"			"\n\t"

On AMD64 the parameter is passed on a register, not on the stack.

Until now this worked because TkFinalize does not use its argument and that position of the stack apparently does not contain important stuff.

By moving the call from assembler to C, we fix that bug too.

In fact, there is more there that could be moved from assembler to C, but for now let's limit the patch to what's strictly necessary to address the reported crash.

Sounds like a good idea. Thanks!