Repository navigation
[BUG] sysroot: Incorrect definition of WIFSTOPPED in <bits/wait.h> #1878
Description
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
Activity
DanAlbert commentedon May 1, 2023
@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 commentedon May 1, 2023
yeah, i've just uploaded https://android-review.googlesource.com/c/platform/bionic/+/2575250 with the obvious fix:
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 commentedon May 2, 2023
Presumably this would not be an issue if
WIFCONTINUEDwere not supported ever.In OpenBSD code base, the definition of
WIFSTOPPEDwas changed at the same time when the definition ofWIFCONTINUEDwas added: openbsd/src@26a45e4.In bionic
WIFCONTINUEDwas added in https://android.googlesource.com/platform/bionic/+/dc06c0562361c35292da8be7b1f627e5e25b1c5c. The issue has been there since then.enh-google commentedon May 2, 2023
ah, that explains how we got here. thanks!
enh-google commentedon May 2, 2023
(and thanks for the super detailed and actionable bug report!)
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
Fix the WIFSTOPPED definition.
DanAlbert commentedon Aug 7, 2023
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)