[PATCH 1/3] libc: add strerrordesc_np

39 views
Skip to first unread message

Waldemar Kozaczuk

unread,
May 25, 2021, 1:03:01 AM5/25/21
to osv...@googlegroups.com, Waldemar Kozaczuk
Per the specification, strerrordesc_np() is supposed to
return a pointer to a string that describes the error code
passed in the argument errnum just like strerror() does.
However unlike strerror(), strerrordesc_np() is NOT supposed
to apply any translation per current locale.

But the OSv implementation of strerror() does not apply any
translation anyway (please see the LCTRANS macro in locale_impl.h
that returns the msg "as is" without any translation). So given
that we simply make strerrordesc_np an alias of strerror.

The strerrordesc_np() is needed by newer version of nginx.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
libc/aliases.ld | 1 +
1 file changed, 1 insertion(+)

diff --git a/libc/aliases.ld b/libc/aliases.ld
index 3db1dec6..14dac146 100644
--- a/libc/aliases.ld
+++ b/libc/aliases.ld
@@ -48,6 +48,7 @@ __strndup = strndup;
__stpcpy = stpcpy;
__stpncpy = stpncpy;
__strdup = strdup;
+strerrordesc_np = strerror;

/* temp */
__mktemp = mktemp;
--
2.30.2

Waldemar Kozaczuk

unread,
May 25, 2021, 1:03:05 AM5/25/21
to osv...@googlegroups.com, Waldemar Kozaczuk
The golang applications use the tgkill syscall to implement the
raise() function as the issue #1047 describes. The raise() function
is then used to propagate SIGTERM signal to the process when
Ctrl-C is pressed.

For that reason this patch adds very basic implementation
of the tgkill syscall. More specifically it only handles the case
where tpid specifies current process or -1 and tid specifies the current
thread of the caller which in essence is what Golang raise() passes.
In this case the tgkill syscall implementation delegates to kill()
otherwise it returns failure.

This patch also modifies the implementation of the pthread_kill()
to make it consistent with the implementation of the tgkill syscall.
The pthread_kill is actually called by raise() (see libc/pthread.cc)
so just like with tgkill, we check if specified pthread_t is equal to
the current thread and in such case we delegate to kill().

Lastly this patch enhances tst-kill.cc to test raise() and
pthread_kill().

Refs #1047

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
libc/pthread.cc | 13 ++++++++++++-
linux.cc | 17 +++++++++++++++++
tests/tst-kill.cc | 5 +++++
3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/libc/pthread.cc b/libc/pthread.cc
index a9a31eaf..4e44082c 100644
--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -995,8 +995,19 @@ int pthread_getschedparam(pthread_t thread, int *policy,

int pthread_kill(pthread_t thread, int sig)
{
- WARN_STUBBED();
+ // We are assuming that if pthread_kill() is called with thread
+ // equal to pthread_self(), then most likely it was called by
+ // raise() (see below) so we simply delegate to kill().
+ // This an approximation as it reality multithreaded apps
+ // may actually send signal to the current thread by directly
+ // calling pthread_kill() for a reason and then thread specific mask
+ // would apply, etc. But OSv does not really support sending signals to
+ // specific threads so we are silently ignoring such case for now.
+ if (thread == current_pthread) {
+ return kill(getpid(), sig);
+ }

+ WARN_STUBBED();
return EINVAL;
}

diff --git a/linux.cc b/linux.cc
index 20da2146..522ed8ba 100644
--- a/linux.cc
+++ b/linux.cc
@@ -371,6 +371,22 @@ static int pselect6(int nfds, fd_set *readfds, fd_set *writefds,
return pselect(nfds, readfds, writefds, exceptfds, timeout_ts, NULL);
}

+static int tgkill(int tgid, int tid, int sig)
+{
+ //
+ // Given OSv supports sigle process only, we only support this syscall
+ // when thread group id is self (getpid()) or -1 (see https://linux.die.net/man/2/tgkill)
+ // AND tid points to the current thread (caller)
+ // Ideally we would want to delegate to pthread_kill() but there is no
+ // easy way to map tgid to pthread_t so we directly delegate to kill().
+ if ((tgid == -1 || tgid == getpid()) && (tid == gettid())) {
+ return kill(tgid, sig);
+ }
+
+ errno = ENOSYS;
+ return -1;
+}
+
long syscall(long number, ...)
{
// Save FPU state and restore it at the end of this function
@@ -451,6 +467,7 @@ long syscall(long number, ...)
SYSCALL2(mkdir, char*, mode_t);
#endif
SYSCALL3(mkdirat, int, char*, mode_t);
+ SYSCALL3(tgkill, int, int, int);
}

debug_always("syscall(): unimplemented system call %d\n", number);
diff --git a/tests/tst-kill.cc b/tests/tst-kill.cc
index 66d7708a..2c226fa4 100644
--- a/tests/tst-kill.cc
+++ b/tests/tst-kill.cc
@@ -13,6 +13,7 @@
#include <stdio.h>
#include <unistd.h>
#include <errno.h>
+#include <pthread.h>

int tests = 0, fails = 0;

@@ -98,6 +99,10 @@ int main(int ac, char** av)
r = kill(17171717, 0);
report(r == -1 && errno == ESRCH, "kill of non-existant process");

+ report(raise(0) == 0, "raise() should succeed");
+ report(pthread_kill(pthread_self(), 0) == 0, "pthread_kill() should succeed with current thread");
+ report(pthread_kill((pthread_t)(-1), 0) == EINVAL, "pthread_kill() should fail for thread different than current one");
+
// Test alarm();
global = 0;
sr = signal(SIGALRM, handler1);
--
2.30.2

Waldemar Kozaczuk

unread,
May 25, 2021, 1:03:06 AM5/25/21
to osv...@googlegroups.com, Waldemar Kozaczuk
Some applications like keydb that use jmalloc, call sbrk()
on startup even though later fall back to mmap().

So this patch provides simple stubs of those 2 functions
that set errno to ENOMEM and return error.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
libc/mman.cc | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/libc/mman.cc b/libc/mman.cc
index add8ef15..895b9b28 100644
--- a/libc/mman.cc
+++ b/libc/mman.cc
@@ -13,6 +13,7 @@
#include "osv/trace.hh"
#include "osv/dentry.h"
#include "osv/mount.h"
+#include <osv/stubbing.hh>
#include "libc/libc.hh"
#include <safe-ptr.hh>

@@ -234,3 +235,17 @@ int madvise(void *addr, size_t length, int advice)
auto err = mmu::advise(addr, length, libc_madvise_to_advise(advice));
return err.to_libc();
}
+
+int brk(void *addr)
+{
+ WARN_STUBBED();
+ errno = ENOMEM;
+ return -1;
+}
+
+void *sbrk(intptr_t increment)
+{
+ WARN_STUBBED();
+ errno = ENOMEM;
+ return (void *)-1;
+}
--
2.30.2

Waldek Kozaczuk

unread,
May 25, 2021, 1:05:52 AM5/25/21
to OSv Development
These last 3 patches are really unrelated - I just generated and sent them with one format-patch command. My mistake.

Nadav Har'El

unread,
May 25, 2021, 4:02:30 AM5/25/21
to Waldemar Kozaczuk, Osv Dev
On Tue, May 25, 2021 at 8:03 AM Waldemar Kozaczuk <jwkoz...@gmail.com> wrote:
Per the specification, strerrordesc_np() is supposed to
return a pointer to a string that describes the error code
passed in the argument errnum just like strerror() does.
However unlike strerror(), strerrordesc_np() is NOT supposed
to apply any translation per current locale.

Interesting - this is a very new glibc feature - https://sourceware.org/pipermail/glibc-cvs/2020q3/070143.html
Sad they gave it such a silly name ("np").

Note that another difference between strerrordesc_np() and strerror() is that the latter is not guaranteed thread safe (a call to strerror() may modify the string returned by a previous call), the former is.
In our implementation, strerror() is already thread-safe, so they can indeed be the same.



But the OSv implementation of strerror() does not apply any
translation anyway (please see the LCTRANS macro in locale_impl.h
that returns the msg "as is" without any translation). So given
that we simply make strerrordesc_np an alias of strerror.

The strerrordesc_np() is needed by newer version of nginx.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
 libc/aliases.ld | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libc/aliases.ld b/libc/aliases.ld
index 3db1dec6..14dac146 100644
--- a/libc/aliases.ld
+++ b/libc/aliases.ld
@@ -48,6 +48,7 @@ __strndup = strndup;
 __stpcpy = stpcpy;
 __stpncpy = stpncpy;
 __strdup = strdup;
+strerrordesc_np = strerror;

 /* temp */
 __mktemp = mktemp;
--
2.30.2

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20210525050253.1141211-1-jwkozaczuk%40gmail.com.

Commit Bot

unread,
May 25, 2021, 4:03:13 AM5/25/21
to osv...@googlegroups.com, Waldemar Kozaczuk
From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

libc: add strerrordesc_np

Per the specification, strerrordesc_np() is supposed to
return a pointer to a string that describes the error code
passed in the argument errnum just like strerror() does.
However unlike strerror(), strerrordesc_np() is NOT supposed
to apply any translation per current locale.

But the OSv implementation of strerror() does not apply any
translation anyway (please see the LCTRANS macro in locale_impl.h
that returns the msg "as is" without any translation). So given
that we simply make strerrordesc_np an alias of strerror.

The strerrordesc_np() is needed by newer version of nginx.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
Message-Id: <20210525050253.11...@gmail.com>

---
diff --git a/libc/aliases.ld b/libc/aliases.ld

Commit Bot

unread,
May 25, 2021, 4:04:47 AM5/25/21
to osv...@googlegroups.com, Waldemar Kozaczuk
From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

syscalls: minimally implement tgkill

The golang applications use the tgkill syscall to implement the
raise() function as the issue #1047 describes. The raise() function
is then used to propagate SIGTERM signal to the process when
Ctrl-C is pressed.

For that reason this patch adds very basic implementation
of the tgkill syscall. More specifically it only handles the case
where tpid specifies current process or -1 and tid specifies the current
thread of the caller which in essence is what Golang raise() passes.
In this case the tgkill syscall implementation delegates to kill()
otherwise it returns failure.

This patch also modifies the implementation of the pthread_kill()
to make it consistent with the implementation of the tgkill syscall.
The pthread_kill is actually called by raise() (see libc/pthread.cc)
so just like with tgkill, we check if specified pthread_t is equal to
the current thread and in such case we delegate to kill().

Lastly this patch enhances tst-kill.cc to test raise() and
pthread_kill().

Refs #1047

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
Message-Id: <20210525050253.11...@gmail.com>

---
diff --git a/libc/pthread.cc b/libc/pthread.cc
--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -995,8 +995,19 @@ int pthread_getschedparam(pthread_t thread, int *policy,

int pthread_kill(pthread_t thread, int sig)
{
- WARN_STUBBED();
+ // We are assuming that if pthread_kill() is called with thread
+ // equal to pthread_self(), then most likely it was called by
+ // raise() (see below) so we simply delegate to kill().
+ // This an approximation as it reality multithreaded apps
+ // may actually send signal to the current thread by directly
+ // calling pthread_kill() for a reason and then thread specific mask
+ // would apply, etc. But OSv does not really support sending signals to
+ // specific threads so we are silently ignoring such case for now.
+ if (thread == current_pthread) {
+ return kill(getpid(), sig);
+ }

+ WARN_STUBBED();
return EINVAL;
}

diff --git a/linux.cc b/linux.cc

Commit Bot

unread,
May 25, 2021, 4:09:29 AM5/25/21
to osv...@googlegroups.com, Waldemar Kozaczuk
From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

libc: add brk()/sbrk() stubs

Some applications like keydb that use jmalloc, call sbrk()
on startup even though later fall back to mmap().

So this patch provides simple stubs of those 2 functions
that set errno to ENOMEM and return error.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
Message-Id: <20210525050253.11...@gmail.com>

---
diff --git a/libc/mman.cc b/libc/mman.cc
Reply all
Reply to author
Forward
0 new messages