SafeStack: Fix thread liveness check on *BSD

[vlad.tsyrklevich]

Summary

The Linux/BSD system call interfaces report errors differently, use the
internal_iserror() function to correctly check errors on either.

[eugenis]

LGTM

[krytarowski]

I'm going to test NetBSD soon.

[krytarowski]

[100%] Running the SafeStack tests
-- Testing: 9 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 0.61s
  Expected Passes    : 8
  Unsupported Tests  : 1
[100%] Built target check-safestack

$ uname -a
NetBSD chieftec 8.99.24 NetBSD 8.99.24 (GENERIC) #2: Thu Aug 16 13:44:34 CEST 2018  root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64

[vlad.tsyrklevich]

In D51368#1216228, @krytarowski wrote:

[100%] Running the SafeStack tests
-- Testing: 9 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 0.61s
  Expected Passes    : 8
  Unsupported Tests  : 1
[100%] Built target check-safestack

$ uname -a
NetBSD chieftec 8.99.24 NetBSD 8.99.24 (GENERIC) #2: Thu Aug 16 13:44:34 CEST 2018  root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64

I don't have time to spin up a NetBSD VM to look at this; however, this failure is most likely related to this TgKill() call. If you can print the TgKill/errno return values that could indicate what's happening.

[krytarowski]

NetBSD is fixed, as I was observing previously:

$ check-safestack

********************
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 0.56s
********************
Failing Tests (1):
    SafeStack :: pthread-cleanup.c

  Expected Passes    : 7
  Unsupported Tests  : 1
  Unexpected Failures: 1

The failure is gone with this patch.

[vlad.tsyrklevich]

In D51368#1216275, @krytarowski wrote:

NetBSD is fixed, as I was observing previously:

$ check-safestack

********************
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 0.56s
********************
Failing Tests (1):
    SafeStack :: pthread-cleanup.c

  Expected Passes    : 7
  Unsupported Tests  : 1
  Unexpected Failures: 1

The failure is gone with this patch.

Doh, skimmed too quickly and read Unsupported Tests as failures.

lib/safestack/safestack.cc

static void thread_cleanup_handler(void *_iter) {
 
   pid_t pid = getpid();
   tid_t tid = GetTid();
 
   // Free stacks for dead threads
   thread_stack_ll **stackp = &temp_stacks;
   while (*stackp) {
     thread_stack_ll *stack = *stackp;
-    if (stack->pid != pid || TgKill(stack->pid, stack->tid, 0) == -ESRCH) {
+    int error;
+    if (stack->pid != pid ||
+        (internal_iserror(TgKill(stack->pid, stack->tid, 0), &error) &&
+         error == ESRCH)) {
       UnmapOrDie(stack->stack_base, stack->size);
       *stackp = stack->next;
       free(stack);
     } else
       stackp = &stack->next;
   }
 
   thread_stack_ll *cur_stack =