[Stdlib] Fix entry-point-based process args #3108
Conversation
CodaFi
changed the title
| size_t size = 0; | ||
| std::vector<char *> argvec; | ||
| while(getdelim(&arg, &size, 0, cmdline) != -1) { | ||
| argvec.push_back(arg); |
There was a problem hiding this comment.
I think you need to reset arg and size here.
There was a problem hiding this comment.
Both of those are purely out parameters and I can't find any precedence for resetting them.
There was a problem hiding this comment.
From the man page:
Alternatively, before calling
getline(),*lineptrcan contain a pointer to amalloc(3)-allocated buffer*nbytes in size.
That is, subsequent calls would overwrite what the first call allocated.
CodaFi
force-pushed
the
argumental
branch
2 times, most recently
from
a2d3ca1 to
b81002b
Compare
| FILE *cmdline = fopen("/proc/self/cmdline", "rb"); | ||
| if (!cmdline) { | ||
| swift::fatalError(0, | ||
| "fatal error: Unable to open command line interface.\n"); |
There was a problem hiding this comment.
I think using /proc/self/cmdline in the error will help a lot of people who run Swift in restricted environments. Seeing a message about /proc makes it immediately obvious what's wrong (/proc not available, or not mounted etc.)
|
|
||
| let newArgs = _unsafeArgv ?? _swift_stdlib_getUnsafeArgcArgv(&_argc) | ||
| if _stdlib_atomicCompareExchangeStrongPtr( | ||
| object: argumentsPtr, expected: &_swift_stdlib_ProcessUnsafeArgv, desired: newArgs) { |
There was a problem hiding this comment.
Expected should be nil. We are initializing _swift_stdlib_ProcessUnsafeArgv from nil to newArgs.
There was a problem hiding this comment.
Yep. Accidentally committed while in the middle of refactoring.
There was a problem hiding this comment.
I feel like we should be able to get rid of _Box and the ARC mess altogether, by doing something like this:
static var unsafeArgv = _unsafeArgvFromMain ?? _unsafeArgvFromRuntime
static var _unsafeArgvFromRuntime = _swift_stdlib_getUnsafeArgcArgv(&_argc)I guess that's not 100% thread-safe because _unsafeArgvFromMain could be set from main while being read on another thread spawned prior to entering main, but something like this could work, right?
There was a problem hiding this comment.
There's not ARC or boxes now, everything is just pointers.
|
I think we should also stop using main to set argc/argv except in interpreter mode, but we can do that in a follow-up commit. |
CodaFi
force-pushed
the
argumental
branch
3 times, most recently
from
d5e60bd to
c0cbd40
Compare
| } | ||
|
|
||
| @_versioned | ||
| /// The backing static variable for argument count may come either from the | ||
| /// entry point or it may need to be computed e.g. if we're in the REPL. |
There was a problem hiding this comment.
The doc comment should be above the attribute.
|
I spoke with @trentxintong earlier today and he does not believe that the old static initializer problems will manifest themselves here. I've folded everything into static lets. |
CodaFi
force-pushed
the
argumental
branch
3 times, most recently
from
f0ad629 to
4314cd0
Compare
|
@swift-ci please test. |
| @@ -34,7 +34,9 @@ namespace swift { | |||
|
|
|||
| namespace immediate { | |||
|
|
|||
| bool loadSwiftRuntime(StringRef runtimeLibPath); | |||
| // Returns a handle to the runtime suitable for other 'dlsym' or 'dlclose' | |||
There was a problem hiding this comment.
Nitpick: please make this a proper doc comment, so that it shows up in Quick Help.
There was a problem hiding this comment.
I'll get it when I remove the compiler intrinsic.
|
Happy to be getting this nailed down! |
|
@swift-ci please test. |
|
The failures look unrelated, but I want a clean merge. |
CodaFi
force-pushed
the
argumental
branch
8 times, most recently
from
d18a092 to
b31b630
Compare
Provides a new fallback for Process arguments for those instances where we do not own main (e.g. Frameworks, Objective-C owns main.m or main.c, etc.). This includes a number of platform-specific specializations of argument grabbing logic and a new thread-safe interface to Process.unsafeArgv. main() | _NSGetArgc/_NSGetArgv | /proc/self/cmdline | __argc/__argv --------|--------------------------|------------------------|--------------- Scripts | OS X, iOS, tvOS, watchOS | Linux, FreeBSD, Cygwin | Windows For interpreted Swift where we must filter out the arguments we now do so by loading the standard library and calling into new SPI to override the arguments that would have been grabbed by the runtime. This implementation completely subsumes the use of the entry point '_stdlib_didEnterMain' and it will be removed in a future commit.
|
@swift-ci please test and merge. |
|
Finally! The build failures aren't my fault. |
What's in this pull request?
Provides a new fallback for
Processarguments for those instances where we do not ownmain(e.g. Frameworks, Objective-C ownsmain.mormain.c, etc.). This includes a number of platform-specific specializations of a new runtime function_swift_get_argsand a new thread-safe interface toProcess.unsafeArgv.Resolved bug number: (SR-1119)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.