-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Fix illumox-x64 build with gcc and clang #129322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9abbcb4
72ba266
acc3f32
626a07c
cd79548
e0346a2
7fe19eb
838c141
344c6f7
9c5570e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,7 +225,7 @@ handler_from_sigaction (struct sigaction *sa) | |
| } | ||
| else | ||
| { | ||
| return sa->sa_handler; | ||
| return (VoidIntFn)sa->sa_handler; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -838,7 +838,10 @@ static int32_t ForkAndExecProcessInternal( | |
| struct sigaction sa_default; | ||
| struct sigaction sa_old; | ||
| memset(&sa_default, 0, sizeof(sa_default)); // On some architectures, sa_mask is a struct so assigning zero to it doesn't compile | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wstrict-prototypes" | ||
| sa_default.sa_handler = SIG_DFL; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are raised from system headers; I couldn't find a way to make clang happy, so I went with the suppression (and gcc surprisingly ignores it so we don't need additional |
||
| #pragma clang diagnostic pop | ||
| for (int sig = 1; sig < NSIG; ++sig) | ||
| { | ||
| if (sig == SIGKILL || sig == SIGSTOP) | ||
|
|
@@ -848,7 +851,12 @@ static int32_t ForkAndExecProcessInternal( | |
| if (!sigaction(sig, NULL, &sa_old)) | ||
| { | ||
| void (*oldhandler)(int) = handler_from_sigaction (&sa_old); | ||
| if (oldhandler != SIG_IGN && oldhandler != SIG_DFL) | ||
| bool hasCustomHandler; | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wstrict-prototypes" | ||
| hasCustomHandler = oldhandler != SIG_IGN && oldhandler != SIG_DFL; | ||
|
janvorli marked this conversation as resolved.
|
||
| #pragma clang diagnostic pop | ||
| if (hasCustomHandler) | ||
| { | ||
| // It has a custom handler, put the default handler back. | ||
| // We check first to preserve flags on default handlers. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,18 +69,28 @@ static bool IsSaSigInfo(struct sigaction* action) | |
| static bool IsSigDfl(struct sigaction* action) | ||
| { | ||
| assert(action); | ||
| bool isDefault; | ||
| // macOS can return sigaction with SIG_DFL and SA_SIGINFO. | ||
| // SA_SIGINFO means we should use sa_sigaction, but here we want to check sa_handler. | ||
| // So we ignore SA_SIGINFO when sa_sigaction and sa_handler are at the same address. | ||
| return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) && | ||
| action->sa_handler == SIG_DFL; | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wstrict-prototypes" | ||
| isDefault = (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) && | ||
| action->sa_handler == SIG_DFL; | ||
| #pragma clang diagnostic pop | ||
| return isDefault; | ||
| } | ||
|
|
||
| static bool IsSigIgn(struct sigaction* action) | ||
| { | ||
| assert(action); | ||
| return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) && | ||
| action->sa_handler == SIG_IGN; | ||
| bool isIgnored; | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wstrict-prototypes" | ||
| isIgnored = (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) && | ||
| action->sa_handler == SIG_IGN; | ||
| #pragma clang diagnostic pop | ||
| return isIgnored; | ||
| } | ||
|
|
||
| bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) | ||
|
|
@@ -239,7 +249,7 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context) | |
| else | ||
| { | ||
| assert(origHandler->sa_handler); | ||
| origHandler->sa_handler(sig); | ||
| ((void (*)(int))origHandler->sa_handler)(sig); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C and C++ have different types: struct sigaction {
int sa_flags;
union {
#ifdef __cplusplus
void (*_handler)(int);
#else
void (*_handler)();
#endif
#if defined(__EXTENSIONS__) || defined(_KERNEL) || \
(!defined(_STRICT_STDC) && !defined(__XOPEN_OR_POSIX)) || \
(_POSIX_C_SOURCE > 2) || defined(_XPG4_2)
void (*_sigaction)(int, siginfo_t *, void *);
#endif
} _funcptr;
sigset_t sa_mask;
#ifndef _LP64
int sa_resv[2];
#endif
};
#define sa_handler _funcptr._handler
#define sa_sigaction _funcptr._sigaction
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our rootfs has an older version of kernel. There is no new update from https://github.com/illumos/sysroot/ in a while.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you set sa_handler, the right type is |
||
| } | ||
|
|
||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed? It is strange that it would have MADV_FREE defined and it would fail in the code below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mono has
HAVE_MADVISE, coreclr doesn't. The error was about madvise not being defined:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, so sunos has MADV_FREE defined and yet no madvise, that's quite weird.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in <sys/mman.h>
Does the build supply -D_STRICT_POSIX or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's consistent once you see it's a namespace thing:
madviseis a non-standard SVR4/BSD extension,posix_madviseis the standardized one, and we build with-D_XPG4_2(strict X/Open). From the rootfs'ssys/mman.h:So on illumos the posix-y path is the preferred one, and
posix_madvise(POSIX_MADV_DONTNEED)is what they consider "modern".