public inbox for insight@sourceware.org
 help / color / mirror / Atom feed
From: "Dave Korn" <dave.korn@artimi.com>
To: "'Andrew STUBBS'" <andrew.stubbs@st.com>, <insight@sources.redhat.com>
Subject: RE: MinGW setjmp SEGV
Date: Fri, 05 Aug 2005 16:35:00 -0000	[thread overview]
Message-ID: <SERRANOjSWFiDWTQHtt000000d6@SERRANO.CAM.ARTIMI.COM> (raw)
In-Reply-To: <op.su1lac0io669wz@terrorhawk.bri.st.com>

----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....

  reply	other threads:[~2005-08-05 16:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-05 13:44 Andrew STUBBS
2005-08-05 14:01 ` Dave Korn
2005-08-05 14:40   ` Andrew STUBBS
2005-08-05 15:28     ` Dave Korn
2005-08-05 15:57       ` Andrew STUBBS
2005-08-05 16:35         ` Dave Korn [this message]
2005-08-05 21:43 ` Steven Johnson
2005-08-08  9:38   ` Andrew STUBBS
2005-09-21  2:25 ` Dave Murphy
2005-09-21  9:00   ` Andrew STUBBS
2005-09-21 21:25     ` Dave Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SERRANOjSWFiDWTQHtt000000d6@SERRANO.CAM.ARTIMI.COM \
    --to=dave.korn@artimi.com \
    --cc=andrew.stubbs@st.com \
    --cc=insight@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).