-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CommandLine][Linux] Don't read argv from /proc/self/cmdline. #71611
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
Conversation
2b886b1 to
1da4132
Compare
|
Can we just save For the record, Rust takes this saving-at-entry approach for some platforms including Linux. (but for Linux + glibc, they use the fact that glibc passes argv/argc/envp to |
The problem with that is that the command line would only be available after Ideally, Swift should behave the same way everywhere. |
Ah, I see... Then it makes sense to depend on initial stack layout as libc-independent approach on Linux. |
Yes. I'd be in favour of the save-at-entry approach if I was doing something like this from scratch, I think, because that's completely portable, whereas with this we're dependent on platform specifics. As for the |
…ine. Instead of reading from `/proc/self/cmdline`, take advantage of the fact that the initial stack layout is ABI specified, and that we already have a pointer into it (`environ`). This lets us walk up the stack until we find `argc`, at which point we also know where `argv` is. We do this from a static initializer because a `setenv()` or `putenv()` can change `environ` (if you add a new environment variable), and it's even permissible to just outright change `environ` yourself too. It seems reasonable to suggest to people that they shouldn't be doing those things from a static initializer, and as long as they don't, they won't run before we've had a chance to find `argv`. Just in case someone _does_ do this, we also check that `environ` points into the stack. If it doesn't, they won't get any arguments, so if that happens, that's a clue that they're messing with `environ` too early. This works around a problem (swiftlang#69658) with Docker Desktop 4.25.0 and Rosetta, wherein we end up with an extra argument visible in `/proc/self/cmdline`, and also avoids allocating memory for the command line arguments. rdar://117963394
1da4132 to
04c496c
Compare
|
@swift-ci Please smoke test Linux platform |
1 similar comment
|
@swift-ci Please smoke test Linux platform |
|
@swift-ci Please smoke test Windows platform |
|
@swift-ci Please smoke test macOS platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awful (laudatory)
This reverts the reversion of swiftlang#71611. rdar://117963394
Instead of reading from
/proc/self/cmdline, take advantage of the fact that the initial stack layout is ABI specified, and that we already have a pointer into it (environ). This lets us walk up the stack until we findargc, at which point we also know whereargvis.We do this from a static initializer because a
setenv()orputenv()can changeenviron(if you add a new environment variable), and it's even permissible to just outright changeenvironyourself too. It seems reasonable to suggest to people that they shouldn't be doing those things from a static initializer, and as long as they don't, they won't run before we've had a chance to findargv.Just in case someone does do this, we also check that
environpoints into the stack. If it doesn't, they won't get any arguments, so if that happens, that's a clue that they're messing withenvirontoo early.This works around a problem (#69658) with Docker Desktop 4.25.0 and Rosetta, wherein we end up with an extra argument visible in
/proc/self/cmdline, and also avoids allocating memory for the command line arguments.rdar://117963394