code review 104200046: runtime: ignore exceptions from foreign threads.

runtime: ignore exceptions from foreign threads.
Fixes issue 8224.

[dfc, 2014-06-18 07:52:16]

This looks pretty safe but I think the asm needs a comment

[brainman, 2014-06-19 01:35:12]

LGTM

Alex

[minux, 2014-06-19 02:45:44]

Hello alex.brainman@gmail.com, rsc@golang.org, dave@cheney.net (cc:
golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2014-06-19 03:05:43]

LGTM

[brainman, 2014-06-19 04:24:21]

Still LGTM

Alex

[brainman, 2014-07-10 01:45:17]

minux, have you forgotten to submit this?

Alex

[minux, 2014-07-10 03:55:37]

*** Submitted as https://code.google.com/p/go/source/detail?r=14683b1cf2cc ***

runtime: ignore exceptions from foreign threads.
Fixes issue 8224.

LGTM=alex.brainman, rsc
R=alex.brainman, rsc, dave
CC=golang-codereviews
https://codereview.appspot.com/104200046

[minux, 2014-07-10 03:55:51]

On 2014/07/10 01:45:17, brainman wrote:
> minux, have you forgotten to submit this?
yes, thank you for the reminder!

[gobot, 2014-07-10 04:16:33]

This CL appears to have broken the openbsd-386-rootbsd builder.
See http://build.golang.org/log/f0c8bc423ad6e8ebd1c2e8daa89fb33e2c8cae83

[minux, 2014-07-10 04:26:12]

On Thu, Jul 10, 2014 at 12:16 AM, <gobot@golang.org> wrote:

> This CL appears to have broken the openbsd-386-rootbsd builder.
> See http://build.golang.org/log/f0c8bc423ad6e8ebd1c2e8daa89fb33e2c8cae83

unrelated, but looks real?

fatal error: unexpected signal during runtime execution
[signal 0xb code=0x1 addr=0x18 pc=0x805c2af]

runtime stack:
runtime.throw(0x821dcca)
/usr/local/go/src/pkg/runtime/panic.c:523 +0x65
runtime.sigpanic()
/usr/local/go/src/pkg/runtime/os_openbsd.c:220 +0x3a
acquirep(0x0)
/usr/local/go/src/pkg/runtime/proc.c:2451 +0x4f
stopm()
/usr/local/go/src/pkg/runtime/proc.c:966 +0x144
exitsyscall0(0x18402640)
/usr/local/go/src/pkg/runtime/proc.c:1703 +0xbe
runtime.mcall(0x8070597)
/usr/local/go/src/pkg/runtime/asm_386.s:192 +0x3f
....
FAIL runtime/pprof 5.083s

[brainman, 2014-07-10 04:30:54]

On 2014/07/10 04:26:12, minux wrote:
> On Thu, Jul 10, 2014 at 12:16 AM, <mailto:gobot@golang.org> wrote:
> 
> > This CL appears to have broken the openbsd-386-rootbsd builder.
> > See http://build.golang.org/log/f0c8bc423ad6e8ebd1c2e8daa89fb33e2c8cae83
> 
> unrelated, but looks real?
> 

I wouldn't know. But to me looks like corrupted memory.

Alex

[minux, 2014-07-10 04:46:12]

On Thu, Jul 10, 2014 at 12:30 AM, <alex.brainman@gmail.com> wrote:

> On 2014/07/10 04:26:12, minux wrote:
>
>> On Thu, Jul 10, 2014 at 12:16 AM, <mailto:gobot@golang.org> wrote:
>
>  > This CL appears to have broken the openbsd-386-rootbsd builder.
>> > See
>>
> http://build.golang.org/log/f0c8bc423ad6e8ebd1c2e8daa89fb33e2c8cae83
>
>> unrelated, but looks real?
>
> I wouldn't know. But to me looks like corrupted memory.
>
yes, same to me.

do you think if we should add a test to misc/cgo/test/cthread_windows.c for
exceptions happening
on foreign threads?

the sketch is:
at the end of addThread, set up SEH, trigger an exception, and when SEH
handler is invoked, call
_endthreadex(0).

VEH has precedence over SEH, so it should ensure that our VEH handler gets
called first.

we can't add the same test to unix, as on those platforms, signals coming
from foreign threads will
crash the whole process.

what do you think? I don't have easy access to windows machine now, so if
I'm going to write it,
it will be during this weekend, the earliest.

[brainman, 2014-07-10 05:22:03]

On 2014/07/10 04:46:12, minux wrote:
> 
> do you think if we should add a test to misc/cgo/test/cthread_windows.c for
> exceptions happening
> on foreign threads?
> 

I think I already tried (when we discovered this problem). I will think about it
again. Please, leave it with me for now.

Alex

[brainman, 2014-07-11 00:45:18]

On 2014/07/10 04:46:12, minux wrote:
> 
> the sketch is:
> at the end of addThread, set up SEH, trigger an exception, and when SEH
> handler is invoked, call
> _endthreadex(0).

That should work, but how do you propose to "set up SEH"?

Alternatively we can make Go asm test function (I can resurrect our old
windows-386 SEH code, windows-amd64 would be harder, but we can test on 386
only) and call it via syscall.CreateThread. How is that sounds to you?

Alex

[minux, 2014-07-11 01:38:59]

On Thu, Jul 10, 2014 at 8:45 PM, <alex.brainman@gmail.com> wrote:

> On 2014/07/10 04:46:12, minux wrote:
>
>> the sketch is:
>> at the end of addThread, set up SEH, trigger an exception, and when
>>
> SEH
>
>> handler is invoked, call
>> _endthreadex(0).
>>
>
> That should work, but how do you propose to "set up SEH"?
>
mingw doesn't support __try and __catch, so we will have to set up SEH
using inline assembly for 386.
i.e. allocate exception_registration struct on stack, and then use inline
assembly to chain into SEH chain.
(http://www.programmingunlimited.net/siteexec/content.cgi?page=mingw-seh)

the problem is how could we test on windows/amd64?

>
> Alternatively we can make Go asm test function (I can resurrect our old
> windows-386 SEH code, windows-amd64 would be harder, but we can test on
> 386 only) and call it via syscall.CreateThread. How is that sounds to
> you?
>

[brainman, 2014-07-11 01:51:30]

On 2014/07/11 01:38:59, minux wrote:
> >
> mingw doesn't support __try and __catch, so we will have to set up SEH
> using inline assembly for 386.
> i.e. allocate exception_registration struct on stack, and then use inline
> assembly to chain into SEH chain.
> (http://www.programmingunlimited.net/siteexec/content.cgi?page=mingw-seh)

I know nothing about all that, so it is all yours.

> 
> the problem is how could we test on windows/amd64?

We don't test on windows/amd64.

Alex