-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[K/N] Expose program name in runtime #5281
base: master
Are you sure you want to change the base?
Changes from 7 commits
1ff362a
05d57ee
f73045e
fb67427
2703f15
3b66b6b
285dd67
69e660f
a69b027
6d07cf2
2fcf1f0
3c933a7
802c8e7
64ffcfc
2d44f5f
06b0d67
27d7688
fba8ce4
0aa4600
7fb104f
7868f85
6323590
e39351d
fd2ebbf
d09c9f0
841f6e0
ee0ed60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
|
||
#include "Porting.h" | ||
#include "Memory.h" | ||
#include "KString.h" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems unused. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed. |
||
|
||
#ifdef __cplusplus | ||
extern "C" { | ||
|
@@ -50,6 +51,8 @@ namespace kotlin { | |
// Returns `true` if initialized. | ||
bool initializeGlobalRuntimeIfNeeded() noexcept; | ||
|
||
extern const char* programName; | ||
|
||
} | ||
|
||
#endif // RUNTIME_RUNTIME_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,6 +101,13 @@ public object Platform { | |
public val isFreezingEnabled: Boolean | ||
get() = Platform_isFreezingEnabled() | ||
|
||
/** | ||
* Representation of the name used to invoke the program executable. | ||
* [null] if the Kotlin code was compiled to a native library and the executable is not a Kotlin program. | ||
*/ | ||
public val programName: String? | ||
get() = Platform_getProgramName() | ||
SvyatoslavScherbina marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @qurbonzoda could you please review the stdlib change? |
||
|
||
/** | ||
* If the memory leak checker is activated, by default `true` in debug mode, `false` in release. | ||
* When memory leak checker is activated, and leak is detected during last Kotlin context | ||
|
@@ -161,6 +168,9 @@ private external fun Platform_isDebugBinary(): Boolean | |
@GCUnsafeCall("Konan_Platform_isFreezingEnabled") | ||
private external fun Platform_isFreezingEnabled(): Boolean | ||
|
||
@GCUnsafeCall("Konan_Platform_getProgramName") | ||
private external fun Platform_getProgramName(): String? | ||
|
||
@GCUnsafeCall("Konan_Platform_getMemoryLeakChecker") | ||
private external fun Platform_getMemoryLeakChecker(): Boolean | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,7 @@ val stdlibK2Test = nativeTest("stdlibK2Test", "stdlib & frontend-fir") | |
val kotlinTestLibraryTest = nativeTest("kotlinTestLibraryTest", "kotlin-test & !frontend-fir") | ||
val kotlinTestLibraryK2Test = nativeTest("kotlinTestLibraryK2Test", "kotlin-test & frontend-fir") | ||
val partialLinkageTest = nativeTest("partialLinkageTest", "partial-linkage") | ||
val programNameTest = nativeTest("programNameTest", "program-name") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems a bit redudant. I mean, running these tests separately shouldn't happen frequently, so adding a Gradle task is overkill. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I've removed it. |
||
val cinteropTest = nativeTest("cinteropTest", "cinterop") | ||
val debuggerTest = nativeTest("debuggerTest", "debugger") | ||
val cachesTest = nativeTest("cachesTest", "caches") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
#include <stdio.h> | ||
#include "programName_api.h" | ||
|
||
int main() { | ||
programName(); | ||
fflush(NULL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add an explicit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, done |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Platform.programName is null within library |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
@file:OptIn(kotlin.experimental.ExperimentalNativeApi::class) | ||
|
||
@CName("programName") | ||
fun programName() { | ||
println("Platform.programName is " + Platform.programName + " within library") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
@file:OptIn(kotlin.experimental.ExperimentalNativeApi::class) | ||
|
||
import kotlin.native.Platform | ||
|
||
fun main(args: Array<String>) { | ||
// Remove path and extension (.kexe or .exe) | ||
val programFileName = Platform.programName?.substringAfterLast("/")?.substringBeforeLast(".") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test fails on Windows, because it uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, thank you. The sanitization is now platform agnostic. |
||
|
||
println("programName: $programFileName") | ||
println("args: ${args.joinToString()}") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
#include <stdio.h> | ||
#include <unistd.h> | ||
|
||
int main(int argc, char *argv[]) { | ||
printf("calling exec...\n"); | ||
fflush(NULL); | ||
|
||
// Kotlin executable name is in argv[1] | ||
// Forward argv[2..n] to kotlin executable as arguments (the first one should be the programName according to posix) | ||
execv(argv[1], &(argv[2])); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
printf("exec failed\n"); | ||
return 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's return non-zero exit code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I also now print the |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
@file:OptIn(kotlin.experimental.ExperimentalNativeApi::class) | ||
|
||
import kotlin.native.Platform | ||
import kotlin.test.* | ||
|
||
fun main(args: Array<String>) { | ||
// Remove path and extension (.kexe or .exe) | ||
val programFileName = Platform.programName!!.substringAfterLast("/").substringBeforeLast(".") | ||
|
||
assertEquals("standalone_entryPoint_programName", programFileName) | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,66 @@ | ||||
package org.jetbrains.kotlin.konan.test.blackbox | ||||
|
||||
import org.jetbrains.kotlin.konan.test.blackbox.support.* | ||||
import org.jetbrains.kotlin.konan.test.blackbox.support.compilation.TestCompilationResult.Companion.assertSuccess | ||||
import org.jetbrains.kotlin.konan.test.blackbox.support.util.ClangDistribution | ||||
import org.jetbrains.kotlin.konan.test.blackbox.support.util.compileWithClang | ||||
import org.jetbrains.kotlin.native.executors.runProcess | ||||
import org.junit.jupiter.api.Tag | ||||
import org.junit.jupiter.api.Test | ||||
import java.io.File | ||||
import kotlin.test.assertEquals | ||||
import kotlin.time.Duration.Companion.seconds | ||||
|
||||
@Tag("program-name") | ||||
class ProgramNameTest : AbstractNativeSimpleTest() { | ||||
|
||||
@Test | ||||
fun programNameTest() { | ||||
// 1. Compile main.c to main.cexe | ||||
|
||||
val cExecutable = buildDir.resolve("main.cexe") | ||||
compileWithClang( | ||||
clangDistribution = ClangDistribution.Llvm, | ||||
sourceFiles = listOf(sourceDir.resolve("main.c")), | ||||
outputFile = cExecutable, | ||||
additionalClangFlags = listOf("-Wall", "-Werror"), | ||||
).assertSuccess() | ||||
|
||||
// 2. Compile kotlinPrintEntryPoint.kt to kotlinPrintEntryPoint.kexe | ||||
|
||||
val kotlinCompilation = compileToExecutableInOneStage( | ||||
generateTestCaseWithSingleFile( | ||||
sourceFile = sourceDir.resolve("kotlinPrintEntryPoint.kt"), | ||||
testKind = TestKind.STANDALONE_NO_TR, | ||||
extras = TestCase.NoTestRunnerExtras("main") | ||||
) | ||||
).assertSuccess() | ||||
|
||||
// 3. run main.cexe (with different parameters) to call kotlin executable | ||||
|
||||
fun validate(expected: String, vararg args: String) { | ||||
val binaryName = kotlinCompilation.resultingArtifact.executableFile.path | ||||
val result = runProcess(cExecutable.absolutePath, binaryName, *args) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fails if the test target is different from the host. You can reproduce this by running this test with Please use a proper executor through Line 18 in 8c69667
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for this insight. I changed it accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test fails on linuxArm64 target with:
To reproduce, you can run the test with Our test infrastructure runs tests on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could reproduce the problem by running the test via
I see now some ways to approach the problem: Do you have any other ideas? If not, are you fine with approach A? -- |
||||
timeout = 60.seconds | ||||
} | ||||
assertEquals("calling exec...\n$expected", result.stdout) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect this code should be fixed to handle There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, thank you again! The assertion is now platform agnostic due to additional sanitization. |
||||
assertEquals("", result.stderr) | ||||
} | ||||
|
||||
// kotlinPrintEntryPoint removes .kexe | ||||
validate("programName: app\nargs:", "app.kexe") | ||||
|
||||
// Simulate a custom program name, see e.g. https://busybox.net/downloads/BusyBox.html#usage | ||||
validate("programName: customProgramName\nargs:", "customProgramName") | ||||
validate("programName: customProgramName\nargs: firstArg, secondArg", "customProgramName", "firstArg", "secondArg") | ||||
|
||||
// No program name - this would not be POSIX compliant, see https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html: | ||||
// "[...] requires a Strictly Conforming POSIX Application to pass at least one argument to the exec function" | ||||
// However, we should not crash the Kotlin runtime because of this. | ||||
validate("programName: null\nargs:") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Linux, this fails for me with
Please take a look. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very interesting. Linux behaves here differently as windows & macOS. I fixed the problem, and added more explanation in the code. I used the following C standard PDF: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf. Please tell me if you would want to let windows+macOS behave like linux. The way I wrote it now feels more natural to me, however I fully understand that all 3 options have its benefits and drawbacks: A) The way I wrote it now: all 3 platforms behave the same; no program name leads to |
||||
} | ||||
|
||||
companion object { | ||||
private val sourceDir = File("native/native.tests/testData/programName") | ||||
} | ||||
} |
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.
How about doing
kotlin::programName = strndup(argv[0], 4096)
? No need tofree
the result, and we don't have to worry about lifetimes.4096
is just some random not-too-small not-too-big number to protect us fromargv[0]
being too large. Technically, the OS has some limits already, but might as well protect ourselves.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.
Ok, I applied your idea.