SigAction isn't missing a field on amd64?

[Alogani]

Description

Hello,

I see the SigAction struct of posix miss the field sa_sigaction*: proc (x: cint, y: ptr SigInfo, z: pointer) {.noconv.} for amd64

Nim/lib/posix/posix_linux_amd64.nim

Lines 304 to 313 in 767a901

	Sigaction* {.importc: "struct sigaction",
	header: "<signal.h>", final, pure.} = object ## struct sigaction
	sa_handler*: proc (x: cint) {.noconv.} ## Pointer to a signal-catching
	## function or one of the macros
	## SIG_IGN or SIG_DFL.
	sa_mask*: Sigset ## Set of signals to be blocked during execution of
	## the signal handling function.
	sa_flags*: cint ## Special flags.
	sa_restorer: proc() {.noconv.} ## not intended for application use.

However, I have tested in c, it seems we can define use this field succesfully on my amd64 machine

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>
#include <string.h>

#define SEGV_STACK_SIZE (MINSIGSTKSZ + 4096)

void sigsegv_handler(int signum, siginfo_t *info, void *data) {
    void *addr = info->si_addr;
    printf("Segmentation fault from reading the address %p\n", addr);
    exit(1);
}

int main() {
    stack_t segv_stack; // #Stack
    segv_stack.ss_sp = posix_memalign(SEGV_STACK_SIZE); //#-> to import, included already in stdlib.h
    segv_stack.ss_flags = 0;
    segv_stack.ss_size = SEGV_STACK_SIZE;
    sigaltstack(&segv_stack, NULL); // #sigaltstack

    struct sigaction action; // #Sigaction
    action.sa_flags = SA_SIGINFO | SA_ONSTACK;
    action.sa_sigaction = &sigsegv_handler;
    sigaction(SIGSEGV, &action, NULL); // #sigaction

    printf("Installed signal handler for SIGSEGV.\n");
    
     int* ptr = NULL;
    *ptr = 42;

    return 0;
}

Nim Version

v2.0.4 Fedora, on amd64

Current Output

No response

Expected Output

No response

Possible Solution

No response

Additional Information

No response

[juancarlospaco]

Make a pull request? 🤔

[Alogani]

Yeah sure ! I was hesitant out of fear that something might explode or I don't know what, I have never contributed before 🙂

[litlighilit]

For struct sigaction, POSIX manual¹ has such declaration:

On some architectures a union² is involved: do not assign to both sa_handler and sa_sigaction.

We can tell two points from it:

1. sa_sigaction field is a must for struct sigaction
2. but it may be overlapped, as it may be in a union. In Nim, it may not mean "missing a field", but using one field to present one union.

Edit:

I later found in the code below the definition, the getter and setter is defined as templates.

Footnotes

1. formal specification here ↩

2. For example, SigAction definition in posix_macos_amd64.nim uses one field of function pointer, that's, on MacOS, according to Apple doc, sa_sigaction and sa_handler are defined as one union (by defining only one field, its layout is right, as union of two pointer is still one-pointer length), though its fields' order is different from linux's. ↩

[litlighilit]

@Alogani

Could you please refer to the <signal.h> header file under your system (Fedora, as you mentioned) about struct sigaction to check if there are unions in it?

I've had a look at Debain's, and there's union in struct sigaction, and the fields' order is the same as nim's definition, so the layout in posix_linux_amd64.nim is correct for such linux.

Different linux distributions may have different definitions.... which requires further inspect and discussion...

[litlighilit]

Further steps:

1. Leave a comment about such a field represent a unoin in some linux like Debain.
2. Once in one linux distribution or other UNIX like FreeBSD, we find a different layout, add a new posix_*.nim and define a different SigAction object.

[litlighilit]

FreeBSD's ¹ is defined as follows:

struct sigaction {
    void    (*sa_handler)(int);
    void    (*sa_sigaction)(int, siginfo_t *, void *);
    int     sa_flags;		     /* see signal options below */
    sigset_t sa_mask;		     /* signal mask to apply */
};

in which case a field is indeed missing.

So may we add a new posix_freebsd*.nim

---

What's more, OpenBSD's is the same as what's defined in posix_linux_amd64.nim,
but in Nim its fields' order is wrong?!

Footnotes

1. note FreeBSD is of POSIX, not Linux ↩

[Alogani]

Wow, that a very detailed research you did. Awesome. I had put more info in the PR request, but the PR request is wrong indeed as you noticed well. I will rewrite the information i found there below

Here it is for my fedora :

/* Structure describing the action to be taken when a signal arrives.  */
struct sigaction
  {
    /* Signal handler.  */
#if defined __USE_POSIX199309 || defined __USE_XOPEN_EXTENDED
    union
      {
	/* Used if SA_SIGINFO is not set.  */
	__sighandler_t sa_handler;
	/* Used if SA_SIGINFO is set.  */
	void (*sa_sigaction) (int, siginfo_t *, void *);
      }
    __sigaction_handler;
# define sa_handler	__sigaction_handler.sa_handler
# define sa_sigaction	__sigaction_handler.sa_sigaction
#else
    __sighandler_t sa_handler;
#endif

    /* Additional set of signals to be blocked.  */
    __sigset_t sa_mask;

    /* Special flags.  */
    int sa_flags;

    /* Restore handler.  */
    void (*sa_restorer) (void);
  };

If I'm not wrong, this is the standard: https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/bits/sigaction.h#L32-L55

But there is something strange, on my machine, there is sa_restorer field, but not on glibc source. In nim, sa_restorer has been defined in multiple different architecture.

[carrexxii]

@Alogani
I have the exact same definition as you do on Arch. If you read the comment above the definition, it says that there should be a system-dependent version for the struct:

/* These definitions match those used by the 4.4 BSD kernel.
   If the operating system has a `sigaction' system call that correctly
   implements the POSIX.1 behavior, there should be a system-dependent
   version of this file that defines `struct sigaction' and the `SA_*'
   constants appropriately.  */

And I think rather check the main repos and not an old mirror to be safe, but it doesn't look like it has changed at all.

[litlighilit]

But there is something strange, on my machine, there is sa_restorer field, but not on glibc source. In nim, sa_restorer has been defined in multiple different architecture.

glibc's definition really seems to contain no sa_restorer field....

Maybe many linux distributions do not use glibc's definition directly.

[Alogani]

So instead of using basing on linux specific architecture, we should put all defs in posix common architecture and use compile switch for each distro -> https://nim-lang.org/docs/distros.html

There is also the problem of how to represent it (dicussion on the PR):
#23705 (comment)

Because some systems uses a union inside a an object, naive implementation will have a wrong sizeof.

[litlighilit]

__sighandler_t sa_handler is a function pointer, so is sa_sigaction.

Isn't each function pointer in C the same size?¹

(as function pointer is still pointer).

So as far as I'm concerned, despite araq's worry, a union of two pointer is still of one pointer length.

Therefore the sizeof will give the right result.

Footnotes

1. It has to be mentioned that Nim's proc is of nimcall convention by default, so it's two pointer length. Thus mark a proc as another convetion like noconv is required. ↩

[Alogani]

@litlighilit

I wasn't sure of what you said, so I verified, and you are indeed right:

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>


int main() {
    struct sigaction action; // #Sigaction
    printf("%ld", sizeof(action));

    return 0;
}

Output = 152

type  
   Sigaction2* {.importc: "struct sigaction", 
               header: "<signal.h>", final, pure.} = object ## struct sigaction 
     sa_sigaction*: proc (x: cint, y: pointer, z: pointer) {.noconv.}
     sa_handler*: proc (x: cint) {.noconv.}
                                           ## SIG_IGN or SIG_DFL. 
     sa_mask*: Sigset ## Set of signals to be blocked during execution of 
                     ## the signal handling function. 
     sa_flags*: cint   ## Special flags. 
     sa_restorer: proc() {.noconv.}   ## not intended for application use. 


  
var a = Sigaction2()
echo sizeof(a)

Output = 152

Because there can be multiple different variations by distribution and only one topic is a bit messy to check that, I propose you make an issue for each different distribution here: https://github.com/Alogani/Nim/issues

Then, we could either:

• remove all definitions of Sigaction from posix_linx/posix_macos, etc to put them in lib/posix/posix.nim with test on which distribution
• Or don't move their definitions and add a test of the specific distro in each file

[litlighilit]

Ops,

I meant either using a union inside object or defining only one function pointer field keeps the struct layout valid.

But defining two field together like above breaks the validaity of such a struct, as in your system, the two function pointers is only one pointer length.

---

Also, I later realized that sizeof in Nim is in fact mapped to C's sizeof so it reflects nothing about the size of a object in Nim side.