* [PATCH] libgo: Access to glibc-specific field in struct sigevent [not found] ` <CAOyqgcVgWaCvV6cTpZF+Q0rSV5DAdsdCTxOd36L0SWoo302L7Q@mail.gmail.com> @ 2022-08-20 10:31 ` Sören Tempel 2022-09-05 12:31 ` Sören Tempel 2022-09-23 13:59 ` [PATCH v2] libgo: Portable access to thread ID " soeren 0 siblings, 2 replies; 4+ messages in thread From: Sören Tempel @ 2022-08-20 10:31 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc-patches, gofrontend-dev Hi Ian, Thanks for your input! I am not familiar with Go runtime internals at all, but the patch below at least compiles for me. Let me know if this works for you too / if there is a better way to do this. The untyped uintptr_t seems to be necessary as otherwise I encountered errors about &sevp escaping to the heap. Sincerely, Sören diff --git a/libgo/go/runtime/os_linux.go b/libgo/go/runtime/os_linux.go index 96fb1788..6653d85e 100644 --- a/libgo/go/runtime/os_linux.go +++ b/libgo/go/runtime/os_linux.go @@ -22,6 +22,8 @@ type mOS struct { profileTimerValid uint32 } +func setProcID(uintptr, int32) + func getProcID() uint64 { return uint64(gettid()) } @@ -365,7 +367,7 @@ func setThreadCPUProfiler(hz int32) { var sevp _sigevent sevp.sigev_notify = _SIGEV_THREAD_ID sevp.sigev_signo = _SIGPROF - *((*int32)(unsafe.Pointer(&sevp._sigev_un))) = int32(mp.procid) + setProcID(uintptr(unsafe.Pointer(&sevp)), int32(mp.procid)) ret := timer_create(_CLOCK_THREAD_CPUTIME_ID, &sevp, &timerid) if ret != 0 { // If we cannot create a timer for this M, leave profileTimerValid false diff --git a/libgo/runtime/go-signal.c b/libgo/runtime/go-signal.c index 528d9b6d..347b24e2 100644 --- a/libgo/runtime/go-signal.c +++ b/libgo/runtime/go-signal.c @@ -183,6 +183,16 @@ setSigactionHandler(struct sigaction* sa, uintptr handler) sa->sa_sigaction = (void*)(handler); } +void setProcID(uintptr_t, int32_t) + __asm__ (GOSYM_PREFIX "runtime.setProcID"); + +void +setProcID(uintptr_t ptr, int32_t v) +{ + struct sigevent *s = (void *)ptr; + s->sigev_notify_thread_id = v; +} + // C code to fetch values from the siginfo_t and ucontext_t pointers // passed to a signal handler. Ian Lance Taylor <iant@golang.org> wrote: > On Fri, Aug 12, 2022 at 10:21 AM Sören Tempel <soeren@soeren-tempel.net> wrote: > > > > This is one of the few remaining issues regarding musl compatibility for > > gofrontend. Unfortunately, I am not sure how to best fix this one. Hence > > reporting it here. > > > > The setThreadCPUProfiler implementation in libgo/go/runtime/os_linux.go > > contains the following code: > > > > var sevp _sigevent > > sevp.sigev_notify = _SIGEV_THREAD_ID > > sevp.sigev_signo = _SIGPROF > > *((*int32)(unsafe.Pointer(&sevp._sigev_un))) = int32(mp.procid) > > > > I am not entirely sure, but I assume the last line is supposed to access > > the thread ID in struct sigevent. Unfortunately, it does so via the > > glibc-specific _sigev_un member which doesn't work on musl libc (the > > union is named __sev_fields on musl and hence causes a build failure). > > > > A portable way to access the thread ID in struct sigevent is via > > sigev_notify_thread_id which is documented in timer_create(2). This is a > > macro provided in siginfo.h from linux-headers for glibc [1] and in > > signal.h for musl [2]. However, since it is macro it probably requires a > > C wrapper function in order to be usable in libgo. Would be possible to > > add that somehow to libgo or is there an alternative feasible solution > > for this issue? > > I think you're right that a tiny C function is the way to go here. > > Ian ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgo: Access to glibc-specific field in struct sigevent 2022-08-20 10:31 ` [PATCH] libgo: Access to glibc-specific field in struct sigevent Sören Tempel @ 2022-09-05 12:31 ` Sören Tempel 2022-09-23 13:59 ` [PATCH v2] libgo: Portable access to thread ID " soeren 1 sibling, 0 replies; 4+ messages in thread From: Sören Tempel @ 2022-09-05 12:31 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc-patches, gofrontend-dev Hello Ian, Have you had the chance to look at this yet? Let me know if there is a better way to do this. Greetings, Sören Sören Tempel <soeren@soeren-tempel.net> wrote: > Hi Ian, > > Thanks for your input! > > I am not familiar with Go runtime internals at all, but the patch below > at least compiles for me. Let me know if this works for you too / if > there is a better way to do this. The untyped uintptr_t seems to be > necessary as otherwise I encountered errors about &sevp escaping to the > heap. > > Sincerely, > Sören > > diff --git a/libgo/go/runtime/os_linux.go b/libgo/go/runtime/os_linux.go > index 96fb1788..6653d85e 100644 > --- a/libgo/go/runtime/os_linux.go > +++ b/libgo/go/runtime/os_linux.go > @@ -22,6 +22,8 @@ type mOS struct { > profileTimerValid uint32 > } > > +func setProcID(uintptr, int32) > + > func getProcID() uint64 { > return uint64(gettid()) > } > @@ -365,7 +367,7 @@ func setThreadCPUProfiler(hz int32) { > var sevp _sigevent > sevp.sigev_notify = _SIGEV_THREAD_ID > sevp.sigev_signo = _SIGPROF > - *((*int32)(unsafe.Pointer(&sevp._sigev_un))) = int32(mp.procid) > + setProcID(uintptr(unsafe.Pointer(&sevp)), int32(mp.procid)) > ret := timer_create(_CLOCK_THREAD_CPUTIME_ID, &sevp, &timerid) > if ret != 0 { > // If we cannot create a timer for this M, leave profileTimerValid false > diff --git a/libgo/runtime/go-signal.c b/libgo/runtime/go-signal.c > index 528d9b6d..347b24e2 100644 > --- a/libgo/runtime/go-signal.c > +++ b/libgo/runtime/go-signal.c > @@ -183,6 +183,16 @@ setSigactionHandler(struct sigaction* sa, uintptr handler) > sa->sa_sigaction = (void*)(handler); > } > > +void setProcID(uintptr_t, int32_t) > + __asm__ (GOSYM_PREFIX "runtime.setProcID"); > + > +void > +setProcID(uintptr_t ptr, int32_t v) > +{ > + struct sigevent *s = (void *)ptr; > + s->sigev_notify_thread_id = v; > +} > + > // C code to fetch values from the siginfo_t and ucontext_t pointers > // passed to a signal handler. > > Ian Lance Taylor <iant@golang.org> wrote: > > On Fri, Aug 12, 2022 at 10:21 AM Sören Tempel <soeren@soeren-tempel.net> wrote: > > > > > > This is one of the few remaining issues regarding musl compatibility for > > > gofrontend. Unfortunately, I am not sure how to best fix this one. Hence > > > reporting it here. > > > > > > The setThreadCPUProfiler implementation in libgo/go/runtime/os_linux.go > > > contains the following code: > > > > > > var sevp _sigevent > > > sevp.sigev_notify = _SIGEV_THREAD_ID > > > sevp.sigev_signo = _SIGPROF > > > *((*int32)(unsafe.Pointer(&sevp._sigev_un))) = int32(mp.procid) > > > > > > I am not entirely sure, but I assume the last line is supposed to access > > > the thread ID in struct sigevent. Unfortunately, it does so via the > > > glibc-specific _sigev_un member which doesn't work on musl libc (the > > > union is named __sev_fields on musl and hence causes a build failure). > > > > > > A portable way to access the thread ID in struct sigevent is via > > > sigev_notify_thread_id which is documented in timer_create(2). This is a > > > macro provided in siginfo.h from linux-headers for glibc [1] and in > > > signal.h for musl [2]. However, since it is macro it probably requires a > > > C wrapper function in order to be usable in libgo. Would be possible to > > > add that somehow to libgo or is there an alternative feasible solution > > > for this issue? > > > > I think you're right that a tiny C function is the way to go here. > > > > Ian ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] libgo: Portable access to thread ID in struct sigevent 2022-08-20 10:31 ` [PATCH] libgo: Access to glibc-specific field in struct sigevent Sören Tempel 2022-09-05 12:31 ` Sören Tempel @ 2022-09-23 13:59 ` soeren 2022-09-27 16:31 ` Ian Lance Taylor 1 sibling, 1 reply; 4+ messages in thread From: soeren @ 2022-09-23 13:59 UTC (permalink / raw) To: iant; +Cc: gcc-patches, gofrontend-dev From: Sören Tempel <soeren@soeren-tempel.net> Tested on x86_64 Arch Linux (glibc) and Alpine Linux (musl libc). Previously, libgo relied on the _sigev_un implementation-specific field in struct sigevent, which is only available on glibc. This patch uses the sigev_notify_thread_id macro instead which is mandated by timer_create(2). In theory, this should work with any libc implementation for Linux. Unfortunately, there is an open glibc bug as glibc does not define this macro. For this reason, a glibc-specific workaround is required. Other libcs (such as musl) define the macro and don't require the workaround. This makes go_signal compatible with musl libc. See: https://sourceware.org/bugzilla/show_bug.cgi?id=27417 Signed-off-by: Sören Tempel <soeren@soeren-tempel.net> --- Changes since v1: Add workaround for glibc. libgo/go/runtime/os_linux.go | 4 +++- libgo/runtime/go-signal.c | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/libgo/go/runtime/os_linux.go b/libgo/go/runtime/os_linux.go index 96fb1788..6653d85e 100644 --- a/libgo/go/runtime/os_linux.go +++ b/libgo/go/runtime/os_linux.go @@ -22,6 +22,8 @@ type mOS struct { profileTimerValid uint32 } +func setProcID(uintptr, int32) + func getProcID() uint64 { return uint64(gettid()) } @@ -365,7 +367,7 @@ func setThreadCPUProfiler(hz int32) { var sevp _sigevent sevp.sigev_notify = _SIGEV_THREAD_ID sevp.sigev_signo = _SIGPROF - *((*int32)(unsafe.Pointer(&sevp._sigev_un))) = int32(mp.procid) + setProcID(uintptr(unsafe.Pointer(&sevp)), int32(mp.procid)) ret := timer_create(_CLOCK_THREAD_CPUTIME_ID, &sevp, &timerid) if ret != 0 { // If we cannot create a timer for this M, leave profileTimerValid false diff --git a/libgo/runtime/go-signal.c b/libgo/runtime/go-signal.c index 528d9b6d..c56350cc 100644 --- a/libgo/runtime/go-signal.c +++ b/libgo/runtime/go-signal.c @@ -16,6 +16,11 @@ #define SA_RESTART 0 #endif +// Workaround for https://sourceware.org/bugzilla/show_bug.cgi?id=27417 +#if __linux__ && !defined(sigev_notify_thread_id) + #define sigev_notify_thread_id _sigev_un._tid +#endif + #ifdef USING_SPLIT_STACK extern void __splitstack_getcontext(void *context[10]); @@ -183,6 +188,16 @@ setSigactionHandler(struct sigaction* sa, uintptr handler) sa->sa_sigaction = (void*)(handler); } +void setProcID(uintptr_t, int32_t) + __asm__ (GOSYM_PREFIX "runtime.setProcID"); + +void +setProcID(uintptr_t ptr, int32_t v) +{ + struct sigevent *s = (void *)ptr; + s->sigev_notify_thread_id = v; +} + // C code to fetch values from the siginfo_t and ucontext_t pointers // passed to a signal handler. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libgo: Portable access to thread ID in struct sigevent 2022-09-23 13:59 ` [PATCH v2] libgo: Portable access to thread ID " soeren @ 2022-09-27 16:31 ` Ian Lance Taylor 0 siblings, 0 replies; 4+ messages in thread From: Ian Lance Taylor @ 2022-09-27 16:31 UTC (permalink / raw) To: soeren; +Cc: gcc-patches, gofrontend-dev [-- Attachment #1: Type: text/plain, Size: 927 bytes --] On Fri, Sep 23, 2022 at 6:59 AM <soeren@soeren-tempel.net> wrote: > > From: Sören Tempel <soeren@soeren-tempel.net> > > Tested on x86_64 Arch Linux (glibc) and Alpine Linux (musl libc). > > Previously, libgo relied on the _sigev_un implementation-specific > field in struct sigevent, which is only available on glibc. This > patch uses the sigev_notify_thread_id macro instead which is mandated > by timer_create(2). In theory, this should work with any libc > implementation for Linux. Unfortunately, there is an open glibc bug > as glibc does not define this macro. For this reason, a glibc-specific > workaround is required. Other libcs (such as musl) define the macro > and don't require the workaround. > > This makes go_signal compatible with musl libc. > > See: https://sourceware.org/bugzilla/show_bug.cgi?id=27417 Thanks. Committed with some changes, as appended. Sorry for the delay. Ian [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 2699 bytes --] e73d9fcafbd07bc3714fbaf8a82db71d50015c92 diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 73aa712dbdf..4793c821eba 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -0140cca9bc0fad1108c7ed369376ac71cc4bfecf +8f1a91aeff400d572857895b7f5e863ec5a4d93e The first line of this file holds the git revision number of the last merge done from the gofrontend repository. diff --git a/libgo/go/runtime/os_linux.go b/libgo/go/runtime/os_linux.go index 96fb178870e..2b2d827cee8 100644 --- a/libgo/go/runtime/os_linux.go +++ b/libgo/go/runtime/os_linux.go @@ -22,6 +22,12 @@ type mOS struct { profileTimerValid uint32 } +// setSigeventTID is written in C to set the sigev_notify_thread_id +// field of a sigevent struct. +// +//go:noescape +func setSigeventTID(*_sigevent, int32) + func getProcID() uint64 { return uint64(gettid()) } @@ -52,9 +58,12 @@ const ( ) // Atomically, +// // if(*addr == val) sleep +// // Might be woken up spuriously; that's allowed. // Don't sleep longer than ns; ns < 0 means forever. +// //go:nosplit func futexsleep(addr *uint32, val uint32, ns int64) { // Some Linux kernels have a bug where futex of @@ -73,6 +82,7 @@ func futexsleep(addr *uint32, val uint32, ns int64) { } // If any procs are sleeping on addr, wake up at most cnt. +// //go:nosplit func futexwakeup(addr *uint32, cnt uint32) { ret := futex(unsafe.Pointer(addr), _FUTEX_WAKE_PRIVATE, cnt, nil, nil, 0) @@ -365,7 +375,7 @@ func setThreadCPUProfiler(hz int32) { var sevp _sigevent sevp.sigev_notify = _SIGEV_THREAD_ID sevp.sigev_signo = _SIGPROF - *((*int32)(unsafe.Pointer(&sevp._sigev_un))) = int32(mp.procid) + setSigeventTID(&sevp, int32(mp.procid)) ret := timer_create(_CLOCK_THREAD_CPUTIME_ID, &sevp, &timerid) if ret != 0 { // If we cannot create a timer for this M, leave profileTimerValid false diff --git a/libgo/runtime/go-signal.c b/libgo/runtime/go-signal.c index 528d9b6d9fe..aa1b6305ad0 100644 --- a/libgo/runtime/go-signal.c +++ b/libgo/runtime/go-signal.c @@ -183,6 +183,24 @@ setSigactionHandler(struct sigaction* sa, uintptr handler) sa->sa_sigaction = (void*)(handler); } +#ifdef __linux__ + +// Workaround for https://sourceware.org/bugzilla/show_bug.cgi?id=27417 +#ifndef sigev_notify_thread_id + #define sigev_notify_thread_id _sigev_un._tid +#endif + +void setSigeventTID(struct sigevent*, int32_t) + __asm__ (GOSYM_PREFIX "runtime.setSigeventTID"); + +void +setSigeventTID(struct sigevent *sev, int32_t v) +{ + sev->sigev_notify_thread_id = v; +} + +#endif // defined(__linux__) + // C code to fetch values from the siginfo_t and ucontext_t pointers // passed to a signal handler. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-27 16:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <3P4Q20UQY09GV.2ZECSRTOW6FRW@8pit.net> [not found] ` <CAOyqgcVgWaCvV6cTpZF+Q0rSV5DAdsdCTxOd36L0SWoo302L7Q@mail.gmail.com> 2022-08-20 10:31 ` [PATCH] libgo: Access to glibc-specific field in struct sigevent Sören Tempel 2022-09-05 12:31 ` Sören Tempel 2022-09-23 13:59 ` [PATCH v2] libgo: Portable access to thread ID " soeren 2022-09-27 16:31 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).