MinGW setjmp SEGV

MinGW setjmp SEGV

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

Hi,

I have been trying to build Insight 6.1 (patched into GDB 6.3) for MinGW  
(and Linux and Solaris but they weren't much trouble).

The problem I hit has been previously reported here:  
http://sources.redhat.com/ml/insight/2004-q1/msg00021.html.

There are a number of patches required to make it build, but most are  
trivial and may not be relevant since 6.1 so I shall not post them.  
Basically I had to check whether the cygwin filename-handling tcl  
functions exist before using them, define __INSIDE_CYGWIN__ in a couple of  
tcl/tk files, and rename the hooks because they now have a 'deprecated'  
prefix.

However, the most serious failure was caused by broken code in the tcl dll  
screwing up the SEH and causing setjmp to fail. This problem has been  
fixed in the latest tcl at tcl.sourceforge.net.

The attached patch (code copied from sourceforge) is sufficient to fix the  
issue. Note that I have included the diff of the whole file and it  
includes one of the minor changes mentioned above.

Hopefully this patch will help others who have hit the same problem.

Andrew Stubbs

[-- Attachment #2: insight-win.patch --]
[-- Type: application/octet-stream, Size: 5419 bytes --]

--- /view/stubbsa-import/vob/insight.cmp/src/tcl/win/tclWin32Dll.c	2005-08-05 14:21:03.000000000 +0100
+++ /view/stubbsa-import/vob/insight.cmp/src/tcl/win/tclWin32Dll.c@@/main/INSIGHT-6.1	2005-07-29 16:22:32.000000000 +0100
@@ -12,10 +12,6 @@
  * RCS: @(#) $Id: tclWin32Dll.c,v 1.16 2002/06/13 09:40:01 vincentdarley Exp $
  */
 
-/* The following is a workaround for a w32api problem.
-   See http://sources.redhat.com/ml/insight/2004-q4/msg00039.html */
-#define __INSIDE_CYGWIN__
-
 #include "tclWinInt.h"
 
 /*
@@ -42,23 +38,6 @@
 static int platformId;		/* Running under NT, or 95/98? */
 
 #ifdef HAVE_NO_SEH
-/*
- * Unlike Borland and Microsoft, we don't register exception handlers by
- * pushing registration records onto the runtime stack. Instead, we register
- * them by creating an EXCEPTION_REGISTRATION within the activation record.
- */
-
-typedef struct EXCEPTION_REGISTRATION {
-    struct EXCEPTION_REGISTRATION *link;
-    EXCEPTION_DISPOSITION (*handler)(
-	    struct _EXCEPTION_RECORD*, void*, struct _CONTEXT*, void*);
-    void *ebp;
-    void *esp;
-    int status;
-} EXCEPTION_REGISTRATION;
-#endif
-
-#ifdef HAVE_NO_SEH
 static void *ESP;
 static void *EBP;
 #endif /* HAVE_NO_SEH */
@@ -368,111 +347,65 @@
 int
 TclpCheckStackSpace()
 {
-
-#ifdef HAVE_NO_SEH
-    EXCEPTION_REGISTRATION registration;
-#endif
     int retval = 0;
 
     /*
-     * We can recurse only if there is at least TCL_WIN_STACK_THRESHOLD bytes
-     * of stack space left. alloca() is cheap on windows; basically it just
-     * subtracts from the stack pointer causing the OS to throw an exception
-     * if the stack pointer is set below the bottom of the stack.
+     * We can recurse only if there is at least TCL_WIN_STACK_THRESHOLD
+     * bytes of stack space left.  alloca() is cheap on windows; basically
+     * it just subtracts from the stack pointer causing the OS to throw an
+     * exception if the stack pointer is set below the bottom of the stack.
      */
 
 #ifdef HAVE_NO_SEH
     __asm__ __volatile__ (
+            "movl  %esp, _ESP" "\n\t"
+            "movl  %ebp, _EBP");
 
-	/*
-	 * Construct an EXCEPTION_REGISTRATION to protect the call to __alloca
-	 */
-
-	"leal	%[registration], %%edx"		"\n\t"
-	"movl	%%fs:0,		%%eax"		"\n\t"
-	"movl	%%eax,		0x0(%%edx)"	"\n\t" /* link */
-	"leal	1f,		%%eax"		"\n\t"
-	"movl	%%eax,		0x4(%%edx)"	"\n\t" /* handler */
-	"movl	%%ebp,		0x8(%%edx)"	"\n\t" /* ebp */
-	"movl	%%esp,		0xc(%%edx)"	"\n\t" /* esp */
-	"movl	%[error],	0x10(%%edx)"	"\n\t" /* status */
-	
-	/*
-	 * Link the EXCEPTION_REGISTRATION on the chain
-	 */
-
-	"movl	%%edx,		%%fs:0"		"\n\t"
-
-	/*
-	 * Attempt a call to __alloca, to determine whether there's sufficient
-	 * memory to be had.
-	 */
-
-	"movl	%[size],	%%eax"		"\n\t"
-	"pushl	%%eax"				"\n\t"
-	"call	__alloca"			"\n\t"
-
-	/*
-	 * Come here on a normal exit. Recover the EXCEPTION_REGISTRATION and
-	 * store a TCL_OK status
-	 */
-
-	"movl	%%fs:0,		%%edx"		"\n\t"
-	"movl	%[ok],		%%eax"		"\n\t"
-	"movl	%%eax,		0x10(%%edx)"	"\n\t"
-	"jmp	2f"				"\n"
-
-	/*
-	 * Come here on an exception. Get the EXCEPTION_REGISTRATION that we
-	 * previously put on the chain.
-	 */
-
-	"1:"					"\t"
-	"movl	%%fs:0,		%%edx"		"\n\t"
-	"movl	0x8(%%edx),	%%edx"		"\n\t"
-	
-	/* 
-	 * Come here however we exited. Restore context from the
-	 * EXCEPTION_REGISTRATION in case the stack is unbalanced.
-	 */
-	
-	"2:"					"\t"
-	"movl	0xc(%%edx),	%%esp"		"\n\t"
-	"movl	0x8(%%edx),	%%ebp"		"\n\t"
-	"movl	0x0(%%edx),	%%eax"		"\n\t"
-	"movl	%%eax,		%%fs:0"		"\n\t"
-	
-	:
-	/* No outputs */
-	:
-	[registration]	"m"	(registration),
-	[ok]		"i"	(TCL_OK),
-	[error]		"i"	(TCL_ERROR),
-	[size]		"i"	(TCL_WIN_STACK_THRESHOLD)
-	:
-	"%eax", "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"
-	);
-    retval = (registration.status == TCL_OK);
-
-#else /* !HAVE_NO_SEH */
-    __try {
-#ifdef HAVE_ALLOCA_GCC_INLINE
-	__asm__ __volatile__ (
-	    "movl  %0, %%eax" "\n\t"
-	    "call  __alloca" "\n\t"
-	    :
-	    : "i"(TCL_WIN_STACK_THRESHOLD)
-	    : "%eax");
+    __asm__ __volatile__ (
+            "pushl $__except_checkstackspace_handler" "\n\t"
+            "pushl %fs:0" "\n\t"
+            "mov   %esp, %fs:0");
 #else
+    __try {
+#endif /* HAVE_NO_SEH */
 	alloca(TCL_WIN_STACK_THRESHOLD);
-#endif /* HAVE_ALLOCA_GCC_INLINE */
 	retval = 1;
+#ifdef HAVE_NO_SEH
+    __asm__ __volatile__ (
+            "jmp   checkstackspace_pop" "\n"
+            "checkstackspace_reentry:" "\n\t"
+            "movl  _ESP, %esp" "\n\t"
+            "movl  _EBP, %ebp");
+
+    __asm__ __volatile__ (
+            "checkstackspace_pop:" "\n\t"
+            "mov   (%esp), %eax" "\n\t"
+            "mov   %eax, %fs:0" "\n\t"
+            "add   $8, %esp");
+#else
     } __except (EXCEPTION_EXECUTE_HANDLER) {}
 #endif /* HAVE_NO_SEH */
-    
+
+    /*
+     * Avoid using control flow statements in the SEH guarded block!
+     */
     return retval;
 }
-
+#ifdef HAVE_NO_SEH
+static
+__attribute__ ((cdecl))
+EXCEPTION_DISPOSITION
+_except_checkstackspace_handler(
+    struct _EXCEPTION_RECORD *ExceptionRecord,
+    void *EstablisherFrame,
+    struct _CONTEXT *ContextRecord,
+    void *DispatcherContext)
+{
+    __asm__ __volatile__ (
+            "jmp checkstackspace_reentry");
+    return 0; /* Function does not return */
+}
+#endif /* HAVE_NO_SEH */
 \f
 /*
  *----------------------------------------------------------------------

RE: MinGW setjmp SEGV

[Dave Korn]

----Original Message----
>From: Andrew STUBBS
>Sent: 05 August 2005 14:44

> Hi,
> 
> I have been trying to build Insight 6.1 

> There are a number of patches required to make it build, but most are
> trivial and may not be relevant since 6.1 so I shall not post them.

> However, the most serious failure was caused by broken code in the tcl dll
> screwing up the SEH and causing setjmp to fail. 

> The attached patch (code copied from sourceforge) is sufficient to fix the
> issue. Note that I have included the diff of the whole file and it
> includes one of the minor changes mentioned above.

  That'll be the deleted _INSIDE_CYGWIN_ then?

> 
> Hopefully this patch will help others who have hit the same problem.


    Hi Andrew,

  I was looking through this area just the other day
(http://sourceware.org/ml/insight/2005-q3/msg00021.html), and ISTM that the
substance of this patch is already in CVS HEAD.  The changes appear to have
been added in rev1.9
(http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/tcl/win/tclWin32Dll.c?cvsr
oot=src#rev1.9) when tcl 8.4.1 was merged from upstream.  Just FYI.


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: MinGW setjmp SEGV

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

On Fri, 05 Aug 2005 15:00:53 +0100, Dave Korn <dave.korn@artimi.com> wrote:

>> The attached patch (code copied from sourceforge) is sufficient to fix  
>> the
>> issue. Note that I have included the diff of the whole file and it
>> includes one of the minor changes mentioned above.
>
>   That'll be the deleted _INSIDE_CYGWIN_ then?
>
>>
>> Hopefully this patch will help others who have hit the same problem.
>
>
>     Hi Andrew,
>
>   I was looking through this area just the other day
> (http://sourceware.org/ml/insight/2005-q3/msg00021.html), and ISTM that  
> the
> substance of this patch is already in CVS HEAD.  The changes appear to  
> have
> been added in rev1.9
> (http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/tcl/win/tclWin32Dll.c?cvsr
> oot=src#rev1.9) when tcl 8.4.1 was merged from upstream.  Just FYI.
>
>
>     cheers,
>       DaveK

Many apologies. I have got the diff backwards! It is tcl 8.4.1 that is  
broken and AFAIKT CVS HEAD is little altered.

Please find a correct one attached.

Andrew Stubbs

[-- Attachment #2: insight-win.patch --]
[-- Type: application/octet-stream, Size: 5413 bytes --]

--- /view/stubbsa-import/vob/insight.cmp/src/tcl/win/tclWin32Dll.c@@/main/INSIGHT-6.1	2005-07-29 16:22:32.000000000 +0100
+++ /view/stubbsa-import/vob/insight.cmp/src/tcl/win/tclWin32Dll.c	2005-08-05 14:21:03.000000000 +0100
@@ -12,6 +12,10 @@
  * RCS: @(#) $Id: tclWin32Dll.c,v 1.16 2002/06/13 09:40:01 vincentdarley Exp $
  */
 
+/* The following is a workaround for a w32api problem.
+   See http://sources.redhat.com/ml/insight/2004-q4/msg00039.html */
+#define __INSIDE_CYGWIN__
+
 #include "tclWinInt.h"
 
 /*
@@ -38,6 +42,23 @@
 static int platformId;		/* Running under NT, or 95/98? */
 
 #ifdef HAVE_NO_SEH
+/*
+ * Unlike Borland and Microsoft, we don't register exception handlers by
+ * pushing registration records onto the runtime stack. Instead, we register
+ * them by creating an EXCEPTION_REGISTRATION within the activation record.
+ */
+
+typedef struct EXCEPTION_REGISTRATION {
+    struct EXCEPTION_REGISTRATION *link;
+    EXCEPTION_DISPOSITION (*handler)(
+	    struct _EXCEPTION_RECORD*, void*, struct _CONTEXT*, void*);
+    void *ebp;
+    void *esp;
+    int status;
+} EXCEPTION_REGISTRATION;
+#endif
+
+#ifdef HAVE_NO_SEH
 static void *ESP;
 static void *EBP;
 #endif /* HAVE_NO_SEH */
@@ -347,65 +368,111 @@
 int
 TclpCheckStackSpace()
 {
+
+#ifdef HAVE_NO_SEH
+    EXCEPTION_REGISTRATION registration;
+#endif
     int retval = 0;
 
     /*
-     * We can recurse only if there is at least TCL_WIN_STACK_THRESHOLD
-     * bytes of stack space left.  alloca() is cheap on windows; basically
-     * it just subtracts from the stack pointer causing the OS to throw an
-     * exception if the stack pointer is set below the bottom of the stack.
+     * We can recurse only if there is at least TCL_WIN_STACK_THRESHOLD bytes
+     * of stack space left. alloca() is cheap on windows; basically it just
+     * subtracts from the stack pointer causing the OS to throw an exception
+     * if the stack pointer is set below the bottom of the stack.
      */
 
 #ifdef HAVE_NO_SEH
     __asm__ __volatile__ (
-            "movl  %esp, _ESP" "\n\t"
-            "movl  %ebp, _EBP");
 
-    __asm__ __volatile__ (
-            "pushl $__except_checkstackspace_handler" "\n\t"
-            "pushl %fs:0" "\n\t"
-            "mov   %esp, %fs:0");
-#else
+	/*
+	 * Construct an EXCEPTION_REGISTRATION to protect the call to __alloca
+	 */
+
+	"leal	%[registration], %%edx"		"\n\t"
+	"movl	%%fs:0,		%%eax"		"\n\t"
+	"movl	%%eax,		0x0(%%edx)"	"\n\t" /* link */
+	"leal	1f,		%%eax"		"\n\t"
+	"movl	%%eax,		0x4(%%edx)"	"\n\t" /* handler */
+	"movl	%%ebp,		0x8(%%edx)"	"\n\t" /* ebp */
+	"movl	%%esp,		0xc(%%edx)"	"\n\t" /* esp */
+	"movl	%[error],	0x10(%%edx)"	"\n\t" /* status */
+	
+	/*
+	 * Link the EXCEPTION_REGISTRATION on the chain
+	 */
+
+	"movl	%%edx,		%%fs:0"		"\n\t"
+
+	/*
+	 * Attempt a call to __alloca, to determine whether there's sufficient
+	 * memory to be had.
+	 */
+
+	"movl	%[size],	%%eax"		"\n\t"
+	"pushl	%%eax"				"\n\t"
+	"call	__alloca"			"\n\t"
+
+	/*
+	 * Come here on a normal exit. Recover the EXCEPTION_REGISTRATION and
+	 * store a TCL_OK status
+	 */
+
+	"movl	%%fs:0,		%%edx"		"\n\t"
+	"movl	%[ok],		%%eax"		"\n\t"
+	"movl	%%eax,		0x10(%%edx)"	"\n\t"
+	"jmp	2f"				"\n"
+
+	/*
+	 * Come here on an exception. Get the EXCEPTION_REGISTRATION that we
+	 * previously put on the chain.
+	 */
+
+	"1:"					"\t"
+	"movl	%%fs:0,		%%edx"		"\n\t"
+	"movl	0x8(%%edx),	%%edx"		"\n\t"
+	
+	/* 
+	 * Come here however we exited. Restore context from the
+	 * EXCEPTION_REGISTRATION in case the stack is unbalanced.
+	 */
+	
+	"2:"					"\t"
+	"movl	0xc(%%edx),	%%esp"		"\n\t"
+	"movl	0x8(%%edx),	%%ebp"		"\n\t"
+	"movl	0x0(%%edx),	%%eax"		"\n\t"
+	"movl	%%eax,		%%fs:0"		"\n\t"
+	
+	:
+	/* No outputs */
+	:
+	[registration]	"m"	(registration),
+	[ok]		"i"	(TCL_OK),
+	[error]		"i"	(TCL_ERROR),
+	[size]		"i"	(TCL_WIN_STACK_THRESHOLD)
+	:
+	"%eax", "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"
+	);
+    retval = (registration.status == TCL_OK);
+
+#else /* !HAVE_NO_SEH */
     __try {
-#endif /* HAVE_NO_SEH */
+#ifdef HAVE_ALLOCA_GCC_INLINE
+	__asm__ __volatile__ (
+	    "movl  %0, %%eax" "\n\t"
+	    "call  __alloca" "\n\t"
+	    :
+	    : "i"(TCL_WIN_STACK_THRESHOLD)
+	    : "%eax");
+#else
 	alloca(TCL_WIN_STACK_THRESHOLD);
+#endif /* HAVE_ALLOCA_GCC_INLINE */
 	retval = 1;
-#ifdef HAVE_NO_SEH
-    __asm__ __volatile__ (
-            "jmp   checkstackspace_pop" "\n"
-            "checkstackspace_reentry:" "\n\t"
-            "movl  _ESP, %esp" "\n\t"
-            "movl  _EBP, %ebp");
-
-    __asm__ __volatile__ (
-            "checkstackspace_pop:" "\n\t"
-            "mov   (%esp), %eax" "\n\t"
-            "mov   %eax, %fs:0" "\n\t"
-            "add   $8, %esp");
-#else
     } __except (EXCEPTION_EXECUTE_HANDLER) {}
 #endif /* HAVE_NO_SEH */
-
-    /*
-     * Avoid using control flow statements in the SEH guarded block!
-     */
+    
     return retval;
 }
-#ifdef HAVE_NO_SEH
-static
-__attribute__ ((cdecl))
-EXCEPTION_DISPOSITION
-_except_checkstackspace_handler(
-    struct _EXCEPTION_RECORD *ExceptionRecord,
-    void *EstablisherFrame,
-    struct _CONTEXT *ContextRecord,
-    void *DispatcherContext)
-{
-    __asm__ __volatile__ (
-            "jmp checkstackspace_reentry");
-    return 0; /* Function does not return */
-}
-#endif /* HAVE_NO_SEH */
+
 \f
 /*
  *----------------------------------------------------------------------

RE: MinGW setjmp SEGV

[Dave Korn]

----Original Message----
>From: Andrew STUBBS
>Sent: 05 August 2005 15:40


> Many apologies. I have got the diff backwards! It is tcl 8.4.1 that is
> broken and AFAIKT CVS HEAD is little altered.
> 
> Please find a correct one attached.
> 
> Andrew Stubbs

  Aha, that makes more sense, thanks.

  I don't understand the need for it yet, though; can you enlarge on the
problem or point me at a tcl.sourceforge bugzilla report?  This patch would
affect cygwin as well as mingw, since it also defines HAVE_NO_SEH; AFAIUI,
the macro means 'have no seh support in the compiler' rather than 'have no
seh in the target OS', but cygwin (apparently) is happy enough using 'doze
SEH, and I don't understand the comment about "Unlike Borland and Microsoft
we don't ....  pushing registration records onto the runtime stack."  Yet
the EXCEPTION_REGISTRATION variable "registration" is on the stack and
appears to be linked into the 'doze SEH chain in pretty much the usual
fashion.  I'm sure this is just my lack of comprehension.

  OTOH this patch would seem to address my concerns about the reentrancy
problems of using static _ESP and _EBP variables.

  Should or shouldn't the same changes be made to the exception handling in
tclWinFCmd.c and tclWinChan.c as well?

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: MinGW setjmp SEGV

[Andrew STUBBS]

>   I don't understand the need for it yet, though; can you enlarge on the
> problem or point me at a tcl.sourceforge bugzilla report?

See: http://sources.redhat.com/ml/insight/2004-q1/msg00021.html

The problem is that the assembly code is broken (reentrancy aside). It  
saves the value of %fs:0 (the pointer to the exception handler I think,  
but I'm not totally clear on this), but the restore code is broken and  
just writes random data. The next function in Insight to dereference this  
pointer is in setjmp so we see the crash there.

> This patch would
> affect cygwin as well as mingw, since it also defines HAVE_NO_SEH;  
> AFAIUI,
> the macro means 'have no seh support in the compiler' rather than 'have  
> no
> seh in the target OS', but cygwin (apparently) is happy enough using  
> 'doze
> SEH, and I don't understand the comment about "Unlike Borland and  
> Microsoft
> we don't ....  pushing registration records onto the runtime stack."  Yet
> the EXCEPTION_REGISTRATION variable "registration" is on the stack and
> appears to be linked into the 'doze SEH chain in pretty much the usual
> fashion.  I'm sure this is just my lack of comprehension.

As I understand it, the assembler code is supposed to simulate the '__try  
{} __except {}' stuff in compilers which do not support that. I don't know  
what compilers do support it though. Cygwin's 'gcc -mno-cygwin' does not.

I don't understand the comment either. Obviously the GCC compiled  
Insight/TCL does not use this stuff automatically, but the msvcrt.dll does  
use it - I can see it in the disassembly. I would imagine that any other  
MSVC compiled DLLs also use it. Perhaps it makes more sense along with the  
rest of the updated TCL, but as far as I can tell it is local to this one  
file and only used three times (once here).

>   OTOH this patch would seem to address my concerns about the reentrancy
> problems of using static _ESP and _EBP variables.

I don't know about that, I just cut a pasted the code from sourceforge.

>   Should or shouldn't the same changes be made to the exception handling  
> in
> tclWinFCmd.c and tclWinChan.c as well?

There is other similar code in sourceforge, but not all of it is relevant  
so I just took enough to fix the problem at hand. See  
http://cvs.sourceforge.net/viewcvs.py/tcl/tcl/win/tclWin32Dll.c?rev=1.46&view=auto

I have submitted this to help, but you should be aware that I am not  
certain whether any ST copyright stuff may be used in Insight  
(particularly the TCL parts), although we do have an assignment for GDB.  
That's part of the reason why I have not submitted the full set of patches  
to make Insight go. The TCL stuff from sourceforge is obviously not a  
problem, at least not if you get it from there yourself.

Andrew Stubbs

RE: MinGW setjmp SEGV

[Dave Korn]

----Original Message----
>From: Andrew STUBBS
>Sent: 05 August 2005 16:58

>  %fs:0 (the pointer to the exception handler I think,
> but I'm not totally clear on this)

  Absolutely so; %fs:0 points to the first exception handler record, it's
the head of a singly-linked list, you follow the chain of pointers at offset
(0) in each record and the handler function is at offset (4).  These EH
records are similar to but smaller than the EXCEPTION_REGISTRATION struct,
they basically consist of the first two members.  Since they are always
allocated by extending the stack at the time control enters the __try block
(alloca, essentially), they don't need a location to save %esp; we know that
%esp was equal to (base address of EH record + sizeof EH record) before the
_try block was entered, so can deduce the original value.

> As I understand it, the assembler code is supposed to simulate the '__try
> {} __except {}' stuff in compilers which do not support that. 

  Yep, that's exactly what it does.  The actual SEH functionality is
provided by the OS; the try/except is just syntactical candy.

> I don't know
> what compilers do support it though. 

  MSVC for sure, and quite possibly Borland too, judging from the comment.

>Cygwin's 'gcc -mno-cygwin' does not.

  As indeed does 'gcc -mcygwin', which is why it is using the HAVE_NO_SEH
assembler code, and why it will be affected (but seemingly for the better)
by this patch.

> I don't understand the comment either. Obviously the GCC compiled
> Insight/TCL does not use this stuff automatically, but the msvcrt.dll does
> use it - I can see it in the disassembly. 

  Not quite sure what you mean here.  The gcc-compiled tcl _does_ use 'this
stuff', as autoconf defines HAVE_NO_SEH.  By "the msvcrt.dll", I take it you
mean "the MSVC-compiled version of the tcl dll" rather than the actual MS
runtime library dll itself?  It would compile with HAVE_NO_SEH *un*defined,
meaning that it'll compile the actual MSVC-syntax _try/_except statements -
which basically compile down to almost exactly the same thing as the
assembler code there.

>I would imagine that any other
> MSVC compiled DLLs also use it. 

  The inline assembler essentially does what the MSVC compiler does
internally when wrapping a block in _try/_except.  The only substantial
difference is that MSVC doesn't use static variables to save the stackframe
context, it saves them in the exception record - so your patch makes our
code more like MSVC's way of invoking the OS's SEH support.  

  I think the comment is just incorrect.  And slightly incoherent.  AFAIUI,
"activation record" is a synonym for "stack frame", so "creating an
EXCEPTION_REGISTRATION record within the activation record" means pushing it
on the runtime stack to me.

> Perhaps it makes more sense along with the
> rest of the updated TCL, but as far as I can tell it is local to this one
> file and only used three times (once here).
> 
>>   OTOH this patch would seem to address my concerns about the reentrancy
>> problems of using static _ESP and _EBP variables.
> 
> I don't know about that, I just cut a pasted the code from sourceforge.

  I alluded to it in the patch I posted earlier
(http://sourceware.org/ml/insight/2005-q3/msg00021.html) but wasn't sure
about whether it was a real potential problem or not.

>>   Should or shouldn't the same changes be made to the exception handling
>> in tclWinFCmd.c and tclWinChan.c as well?
> 
> There is other similar code in sourceforge, but not all of it is relevant
> so I just took enough to fix the problem at hand. See
>
http://cvs.sourceforge.net/viewcvs.py/tcl/tcl/win/tclWin32Dll.c?rev=1.46&vie
w=auto
> 
> I have submitted this to help, but you should be aware that I am not
> certain whether any ST copyright stuff may be used in Insight
> (particularly the TCL parts), although we do have an assignment for GDB.
> That's part of the reason why I have not submitted the full set of patches
> to make Insight go. The TCL stuff from sourceforge is obviously not a
> problem, at least not if you get it from there yourself.

  Indeed.  I might take a look at this myself; I think the static variables
ESP and EBP presented a re-entrancy problem that this approach would fix.
Taking a look through the tcl sourcforge cvs, I see that the other files
indeed have been changed similarly, and that the changelog explicitly
mentions re-entrancy as the issue.

  Maybe it's just time to update the src/ version of tcl.

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: MinGW setjmp SEGV

[Steven Johnson]

Hi Andrew,

You may get better results from a CVS snapshot, if you dont want to 
check out CVS HEAD.  Can you try and build one, and see if that fixes 
your problem.

See the insight web page for details on the latest snapshots.  
http://sources.redhat.com/insight/

Steven

Andrew STUBBS wrote:

> Hi,
>
> I have been trying to build Insight 6.1 (patched into GDB 6.3) for 
> MinGW  (and Linux and Solaris but they weren't much trouble).
>
> The problem I hit has been previously reported here:  
> http://sources.redhat.com/ml/insight/2004-q1/msg00021.html.
>
> There are a number of patches required to make it build, but most are  
> trivial and may not be relevant since 6.1 so I shall not post them.  
> Basically I had to check whether the cygwin filename-handling tcl  
> functions exist before using them, define __INSIDE_CYGWIN__ in a 
> couple of  tcl/tk files, and rename the hooks because they now have a 
> 'deprecated'  prefix.
>
> However, the most serious failure was caused by broken code in the tcl 
> dll  screwing up the SEH and causing setjmp to fail. This problem has 
> been  fixed in the latest tcl at tcl.sourceforge.net.
>
> The attached patch (code copied from sourceforge) is sufficient to fix 
> the  issue. Note that I have included the diff of the whole file and 
> it  includes one of the minor changes mentioned above.
>
> Hopefully this patch will help others who have hit the same problem.
>
> Andrew Stubbs

Re: MinGW setjmp SEGV

[Andrew STUBBS]

On Sat, 06 Aug 2005 20:45:25 +0100, Steven Johnson  
<sjohnson@sakuraindustries.com> wrote:

> Hi Andrew,
>
> You may get better results from a CVS snapshot, if you dont want to  
> check out CVS HEAD.  Can you try and build one, and see if that fixes  
> your problem.

It won't. The files in question have not changed in CVS since the last tcl  
import. As I said in the original message, I have already fixed the  
problem. I was merely reporting how I had done so.

Thanks anyway.

Andrew Stubbs

Re: MinGW setjmp SEGV

[Dave Murphy]

Andrew STUBBS wrote:

> Hi,
>
> I have been trying to build Insight 6.1 (patched into GDB 6.3) for 
> MinGW  (and Linux and Solaris but they weren't much trouble).
>
> The problem I hit has been previously reported here:  
> http://sources.redhat.com/ml/insight/2004-q1/msg00021.html.
>
> There are a number of patches required to make it build, but most are  
> trivial and may not be relevant since 6.1 so I shall not post them.  
> Basically I had to check whether the cygwin filename-handling tcl  
> functions exist before using them, define __INSIDE_CYGWIN__ in a 
> couple of  tcl/tk files, and rename the hooks because they now have a 
> 'deprecated'  prefix.
>
> However, the most serious failure was caused by broken code in the tcl 
> dll  screwing up the SEH and causing setjmp to fail. This problem has 
> been  fixed in the latest tcl at tcl.sourceforge.net.
>
> The attached patch (code copied from sourceforge) is sufficient to fix 
> the  issue. Note that I have included the diff of the whole file and 
> it  includes one of the minor changes mentioned above.
>
> Hopefully this patch will help others who have hit the same problem.
>
> Andrew Stubbs

You're an absolute star :)

Thanks for posting the patch. I got inspired to have another go at 
building an arm-elf Insight under mingw/msys. I've now managed to build 
Inisght 6.1 with gdb 6.3 up to a point. I have a functioning gdb but I'm 
having problems with some deprecated functions in gdbtk-cmds.c and 
gdb-register.c. Do you know what I need to use instead of these functions?

bfd_get_section_size_before_reloc'
DEPRECATED_REGISTER_CONVERTIBLE'
DEPRECATED_REGISTER_CONVERT_TO_VIRTUAL
DEPRECATED_REGISTER_VIRTUAL_SIZE

gcc -g -O2       \
        -o insight.exe gdbtk-main.o libgdb.a \
          ../sim/arm/libsim.a ../bfd/libbfd.a ../readline/libreadline.a 
../opcodes/libopcodes.a  ../libiberty/libiberty.a    ../libgui/src/l
ibgui.a -L/c/projects/devkitPro/sources/arm-elf/insight/itcl/itcl 
-litcl32 -L/c/projects/devkitPro/sources/arm-elf/insight/itcl/itk -litk32
-L/c/projects/devkitPro/sources/arm-elf/insight/tk/win -ltk84 
-L/c/projects/devkitPro/sources/arm-elf/insight/tcl/win -ltcl84    
-lgdi32 -lc
omdlg32 -limm32 -lcomctl32 -lshell32 -lm  ../libiberty/libiberty.a 
-luser32 -lwsock32 -lpsapi
libgdb.a(gdbtk-cmds.o)(.text+0x2ee1): In function `gdb_load_info':
c:/projects/devkitPro/sources/arm-elf/insight/gdb/../../../insight/src-new/gdb/gdbtk/generic/gdbtk-cmds.c:901: 
undefined reference to `bfd_g
et_section_size_before_reloc'
libgdb.a(gdbtk-register.o)(.text+0x368): In function `get_register':
c:/projects/devkitPro/sources/arm-elf/insight/gdb/../../../insight/src-new/gdb/gdbtk/generic/gdbtk-register.c:305: 
undefined reference to `D
EPRECATED_REGISTER_CONVERTIBLE'
libgdb.a(gdbtk-register.o)(.text+0x38d):c:/projects/devkitPro/sources/arm-elf/insight/gdb/../../../insight/src-new/gdb/gdbtk/generic/gdbtk-r
egister.c:307: undefined reference to 
`DEPRECATED_REGISTER_CONVERT_TO_VIRTUAL'
libgdb.a(gdbtk-register.o)(.text+0x4b9):c:/projects/devkitPro/sources/arm-elf/insight/gdb/../../../insight/src-new/gdb/gdbtk/generic/gdbtk-r
egister.c:311: undefined reference to `DEPRECATED_REGISTER_VIRTUAL_SIZE'
collect2: ld returned 1 exit status
make: *** [insight.exe] Error 1

cross posted to gdb list in case anyone there can point me in the right 
direction.

Dave

Re: MinGW setjmp SEGV

[Andrew STUBBS]

Dave Murphy wrote:
> Thanks for posting the patch. I got inspired to have another go at 
> building an arm-elf Insight under mingw/msys. I've now managed to build 
> Inisght 6.1 with gdb 6.3 up to a point. I have a functioning gdb but I'm 
> having problems with some deprecated functions in gdbtk-cmds.c and 
> gdb-register.c. Do you know what I need to use instead of these functions?
> 
> bfd_get_section_size_before_reloc'

This can be replaced with bfd_get_section_size().

> DEPRECATED_REGISTER_CONVERTIBLE'
> DEPRECATED_REGISTER_CONVERT_TO_VIRTUAL
> DEPRECATED_REGISTER_VIRTUAL_SIZE

These can just be removed. See 
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/gdbtk/generic/gdbtk-register.c.diff?r1=1.24&r2=1.25&cvsroot=src&f=h

HTH
Andrew Stubbs

Re: MinGW setjmp SEGV

[Dave Murphy]

Andrew STUBBS wrote:

> Dave Murphy wrote:
>
>> bfd_get_section_size_before_reloc'
>
>
> This can be replaced with bfd_get_section_size().
>
>> DEPRECATED_REGISTER_CONVERTIBLE'
>> DEPRECATED_REGISTER_CONVERT_TO_VIRTUAL
>> DEPRECATED_REGISTER_VIRTUAL_SIZE
>
>
> These can just be removed. See 
> http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gdb/gdbtk/generic/gdbtk-register.c.diff?r1=1.24&r2=1.25&cvsroot=src&f=h 
>
>
Excellent, that was the final piece in the puzzle to get me up and 
running. Looking good on native i386. Tnanks again for finding the tcl 
problem :)


Dave