libc: support to run LibC signals on separate stack
alex-ab opened this issue · 12 comments
@chelmuth: i pushed two branches, the one eager stack allocation and the second with lazy (on first use) allocation with your comments incorporated
I merged the lazy variant to staging.
One last point that I see not addresses is the following remark from https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaltstack.html.
Signals that have been explicitly declared to execute on the alternate stack shall be delivered on the alternate stack.
In combination with https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaction.html this means.
SA_ONSTACK
If set and an alternate signal stack has been declared with sigaltstack(), the signal shall be delivered to the calling process on that stack. Otherwise, the signal shall be delivered on the current stack.
SO, per default signals are still delivered on the user stack.
@chelmuth: thanks for the info, I added a fixup which runs the signal only on the alternative stack if the S_ONSTACK is set actually
@alex-ab [init -> depot_autopilot] [init -> test-libc] bails out on all platforms.
[2024-07-25 06:15:28] [init -> depot_autopilot] 3.300 [init -> test-libc] test_sigalt stack=0x403fe990
[2024-07-25 06:15:28] [init -> depot_autopilot] 3.308 [init -> test-libc] test_signal_handler stack=0x404feea8
[2024-07-25 06:15:28] [init -> depot_autopilot] 3.317 [init -> test-libc] Warning: attempt to nested execution of signal handlers
[2024-07-25 06:15:28] [init -> depot_autopilot] 3.328 [init -> test-libc] test_signal_handler done
[2024-07-25 06:15:28] [init -> depot_autopilot] 3.335 [init -> test-libc] Warning: attempt to nested execution of signal handler
[2024-07-25 06:15:28] [init -> depot_autopilot] 3.346 [init -> test-libc] Error: leaking secondary stack memory
[2024-07-25 06:15:28] [init -> depot_autopilot]
[2024-07-25 06:15:28] [init -> depot_autopilot] test-libc failed 3.346 log
Hmm, just had a look at the logs of failing run/gdb on nova.
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
...
[2024-08-14 04:16:54] gdb: warning: Couldn't determine a path for the index cache directory.
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
...
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
[2024-08-14 04:16:54] Reading symbols from debug/ld.lib.so...
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
[2024-08-14 04:16:54] [init -> gdb] Warning: sigaltstack using self chosen stack is not supported - stack ptr is ignored !!!
The actual reason is the ram_quota request:
[2024-08-14 04:16:56] [init] child "gdb" requests resources: ram_quota=2101248�[0m�[0m
[2024-08-14 04:17:16] Error: Test execution timed out
With the sigaltstack commit, alternative stacks are indeed allocated (not done before) and so more memory is required. With increasing the memory a bit, the test succeeds.
The warnings is, as agreed with you @chelmuth in person, to warn about the fact that the actual stack pointer provided by the caller (gdb) can't be used, because of the internal nature of our stacks on Genode. Your argument was, in such cases, the contrib code should be adjusted. I will look into the gdb code, whether this is possible.
@chelmuth: please have a look, I added two commits. Even so I avoided the double allocation, the increase of the ram quota on the nightly test machine is necessary.
@alex-ab: the gdb contrib code is also used for the host gdb (tool/tool_chain script). In case the change is not compatible with glibc, something like #ifdef __GENODE__ might be needed. gdbserver_genode.patch still contains such checks for the removed gdbserver component, but it looks like the __GENODE__ macro is currently not defined for gdb on Genode.
The additional RAM demand seems not related to the sigaltstack commit as the test failed before the commit was merged
[2024-07-24 04:47:44] #5 Genode::Signal_receiver::block_for_signal (
[2024-07-24 04:47:44] this=this@entry=0x139f90 <Genode::bootstrap_component(Genode::Platform&)::startup+3536>) at /data/genode/repos/base/src/lib/base/signal.cc:280
[2024-07-24 04:47:44] [init] child "gdb" requests resources: ram_quota=2101248
and with the commit in the same way.
[2024-07-25 06:58:56] #5 Genode::Signal_receiver::block_for_signal (
[2024-07-25 06:58:56] this=this@entry=0x139f90 <Genode::bootstrap_component(Genode::Platform&)::startup+3536>) at /data/genode/repos/base/src/lib/base/signal.cc:280
[2024-07-25 06:58:56] [init] child "gdb" requests resources: ram_quota=2101248
@cproc: thanks, did not know about this. I tried to add in repos/ports/src/noux-pkg/gdb/target.inc -D__GENODE__ , but did not succeed. Do you have a idea, where to add this differentiation best ?
@cproc: thanks, did not know about this. I tried to add in repos/ports/src/noux-pkg/gdb/target.inc -D__GENODE__ , but did not succeed. Do you have a idea, where to add this differentiation best ?
It is the right place, but the definition caused problems with the changes from gdbserver_genode.patch. Commit 6fcc9df removes gdbserver_genode.patch and commit d54a3b9 adds the Genode check to your fix.