Make exit_on_sigint work again #17727

yuyichao · 2016-07-31T00:29:16Z

Add more try-catch and sigatomic for top-level code/new tasks

So that we don't need to run jl_exit in strange (signal handler) context
due to missing exception handler.
Implement jl_call_in_ctx on unix.

Use it to make sure that jl_rethrow and jl_exit are running on the right
thread and right stack when an exception/exit is caused by a signal.

Fix Unreliable SIGINT delivery #17706

Tested locally on Linux x86/x64/aarch64/arm and Mac (windows sigint handling seems to be really unreliable in general). According to musl source code, it should work there too. Implementation on platforms that I don't have access to (FreeBSD, PowerPC) is left as an exercise for people who do. =)

This uses assembly to set the register values instead of using sigreturn since it doesn't work reliably on Mac and may not work in the future due to mitigation of sigreturn oriented programming. (Just like all other low level features that we want to use that is blocked by security fixes....)

The final implementation is on the edge of what I'm happy to merge during feature freeze/backport after branching so I'll let others decide when this should be merge and whether it should be backported to 0.5 branch.

vtjnash · 2016-07-31T01:30:54Z

src/signals-unix.c

+    // (Only a few actually needs these but doesn't hurt to do on all of them).
+    rsp &= -16; // ensure 16-byte alignment
+    rsp -= 128; // 128bytes red zone
+#if defined(_CPU_X86_64_)


tkelman's going to complain that MSVC doesn't support the asm keyword

Well, this is in ...-unix.c, I'll be happy to learn how to write this in msvc compatible way when we support using MSVC to build for unix =)

vtjnash · 2016-07-31T01:31:35Z

looks like a good improvement to me

tkelman · 2016-07-31T02:02:11Z

base/task.jl

@@ -105,6 +105,7 @@ suppress_excp_printing(t::Task) = isa(t.storage, ObjectIdDict) ? get(get_task_tl

 # runtime system hook called when a task finishes
 function task_done_hook(t::Task)
+    # `finish_task` set `sigatomic` before entering this function


yuyichao · 2016-07-31T06:06:39Z

Go back to modifying the ucontext since the concern I had was mainly due to a misunderstanding of a LWN article....

I'll need to repeat the test on the 5 architectures again.

yuyichao · 2016-07-31T06:20:12Z

Tests (make sure Ctrl-C with exit_on_sigint doesn't segfault most of the time) all passed.

I left jl_get_rsp_from_ctx as a separate function (it was not in an earlier implementation that modifies ucontext) since it is also useful for GC debugging code.

vtjnash · 2016-07-31T13:37:49Z

src/signals-unix.c

+    // Do not use the main stack if this is a stack overflow since that will
+    // not work....
+    if (e == jl_stackovf_exception)
+        jl_rethrow();


on mach, we re-use the signal stack for the rethrow call. should we eventually do that here too?

Set sp to the top of the signal stack and jmp to the function?

it does jl_call_in_ctx with sp set to the top of the signal stack (ptls2->signal_stack + sig_stack_size), to preserve call to sigreturn (and in the mach case, to get back to the right tid)

Sure, Just need quite a bit of signal stack space for this to be reliable.

yuyichao · 2016-08-01T00:20:11Z

Increase the signal stack size and use that stack for every call from the signal handler now.
This version relies much less on low level access to ucontext (only for playing nicely with the kernel). Also added a signal stack overflow detection (and call _exit).

tkelman · 2016-08-01T09:55:37Z

src/signals-mach.c

+    kern_return_t ret = thread_suspend(thread);
+    HANDLE_MACH_ERROR("thread_suspend", ret);
+
+    // This abort `sleep` and other syscall.


aborts sleep and other syscalls

JeffBezanson · 2016-08-02T16:24:25Z

I would probably not put this in 0.5.0 but can go in 0.5.x once we're confident that it works well.

vtjnash · 2016-08-09T20:50:58Z

what's left on this? it lgtm, and it's after branching now so that resolves the question of whether to do it before.

yuyichao · 2016-08-10T06:12:29Z

Should be ready. I guess I need to rebase though.

So that we don't need to run `jl_exit` in strange (signal handler) context due to missing exception handler.

Use it to make sure that `jl_rethrow` and `jl_exit` are running on the right thread and right stack when an exception/exit is caused by a signal. Fix #17706

yuyichao · 2016-08-10T06:43:25Z

Rebased.

(cherry picked from commit 6a62d30) ref #18063 Conflicts: src/signals-unix.c Not including changes to jl_call_in_ctx in this file because those changes were part of #17727 which has not been backported to release-0.5 at this time.

This makes the SIGINT signal handler work in combination with ASAN.

maleadt · 2016-08-20T18:44:59Z

I've extracted the signal stack size change for backporting: see 467e181. @yuyichao, does this look OK?

yuyichao · 2016-08-21T00:51:33Z

Yeah, should be fine and it should fix most of the symptoms.

This makes the SIGINT signal handler work in combination with ASAN. (partially cherry picked from commit 03c3c70, ref #17727)

vtjnash reviewed Jul 31, 2016
View reviewed changes

tkelman reviewed Jul 31, 2016
View reviewed changes

yuyichao force-pushed the yyc/signals/safer-exit branch from 3c56850 to 9648228 Compare July 31, 2016 06:06

vtjnash reviewed Jul 31, 2016
View reviewed changes

ViralBShah added this to the 0.5.x milestone Jul 31, 2016

yuyichao force-pushed the yyc/signals/safer-exit branch from 9648228 to 81ca3e6 Compare August 1, 2016 00:12

yuyichao mentioned this pull request Aug 1, 2016

Minor ASAN improvements #17731

Merged

tkelman reviewed Aug 1, 2016
View reviewed changes

yuyichao mentioned this pull request Aug 1, 2016

Segfault when hitting ^C on backtrace #17744

Closed

yuyichao force-pushed the yyc/signals/safer-exit branch 2 times, most recently from bf8479e to 82f0b93 Compare August 2, 2016 06:21

yuyichao modified the milestones: 0.6.0, 0.5.x Aug 2, 2016

yuyichao added 2 commits August 10, 2016 14:42

Add more try-catch and sigatomic for top-level code/new tasks

08465b4

So that we don't need to run `jl_exit` in strange (signal handler) context due to missing exception handler.

Implement jl_call_in_ctx on unix.

03c3c70

Use it to make sure that `jl_rethrow` and `jl_exit` are running on the right thread and right stack when an exception/exit is caused by a signal. Fix #17706

yuyichao force-pushed the yyc/signals/safer-exit branch from 82f0b93 to 03c3c70 Compare August 10, 2016 06:43

yuyichao merged commit a57b50d into master Aug 10, 2016

yuyichao deleted the yyc/signals/safer-exit branch August 10, 2016 11:27

tkelman mentioned this pull request Aug 20, 2016

improve FreeBSD / generic posix support #18063

Merged

1 task

maleadt mentioned this pull request Aug 20, 2016

Document use of sanitizers and provide default options #18067

Merged

maleadt added a commit that referenced this pull request Aug 20, 2016

Backport the signal stack size change from #17727.

467e181

This makes the SIGINT signal handler work in combination with ASAN.

maleadt mentioned this pull request Aug 20, 2016

[release-0.5] backports for 0.5.0-rc3 #18156

Merged

maleadt added a commit that referenced this pull request Aug 21, 2016

Increase the signal stack size.

d38ed89

This makes the SIGINT signal handler work in combination with ASAN. (partially cherry picked from commit 03c3c70, ref #17727)

maleadt added a commit that referenced this pull request Aug 21, 2016

Increase the signal stack size.

be350f2

This makes the SIGINT signal handler work in combination with ASAN. (partially cherry picked from commit 03c3c70, ref #17727)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make exit_on_sigint work again #17727

Make exit_on_sigint work again #17727

yuyichao commented Jul 31, 2016 •

edited

Loading

vtjnash Jul 31, 2016 •

edited

Loading

yuyichao Jul 31, 2016

vtjnash commented Jul 31, 2016

tkelman Jul 31, 2016

yuyichao commented Jul 31, 2016 •

edited

Loading

yuyichao commented Jul 31, 2016

vtjnash Jul 31, 2016

yuyichao Jul 31, 2016

vtjnash Jul 31, 2016

yuyichao Jul 31, 2016

yuyichao commented Aug 1, 2016

tkelman Aug 1, 2016

JeffBezanson commented Aug 2, 2016

vtjnash commented Aug 9, 2016

yuyichao commented Aug 10, 2016

yuyichao commented Aug 10, 2016

maleadt commented Aug 20, 2016

yuyichao commented Aug 21, 2016

Make exit_on_sigint work again #17727

Make exit_on_sigint work again #17727

Conversation

yuyichao commented Jul 31, 2016 • edited Loading

vtjnash Jul 31, 2016 • edited Loading

Choose a reason for hiding this comment

yuyichao Jul 31, 2016

Choose a reason for hiding this comment

vtjnash commented Jul 31, 2016

tkelman Jul 31, 2016

Choose a reason for hiding this comment

yuyichao commented Jul 31, 2016 • edited Loading

yuyichao commented Jul 31, 2016

vtjnash Jul 31, 2016

Choose a reason for hiding this comment

yuyichao Jul 31, 2016

Choose a reason for hiding this comment

vtjnash Jul 31, 2016

Choose a reason for hiding this comment

yuyichao Jul 31, 2016

Choose a reason for hiding this comment

yuyichao commented Aug 1, 2016

tkelman Aug 1, 2016

Choose a reason for hiding this comment

JeffBezanson commented Aug 2, 2016

vtjnash commented Aug 9, 2016

yuyichao commented Aug 10, 2016

yuyichao commented Aug 10, 2016

maleadt commented Aug 20, 2016

yuyichao commented Aug 21, 2016

yuyichao commented Jul 31, 2016 •

edited

Loading

vtjnash Jul 31, 2016 •

edited

Loading

yuyichao commented Jul 31, 2016 •

edited

Loading