Opened 9 years ago

Closed 6 years ago

#2695 closed enhancement (fixed)

[XSI] Implement SA_SIGINFO for sigaction

Reported by: Andreas Färber Owned by: Ingo Weinhold
Priority: normal Milestone: R1
Component: System/POSIX Version: R1/Development
Keywords: Cc: haiku@…, kallisti5@…, service.exchange@…
Blocked By: #1935 Blocking:
Has a Patch: no Platform: All

Description

Haiku currently supports default POSIX signal handlers with no additional signal info, and BeOS' derived signal handlers with additional register access and user-supplied data.

Not yet supported are the extended POSIX signal handlers with siginfo_t and ucontext_t arguments. Some parts thereof are already prepared in the headers but commented out since not implemented on the kernel side.

http://www.opengroup.org/onlinepubs/009695399/functions/sigaction.html

I had recently submitted a first draft patch for the x86 kernel side signal frame setup. Some fields are not initialized yet, and some information is not (yet) easily obtainable according to Ingo.

http://www.freelists.org/archives/haiku-development/08-2008/msg00262.html

Attachments (3)

siginfo_2009-11-17.diff (5.5 KB) - added by Andreas Färber 8 years ago.
Git branch from 2008-08-11, rebased
siginfo_2010-01-25.diff (5.7 KB) - added by Andreas Färber 8 years ago.
rebased branch
siginfo-2010-04-05.diff (14.6 KB) - added by Andreas Färber 8 years ago.
draft patch

Download all attachments as: .zip

Change History (20)

comment:1 Changed 9 years ago by scottmc

Cc: haiku@… added

comment:2 Changed 8 years ago by Grzegorz Dąbrowski

Component: SystemSystem/POSIX

Changed 8 years ago by Andreas Färber

Attachment: siginfo_2009-11-17.diff added

Git branch from 2008-08-11, rebased

comment:3 Changed 8 years ago by Andreas Färber

I've attached my incomplete work on this, in case someone wants to pick it up.

My patches still cleanly applied to HEAD and didn't appear to crash the system. Maybe the kernel part could be integrated already while leaving the preprocessor symbols undefined for userland apps?

comment:4 Changed 8 years ago by Alexander von Gluck

+1 for someone implementing this.

This is what currently holds ruby 1.9 back from cleanly compiling cleanly on Haiku.

http://redmine.ruby-lang.org/issues/show/2640

comment:5 Changed 8 years ago by Alexander von Gluck

Cc: kallisti5@… added

Changed 8 years ago by Andreas Färber

Attachment: siginfo_2010-01-25.diff added

rebased branch

comment:6 Changed 8 years ago by Andreas Färber

Blocked By: 5324 added

comment:7 Changed 8 years ago by Stephan Aßmus

Thanks for the patch. I am too clueless with regards to POSIX to know whether this should already be applied or not. Maybe someone with more overview of the possible effects could have a look.

comment:8 Changed 8 years ago by Alexander von Gluck

this bug can be worked around on the application side, but if an app assumes that the full POSIX standard is adopted it will fail to compile.

Ruby added a work around in http://redmine.ruby-lang.org/repositories/revision/ruby-19?rev=26399

#if defined(SIGSEGV) && defined(HAVE_SIGALTSTACK) && defined(SA_SIGINFO)		
#define USE_SIGALTSTACK		
#endif

If SA_SIGINFO is defined down the road improperly it may cause problems as code will try and use it.

comment:9 Changed 8 years ago by Ingo Weinhold

Blocked By: 5324 removed

(In #5324) ucontext_t and mcontext_t should rather be added as part of implementing SA_SIGINFO (the only feature using these structures), which in turn should be implemented as part of real-time signal support. Cf. #1935.

comment:9 Changed 8 years ago by Ingo Weinhold

Blocked By: 1935 added
Resolution: duplicate
Status: newclosed

Sorry for not doing this earlier. As discussed via mail, this feature cannot be implemented without real-time signal support. Therefore closing it as duplicate of #1935.

comment:10 Changed 8 years ago by Andreas Färber

Resolution: duplicate
Status: closedreopened

Following a discussion on HaikuPorts-devs about how to handle lack of SA_SIGINFO, I've given this a new try. Not having implemented sigqueue or waitid, I'm reopening this ticket.

Signals are actually queued in signal.cpp this time and the faulting address of a SIGSEGV is stuffed into si_addr in vm.cpp. The x86/arch_thread.cpp frame setup code is unchanged.

Unfortunately it doesn't work yet, due to use of malloc while interrupts disabled. It's probably not thread-safe either. Comments on how to improve appreciated!

KDL output follows, from QEMU during splash screen rocket:

PANIC: memalign(): called with interrupts disabled

Welcome to Kernel Debugging Land...
Thread 12 "main2" running on CPU 0
stack trace for thread 12 "main2"
    kernel stack: 0x81d10000 to 0x81d14000
frame               caller     <image>:function + offset
 0 81d13d5c (+  32) 800df3c4   <kernel_x86>:arch_debug_stack_trace + 0x000f
 1 81d13d7c (+  16) 8006ce98   <kernel_x86> stack_trace_trampoline(void*: NULL) + 0x000b
 2 81d13d8c (+  12) 800e591a   <kernel_x86>:arch_debug_call_with_fault_handler + 0x001b
 3 81d13d98 (+  48) 8006d38e   <kernel_x86>:debug_call_with_fault_handler + 0x0053
 4 81d13dc8 (+  64) 8006e142   <kernel_x86> kernel_debugger_loop(char const*: 0x0 "<NULL>", char const*: 0x8011f8c0 "memalign(): called with interrupts disabled
", char*: 0x81d13e38, int32: -2147032130) + 0x0212
 5 81d13e08 (+  48) 8006e3dc   <kernel_x86> kernel_debugger_internal(char const*: 0x0 "<NULL>", char const*: 0x8280c018 "", char*: 0x81d13e58, int32: -2147031636) + 0x0110
 6 81d13e38 (+  32) 8006e5bf   <kernel_x86>:panic + 0x0023
 7 81d13e58 (+  80) 8004947f   <kernel_x86>:memalign + 0x002a
 8 81d13ea8 (+  32) 8004976a   <kernel_x86>:malloc + 0x0010
 9 81d13ec8 (+  48) 800592d9   <kernel_x86> deliver_signal(thread*: 0x828578bd, siginfo_t const*: NULL, uint32: 0x81d13f68) + 0x0077
10 81d13ef8 (+  48) 80059919   <kernel_x86> send_signal_etc_AF(int32: -2116993200, siginfo_t const*: NULL, uint32: 0x24 (36)) + 0x00b5
11 81d13f28 (+  80) 80059a9c   <kernel_x86>:send_signal_etc + 0x0027
12 81d13f78 (+  32) 80062a0e   <kernel_x86>:resume_thread + 0x0022
13 81d13f98 (+  64) 8004f0c7   <kernel_x86> main2(void*: NULL) + 0x01d6
14 81d13fd8 (+  32) 80062150   <kernel_x86> _create_kernel_thread_kentry() + 0x0015
15 81d13ff8 (+2116993032) 80066487   <kernel_x86> thread_kthread_exit() + 0x0000
kdebug> 

Changed 8 years ago by Andreas Färber

Attachment: siginfo-2010-04-05.diff added

draft patch

comment:11 in reply to:  10 ; Changed 8 years ago by Ingo Weinhold

Replying to andreasf:

Following a discussion on HaikuPorts-devs about how to handle lack of SA_SIGINFO, I've given this a new try. Not having implemented sigqueue or waitid, I'm reopening this ticket.

I'd rather see the realtime signal support tackled (or at least designed) en bloc to avoid surprises of the "oops, to add this feature we have to rewrite the whole stuff" kind. While that is often not that case, we *do* have a wonderfully complete and precise feature specification (except maybe for the user debugger interface). So it would make most sense to first go through the features and derive requirements for the kernel implementation, and only afterwards design and implement the thing.

Signals are actually queued in signal.cpp this time and the faulting address of a SIGSEGV is stuffed into si_addr in vm.cpp. The x86/arch_thread.cpp frame setup code is unchanged.

Unfortunately it doesn't work yet, due to use of malloc while interrupts disabled. It's probably not thread-safe either. Comments on how to improve appreciated!

Welcome to kernel development! Invocation of malloc(), free(), or any other function that uses locking primitives other than spinlocks is forbidden when interrupts are disabled. Usually that means moving allocations before disabling interrupts and postponing freeing until after re-enabling them. I haven't investigated whether there is a reasonable work-around in this case or whether we might have to rethink the locking concept for a few major kernel locks (team and thread spinlock) first.

comment:12 in reply to:  11 Changed 8 years ago by Andreas Färber

Well my design suggestion was implicit:

  • Extend in-kernel interfaces to pass siginfo_t* around (*_AF).
  • For existing exported functions, provide wrappers that do an instant signal -> siginfo_t conversion.
  • Queue signals per thread, like you suggested. Directly use siginfo_t for that.
  • As before, the existing sigaction structs can be used to determine the expected signature.

What's still missing:

  • Fix the malloc issue, for early testing.
  • Comment out sig_pending and migrate any remaining uses.
  • For signals arriving from userspace, set SI_USER flag.
  • Any signal-specific info.
  • Add an extra char *userData parameter for SA_SIGINFO handlers, like I suggested.

To later extend this with sigqueue, all that's needed is:

  • A new kernel function to obtain the team, choose which thread(s) to queue it on and call one of the above new functions.
  • Any userspace plumbing; sigvalue is already part of my patch.

http://www.opengroup.org/onlinepubs/9699919799/functions/sigqueue.html

waitid seems only vaguely related. There's already code to notify a parent of signals; my guess is this is rather related to scheduling such a waiting parent thread from inside the signal-handling code than to the actual queue implementation attempted here. http://www.opengroup.org/onlinepubs/9699919799/functions/waitid.html

comment:13 Changed 7 years ago by HaikuBot

Cc: service.exchange@… added
Version: R1/pre-alpha1R1/Development

comment:14 Changed 7 years ago by HaikuBot

This ticket is blocks successful port of VLC 1.0.x and Cherokee Web Server, and who knows maybe some others great apps. Maybe someone take a look it closer?

comment:15 Changed 7 years ago by Ingo Weinhold

Owner: changed from axeld to Ingo Weinhold
Status: reopenedin-progress

Applied siginfo-2010-04-05.diff partially in hrev41568.

comment:16 Changed 6 years ago by Ingo Weinhold

Resolution: fixed
Status: in-progressclosed

Implemented in hrev42116.

Note: See TracTickets for help on using tickets.