Bug 140189 - libreofficekit & golang: fatal error: non-Go code set up signal handler without SA_ONSTACK flag
Summary: libreofficekit & golang: fatal error: non-Go code set up signal handler witho...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
7.1.0.3 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL: https://github.com/dveselov/go-libreo...
Whiteboard:
Keywords:
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2021-02-05 15:10 UTC by Christophe de Vienne
Modified: 2023-05-13 17:02 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Enable SA_STACK flag (415 bytes, patch)
2021-02-05 15:10 UTC, Christophe de Vienne
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christophe de Vienne 2021-02-05 15:10:58 UTC
Created attachment 169502 [details]
Enable SA_STACK flag

Trying to use https://github.com/dveselov/go-libreofficekit I get the following errors:

"""
signal 23 received but handler not on signal stack
fatal error: non-Go code set up signal handler without SA_ONSTACK flag

runtime stack:
runtime: unexpected return pc for runtime.sigtramp called from 0x7f25a7ad1228
stack: frame={sp:0xc000078ed8, fp:0xc000078f30} stack=[0xc000070e08,0xc000079208)
"""

Applying the attached patch on the latest release (7.1.0.3) fixed the issue, but I don't know if it could be acceptable as is or if adding the SA_ONSTACK flag should be done only with loading from libreofficekit in which case I have no idea where to start.

Here is the patch I applied:


diff --git a/sal/osl/unx/signal.cxx b/sal/osl/unx/signal.cxx
index 79721def6c5e..0694fc3c7b26 100644
--- a/sal/osl/unx/signal.cxx
+++ b/sal/osl/unx/signal.cxx
@@ -197,7 +197,7 @@ bool onInitSignal()
 
     struct sigaction act;
     act.sa_sigaction = signalHandlerFunction;
-    act.sa_flags = SA_RESTART | SA_SIGINFO;
+    act.sa_flags = SA_RESTART | SA_SIGINFO | SA_ONSTACK;
 
     sigfillset(&(act.sa_mask));
Comment 1 Christophe de Vienne 2021-04-23 07:55:44 UTC
The bug is confirmed by other users on:
https://github.com/dveselov/go-libreofficekit/issues/13
Comment 2 gschreiber 2022-08-22 11:42:20 UTC
I can confirm that the same issue is still present with

* go 1.18.5 and go 1.19
* LibreOffice 7.0.4.2 00(Build:2)
* Debian 11 (bullseye)
Comment 3 Buovjaga 2022-11-22 12:19:43 UTC
Thanks for the patch! If possible could you submit it to our code review system:

https://wiki.documentfoundation.org/Development/gerrit/setup
https://wiki.documentfoundation.org/Development/gerrit/SubmitPatch
https://wiki.documentfoundation.org/Development/GetInvolved#License_statement

If you need help, you can contact me via email.
Comment 4 Stephan Bergmann 2022-11-22 13:28:55 UTC
I don't know if including SA_ONSTACK in sigaction calls could have any negative consequences.  The main issue that comes to mind might be if a JVM is running in-process alongside the LibreOffice code, and the JVM might for some reason be picky about receiving its signals not on an altstack.  (I looked at <https://github.com/openjdk/jdk> as an example, to see whether or not the JVM itself would routinely include SA_ONSTACK in sigaction calls.  But it appears to not include it in general and only use it for one specific signal on macOS, see <https://github.com/openjdk/jdk/commit/2fad2249a2f769677c3c94a1ed23f1fc089701af> "8009302: Mac OS X: JVM crash on infinite recursion on Appkit Thread".)

Alternatively, LibreOfficeKit in general or that go-libreofficekit in particular might want to not call osl_addSignalHandler (which is the only code path that leads to that

>     act.sa_flags = SA_RESTART | SA_SIGINFO;

line) in the first place?