Skip to content

[libc][wait] hard code __W_CONTINUED for SYS_waitid fallback #125929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

nickdesaulniers
Copy link
Member

riscv32 currently doesn't have SYS_wait4, so wait4 is implemented via fallback
to SYS_waitid. In #125572, I missed that we had one use of the removed
__W_CONTINUED value. Hard code it here.

Fixes: #125572

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
riscv32 currently doesn't have SYS_wait4, so wait4 is implemented via fallback
to SYS_waitid. In llvm#125572, I missed that we had one use of the removed
__W_CONTINUED value. Hard code it here.

Fixes: llvm#125572
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

riscv32 currently doesn't have SYS_wait4, so wait4 is implemented via fallback
to SYS_waitid. In #125572, I missed that we had one use of the removed
__W_CONTINUED value. Hard code it here.

Fixes: #125572


Full diff: https://github.com/llvm/llvm-project/pull/125929.diff

1 Files Affected:

  • (modified) libc/src/sys/wait/wait4Impl.h (+1-1)
diff --git a/libc/src/sys/wait/wait4Impl.h b/libc/src/sys/wait/wait4Impl.h
index 5c2cb3e8e0abf4..ec84eee491de24 100644
--- a/libc/src/sys/wait/wait4Impl.h
+++ b/libc/src/sys/wait/wait4Impl.h
@@ -65,7 +65,7 @@ LIBC_INLINE ErrorOr<pid_t> wait4impl(pid_t pid, int *wait_status, int options,
       *wait_status = W_STOPCODE(info.si_status);
       break;
     case CLD_CONTINUED:
-      *wait_status = __W_CONTINUED;
+      *wait_status = 0xffff;
       break;
     default:
       *wait_status = 0;

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but we should look into adding some macro to define this value as a followup. It looks like the same value is used in WIFCONTINUED

@nickdesaulniers
Copy link
Member Author

LGTM, but we should look into adding some macro to define this value as a followup. It looks like the same value is used in WIFCONTINUED

I think end users are supposed to use WIFCONTINUED to check if what they got back from wait4 was a "continued status." i.e. the whole point is to hide the actual value and force users to check that. That said, that forces users to use a conditional chain, and can't do a switch statement.

@nickdesaulniers
Copy link
Member Author

@SchrodingerZhu seems like the windows presubmit bot is failing to install sccache.

https://github.com/llvm/llvm-project/actions/runs/13166422042/job/36747558716?pr=125929

curl: (35) schannel: next InitializeSecurityContext failed: CRYPT_E_REVOCATION_OFFLINE (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline.

Guessing that's referring to SSL cert revocation lists; sounds like someone is having an outage somewhere today!

@nickdesaulniers nickdesaulniers merged commit b4d547a into llvm:main Feb 5, 2025
12 of 15 checks passed
@nickdesaulniers nickdesaulniers deleted the wait_riscv32 branch February 5, 2025 21:10
@@ -65,7 +65,7 @@ LIBC_INLINE ErrorOr<pid_t> wait4impl(pid_t pid, int *wait_status, int options,
*wait_status = W_STOPCODE(info.si_status);
break;
case CLD_CONTINUED:
*wait_status = __W_CONTINUED;
*wait_status = 0xffff;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some comments about the hardcoded value and why we don't use the macro?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, sorry, mid air collision. I'll post another patch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…5929)

riscv32 currently doesn't have SYS_wait4, so wait4 is implemented via fallback
to SYS_waitid. In llvm#125572, I missed that we had one use of the removed
__W_CONTINUED value. Hard code it here.

Fixes: llvm#125572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants