[PATCH] Fixing SEH exceptions for languages != C++

[PATCH] Fixing SEH exceptions for languages != C++

[Jonathan Schleifer]

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

Hi!

The following patch fixes a bug in SEH exception handling that made it
crash with ObjC (and most likely other languages as well). The problem
is that the SEH exception handler always passes the unwind exception as
4th parameter to RtlUnwindEx, which RtlUnwindEx then later passes to
the landing pad as argument. This works for C++, as libstdc++ sets data
register 0 to the unwind exception anyway, but it crashes for ObjC as
the landing pad expects the thrown object to be in data register 0. The
solution is of course to fix the SEH wrapper to get the value that was
set for data register 0 using _Unwind_SetGR and pass that to
RtlUnwindEx, so that later on the correct value is passed to the
landing pad.

The patch was tested for C++ and ObjC, the latter with both, the GNU
libobjc runtime and my own. (With -O0, it still crashed and complained
about invalid frames, but that is another issue.)

I don't think this patch needs transfer of copyright, as it is small
enoguh, so would it be possible to please include that in GCC 4.8.3?
This would finally make ObjC usable on Windows again - and most likely
other languages using exceptions as well.

Thanks!

PS: Please CC me as I'm not on the list!

-- 
Jonathan

[-- Attachment #2: gcc_seh.patch --]
[-- Type: text/x-patch, Size: 590 bytes --]

--- libgcc/unwind-seh.c.orig	2014-02-15 17:01:59.012396423 +0100
+++ libgcc/unwind-seh.c	2014-02-15 17:03:54.064755427 +0100
@@ -313,8 +313,9 @@
 	  ms_exc->ExceptionInformation[3] = gcc_context.reg[1];
 
 	  /* Begin phase 2.  Perform the unwinding.  */
-	  RtlUnwindEx (this_frame, gcc_context.ra, ms_exc, gcc_exc,
-		       ms_orig_context, ms_disp->HistoryTable);
+	  RtlUnwindEx (this_frame, gcc_context.ra, ms_exc,
+		       (PVOID)gcc_context.reg[0], ms_orig_context,
+		       ms_disp->HistoryTable);
 	}
 
       /* In _Unwind_RaiseException we return _URC_FATAL_PHASE1_ERROR.  */

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Mike Stump]

On Feb 15, 2014, at 9:27 AM, Jonathan Schleifer <js@webkeks.org> wrote:
> The following patch fixes a bug in SEH exception handling that made it
> crash with ObjC

From an ObjC perspective, I’m fine with the work; though, an seh person needs to weigh in.  I’m fine with the back port as well.

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Jonathan Schleifer]

Am 16.02.2014 um 16:47 schrieb Mike Stump <mikestump@comcast.net>:

> On Feb 15, 2014, at 9:27 AM, Jonathan Schleifer <js@webkeks.org> wrote:
>> The following patch fixes a bug in SEH exception handling that made it
>> crash with ObjC
> 
> From an ObjC perspective, I’m fine with the work; though, an seh person needs to weigh in.  I’m fine with the back port as well.

Is there anybody specific whom I should ping, like a maintainer for SEH exceptions in GCC?

--
Jonathan

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Kai Tietz]

2014-02-17 12:22 GMT+01:00 Jonathan Schleifer <js@webkeks.org>:
> Am 16.02.2014 um 16:47 schrieb Mike Stump <mikestump@comcast.net>:
>
>> On Feb 15, 2014, at 9:27 AM, Jonathan Schleifer <js@webkeks.org> wrote:
>>> The following patch fixes a bug in SEH exception handling that made it
>>> crash with ObjC
>>
>> From an ObjC perspective, I'm fine with the work; though, an seh person needs to weigh in.  I'm fine with the back port as well.
>
> Is there anybody specific whom I should ping, like a maintainer for SEH exceptions in GCC?
>
> --
> Jonathan

Hi Mike,

the patch is reasonable, and my testings haven't shown any
regressions.  So from that POV patch would be ok.
Nevertheless it would be good to have a bug-report for it, and it is a
regression from SjLj.

Kai

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Jonathan Schleifer]

Am 18.02.2014 um 17:41 schrieb Kai Tietz <ktietz70@googlemail.com>:

> Hi Mike,
> 
> the patch is reasonable, and my testings haven't shown any
> regressions.  So from that POV patch would be ok.
> Nevertheless it would be good to have a bug-report for it, and it is a
> regression from SjLj.

I reported it being a regression before GCC 4.8.0 was released:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56870

--
Jonathan

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Kai Tietz]

2014-02-18 17:43 GMT+01:00 Jonathan Schleifer <js@webkeks.org>:
> Am 18.02.2014 um 17:41 schrieb Kai Tietz <ktietz70@googlemail.com>:
>
>> Hi Mike,
>>
>> the patch is reasonable, and my testings haven't shown any
>> regressions.  So from that POV patch would be ok.
>> Nevertheless it would be good to have a bug-report for it, and it is a
>> regression from SjLj.
>
> I reported it being a regression before GCC 4.8.0 was released:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56870
>
> --
> Jonathan

So patch is ok with proper ChangeLog mentioning PR.  Patch is ok for
back-port too.

Thanks,
Kai

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Jonathan Schleifer]

Am Tue, 18 Feb 2014 17:51:00 +0100
schrieb Kai Tietz <ktietz70@googlemail.com>:

> So patch is ok with proper ChangeLog mentioning PR.  Patch is ok for
> back-port too.

I wonder if the instaned of RtlUnwindEx that come before the patched
line should be changed as well, though.

--
Jonathan

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Jonathan Schleifer]

There is also definitely a use-after-free if you call _Unwind_DeleteException in your personality before returning _URC_INSTALL_CONTEXT (which you should, if you don't want to leak and your landing pad doesn't call it). I'm not sure though how to fix it. It seems the problem that register 0 is ignored is present throughout the whole file and it seems that a proper fix gets a little bit more complicated.

--
Jonathan

Re: [PATCH] Fixing SEH exceptions for languages != C++

[Kai Tietz]

Hello Jonathan,

2014-02-20 20:19 GMT+01:00 Jonathan Schleifer <js@webkeks.org>:
> There is also definitely a use-after-free if you call _Unwind_DeleteException in your personality before returning _URC_INSTALL_CONTEXT (which you should, if you don't want to leak and your landing pad doesn't call it). I'm not sure though how to fix it. It seems the problem that register 0 is ignored is present throughout the whole file and it seems that a proper fix gets a little bit more complicated.
>
> --
> Jonathan

Sorry for replying a bit late to your thread.  For current stage of
gcc further changes in this area need to be postponed.
The use-after-free issue you mention is for sure something we should
address in upcoming stage 1. So you are welcome to work on this area.
You might want to make already your papers with fsf, as they are
required for further contributions to gcc.

Regards,
Kai