From dc8186cb830165eb8facc752f42f8da6cb946d89 Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Fri, 25 Jun 2021 20:01:53 +0700 Subject: [PATCH] [K/N][Interop] Fix throwing exceptions through bridges in opt mode By default, C functions compiled to bitcode by clang have the nounwind attribute. If such functions throws an exception, the behaviour is undefined. Our interop machinery can process foreign exceptions on call sites (terminate or wrap them in Kotlin exceptions). But if the interop bridges have the nounwind attribute, LLVM optimizations (particularly inlining) may lead to the situation when a foreign exception is ignored by our foreign exception handler. This patch fixes the issue by compiling bridges with -fexceptions flag. This flag makes clang to not set the nounwind attribute, so exceptions can be thrown through C frames. --- .../kotlin/native/interop/gen/jvm/main.kt | 2 ++ .../kotlin/backend/konan/CStubsManager.kt | 5 +++-- kotlin-native/backend.native/tests/build.gradle | 17 +++++++++++++++-- .../interop/exceptions/throwThroughBridge.cpp | 11 +++++++++++ .../interop/exceptions/throwThroughBridge.def | 5 +++++ .../interop/exceptions/throwThroughBridge.kt | 5 +++++ .../exceptions/throwThroughBridgeInterop.cpp | 5 +++++ 7 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.cpp create mode 100644 kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.def create mode 100644 kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.kt create mode 100644 kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridgeInterop.cpp diff --git a/kotlin-native/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/main.kt b/kotlin-native/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/main.kt index fb2adc9ba5a..a9ce7b525af 100644 --- a/kotlin-native/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/main.kt +++ b/kotlin-native/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/main.kt @@ -483,6 +483,8 @@ internal fun buildNativeLibrary( // // Note that PCH and the *.c file should be compiled with the same optimization level. add("-O2") + // Allow throwing exceptions through generated stubs. + add("-fexceptions") addAll(additionalCompilerOpts) addAll(getCompilerFlagsForVfsOverlay(arguments.headerFilterPrefix.toTypedArray(), def)) addAll(when (language) { diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/CStubsManager.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/CStubsManager.kt index ef217806c34..d1e868a2d6d 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/CStubsManager.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/CStubsManager.kt @@ -40,8 +40,9 @@ class CStubsManager(private val target: KonanTarget) { val cSourcePath = cSource.absolutePath val clangCommand = clang.clangC( - *compilerOptions.toTypedArray(), "-O2", - cSourcePath, "-emit-llvm", "-c", "-o", bitcode.absolutePath + *compilerOptions.toTypedArray(), "-O2", + "-fexceptions", // Allow throwing exceptions through generated stubs. + cSourcePath, "-emit-llvm", "-c", "-o", bitcode.absolutePath ) if (dumpBridges) { println("CSTUBS for ${language}") diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index bb013b1c800..9874d52a366 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/kotlin-native/backend.native/tests/build.gradle @@ -3970,6 +3970,11 @@ createInterop("withSpaces") { it.defFile generateWithSpaceDefFile() } +createInterop("exceptions_throwThroughBridge") { + it.defFile "interop/exceptions/throwThroughBridge.def" + it.extraOpts "-Xcompile-source", "$projectDir/interop/exceptions/throwThroughBridgeInterop.cpp" +} + /** * Creates a task for the interop test. Configures runner and adds library and test build tasks. */ @@ -4208,8 +4213,7 @@ interopTest("interop_threadStates") { interopTest("interop_threadStates_callbacksWithExceptions") { disabled = (project.testTarget == 'wasm32') || // No interop for wasm yet. - !isExperimentalMM || // No thread state switching in the legacy MM. - project.globalTestArgs.contains('-opt') // TODO: Fix this test for -opt. + !isExperimentalMM // No thread state switching in the legacy MM. source = "interop/threadStates/callbacksWithExceptions.kt" interop = "threadStates" } @@ -4711,6 +4715,15 @@ dynamicTest("interop_memory_leaks") { outputChecker = { s -> s.contains("Memory leaks detected, 1 objects leaked!") } } +dynamicTest("interop_exceptions_throwThroughBridge") { + source = "interop/exceptions/throwThroughBridge.kt" + cSource = "$projectDir/interop/exceptions/throwThroughBridge.cpp" + clangTool = "clang++" + expectedExitStatusChecker = { it != 0 } + outputChecker = { s -> !s.contains("Should not happen") } + interop = "exceptions_throwThroughBridge" +} + tasks.register("library_mismatch", KonanDriverTest) { // Does not work for cross targets yet. enabled = !(project.testTarget != null && project.target.name != project.hostName) diff --git a/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.cpp b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.cpp new file mode 100644 index 00000000000..20ce76ae37a --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.cpp @@ -0,0 +1,11 @@ +#include "testlib_api.h" + +#include + +int main(int argc, char** argv) { + try { + testlib_symbols()->kotlin.root.kotlinFun(); + } catch (...) { + printf("Should not happen\n"); + } +} \ No newline at end of file diff --git a/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.def b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.def new file mode 100644 index 00000000000..243500cf916 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.def @@ -0,0 +1,5 @@ +language = C + +--- + +__attribute__((always_inline)) void throwCppException(); \ No newline at end of file diff --git a/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.kt b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.kt new file mode 100644 index 00000000000..ab38d972c49 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridge.kt @@ -0,0 +1,5 @@ +import throwThroughBridge.* + +fun kotlinFun() { + throwCppException() +} \ No newline at end of file diff --git a/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridgeInterop.cpp b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridgeInterop.cpp new file mode 100644 index 00000000000..18385211f93 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/exceptions/throwThroughBridgeInterop.cpp @@ -0,0 +1,5 @@ +#include + +extern "C" __attribute__((always_inline)) void throwCppException() { + throw std::runtime_error("Error"); +} \ No newline at end of file