Modernize signal handling on Linux i386, PowerPC, and s390x#9543
Modernize signal handling on Linux i386, PowerPC, and s390x#9543xavierleroy merged 8 commits intoocaml:trunkfrom
Conversation
Use the same approach as the other Linux ports.
So that <signal.h> defines the names of registers in the context. Define it for all Linux ports, this should be harmless.
stedolan
left a comment
There was a problem hiding this comment.
Looks good!
While you're there, would you consider making the same change on the other ports that use it? Linux on Power, Linux on s390x, and "other BSDs" on Power are still using sa_handler and weird casts.
| typedef greg_t context_reg; | ||
| #define CONTEXT_PC (context->uc_mcontext.gregs[REG_EIP]) | ||
| #define CONTEXT_SP (context->uc_mcontext.gregs[REG_ESP]) | ||
| #define CONTEXT_FAULTING_ADDRESS ((char *)context->uc_mcontext.cr2) |
There was a problem hiding this comment.
Why uc_mcontext.cr2 rather than info->si_addr?
There was a problem hiding this comment.
Conservatism. Using CR2 is the way it was done before for i386 and the way it is still done for AMD64. If it ain't broke...
There was a problem hiding this comment.
If it ain't broke...
Then fix it until it is?
(Eventually, it would be nice to use the POSIX info->si_addr everywhere rather than per-platform tricks, but this may not be the PR for that change)
|
I didn't notice PowerPC and S390X were using the old approach too. I'll look into modernizing them too. The BSD ports for PowerPC are unmaintained, so I won't touch them. |
PowerPC 32 bits must keep using old-style signal handlers because "mcontext_t" does not contain the PC register. PowerPC 64 bits can use the new style, with a trick so that it works both with Glibc and with Musl (see PR#1795).
|
Why doesn't the same approach work on ppc32 and ppc64? (that is, using "register" 32) Linux on ppc32 also seems to store the PC right after the 32 general purpose registers. See the kernel sources here and here as well as in the very convenient |
First, I don't know, and since we don't have ppc32 hardware in the CI, I'd rather minimize ppc32 changes. (Plus: ppc32 will be removed eventually.) Second, the fields of |
|
CI precheck passed. One thing to note: by themselves the changes do not suffice to make stack overflow detection work on non-x86 platforms, but it's a step in this direction for ppc64le and s390x. |
To be clear, I wasn't suggesting sharing exactly the same code. I was suggesting using the same approach as glibc's
This makes sense! I'd forgotten that we had no means to CI ppc32 at the moment. |
|
Thanks for the careful review. I updated the description of this PR. Will now squash and merge. |
…ignals_nat.c vs upstream; pick up changes in ocaml#9543
Modernise signal handling (ocaml#9543)
…ignals_nat.c vs upstream; pick up changes in ocaml#9543
…row_gap_upstream_signals Modernise signal handling (ocaml#9543)
…ignals_nat.c vs upstream; pick up changes in ocaml#9543
…row_gap_upstream_signals Modernise signal handling (ocaml#9543)
OCaml's signal handlers need to know the state of the processor when the signal was received. The approach standardized by the Single Unix specification is tu use 3-argument signal handing functions and the
SA_SIGINFOflag tosigaction. However, as the Linux man page forsigactionsays,Undocumented:
Before the introduction of SA_SIGINFO, it was also possible to get some
additional information, namely by using a sa_handler with a second
argument of type struct sigcontext. See the relevant Linux kernel
sources for details. This use is obsolete now.
For historical reasons, the i386, PowerPC and s390x Linux ports of OCaml still use the undocumented, obsolete approach, while the other Linux ports use the modern
SA_SIGINFOapproach. [EDIT: added PowerPC and s390x to the list.]For consistency and future-proofing, this PR updates the i386 Linux port to use the modern approach to signal handling. [EDIT:] And also the PowerPC 64 bits and s390x ports.