[BUG] sysroot: Incorrect definition of WIFSTOPPED in <bits/wait.h>

[xtkoba]

Description

Look at the definition of WIFSTOPPED and WIFCONTINUED in <bits/wait.h>:

#define WTERMSIG(__status) ((__status) & 0x7f)
/* ... */
#define WIFSTOPPED(__status) (WTERMSIG(__status) == 0x7f)
/* ... */
#define WIFCONTINUED(__status) ((__status) == 0xffff)

This implies that WIFSTOPPED(__status) is true whenever WIFCONTINUED(__status) is true, leading to that the following pattern does not work as expected:

#include <sys/wait.h>

void bar(void);
void baz(void);

void
foo(int status)
{
    if (WIFSTOPPED(status)) {
        bar();
    } else if (WIFCONTINUED(status)) {
        /* unreachable when built with NDK r25c */
        baz();
    }
}

Unfortunately mksh bundled with Android adopts this pattern and so the job control in mksh is not working correctly:

https://android.googlesource.com/platform/external/mksh/+/refs/tags/android-13.0.0_r42/src/jobs.c#1394
https://android.googlesource.com/platform/external/mksh/+/refs/tags/android-13.0.0_r42/src/jobs.c#1398

This issue was initially reported in termux/termux-packages#15677.

Affected versions

r25

Canary version

No response

Host OS

Linux

Host OS version

Portage 3.0.46 (python 3.11.3-final-0, default/linux/amd64/17.1, gcc-13, glibc-2.37-r2, 5.4.203-gentoo x86_64)

Affected ABIs

armeabi-v7a, arm64-v8a, x86, x86_64

Build system

Other (specify below)

Other build system

No response

minSdkVersion

24

Device API level

31

[DanAlbert]

@enh-google is this something you think you'll have time to fix this week? If so I can include it in r26 beta 1.

[enh-google]

yeah, i've just uploaded https://android-review.googlesource.com/c/platform/bionic/+/2575250 with the obvious fix:

-#define WIFSTOPPED(__status) (WTERMSIG(__status) == 0x7f)
+#define WIFSTOPPED(__status) (((__status) & 0xff) == 0x7f)

i was worried that i'd introduced this bug during a refactoring, but it's been there since the initial import. it looks like this actually is correct for BSD, so that's probably where it came from.

[xtkoba]

Presumably this would not be an issue if WIFCONTINUED were not supported ever.

In OpenBSD code base, the definition of WIFSTOPPED was changed at the same time when the definition of WIFCONTINUED was added: openbsd/src@26a45e4.

In bionic WIFCONTINUED was added in https://android.googlesource.com/platform/bionic/+/dc06c0562361c35292da8be7b1f627e5e25b1c5c. The issue has been there since then.

[enh-google]

ah, that explains how we got here. thanks!

[enh-google]

(and thanks for the super detailed and actionable bug report!)

[DanAlbert]

It looks like the fix for this was actually in r26 beta 1 and we just missed it in the changelog. Uploaded https://android-review.googlesource.com/c/platform/ndk/+/2697495 to fix that.

(if my git spelunking is wrong and it wasn't in beta 1, it will be in rc 1, so closing either way)