Fix stacktrace printing during _tryRetain termination (#4213)

Avoids using objc runtime for termination handling if illegal sharing is detected in _tryRetain, and adds a test for it.

ObjC runtime uses shared non-recursive locks for object lifetime management methods, which include _tryRetain and object destruction. There are several locks per runtime divided between groups of objects based on their addresses.

_tryRetain takes a lock, and if we terminate with unhandled exception inside its stack, we are not allowed to make ObjC runtime to destruct any object, because that object may take the same lock and consequently fail since locks are not recursive.

Therefore, avoid touching ObjC runtime when terminating inside _tryRetain. This includes avoiding using ObjC's default unhandled exception handler, and our own usage of CoreSymbolication framework.

backend.native/tests/build.gradle

@@ -3560,6 +3560,10 @@ if (PlatformInfo.isAppleTarget(project)) {
         it.headers "$projectDir/framework/gh3343/objclib.h"
         it.linkerOpts "-lobjcgh3343"
     }
+    createInterop("objc_illegal_sharing_with_weak") {
+      it.defFile 'interop/objc/illegal_sharing_with_weak/objclib.def'
+      it.headers "$projectDir/interop/objc/illegal_sharing_with_weak/objclib.h"
+    }
 }
 
 createInterop("withSpaces") {
@@ -3913,6 +3917,18 @@ if (PlatformInfo.isAppleTarget(project)) {
         interop = 'objcKt34467'
         goldValue = "OK\n42\n"
     }
+
+    interopTest("interop_objc_illegal_sharing_with_weak") {
+        source = "interop/objc/illegal_sharing_with_weak/main.kt"
+        interop = 'objc_illegal_sharing_with_weak'
+
+        expectedExitStatusChecker = { it != 0 }
+        outputChecker = {
+            it.startsWith("Before") &&                   // Should crash after "Before"
+                !it.contains("After") &&                 // But before "After"
+                it.contains("isObjectAliveShouldCrash")  // And should contain stack trace.
+        }
+    }
 }
 
 standaloneTest("jsinterop_math") {
@@ -4479,4 +4495,4 @@ if (target.family == Family.IOS && (target.architecture == ARM32 || target.archi
                     it instanceof KonanLinkTest
             }
             .forEach { it.enabled = false }
-}
+}

backend.native/tests/interop/objc/illegal_sharing_with_weak/main.kt

@@ -0,0 +1,28 @@
+import kotlin.native.concurrent.*
+import kotlinx.cinterop.autoreleasepool
+import objclib.*
+
+fun main() {
+    autoreleasepool {
+        run()
+    }
+}
+
+private class NSObjectImpl : NSObject() {
+    var x = 111
+}
+
+fun run() = withWorker {
+    val obj = NSObjectImpl()
+    setObject(obj)
+
+    println("Before")
+    val isAlive = try {
+        execute(TransferMode.SAFE, {}) {
+            isObjectAliveShouldCrash()
+        }.result
+    } catch (e: Throwable) {
+        false
+    }
+    println("After $isAlive")
+}

backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.def

@@ -0,0 +1,2 @@
+language = Objective-C
+headerFilter = **/objclib.h

backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.h

@@ -0,0 +1,13 @@
+#import <objc/NSObject.h>
+
+static NSObject* __weak globalObject = nil;
+
+void setObject(NSObject* obj) {
+  globalObject = obj;
+}
+
+// Make sure this function persists, because the test expects to find this function in the stack trace.
+__attribute__((noinline))
+bool isObjectAliveShouldCrash() {
+  return globalObject != nil;
+}

runtime/src/main/cpp/Exceptions.cpp

@@ -103,6 +103,14 @@ _Unwind_Reason_Code unwindCallback(
 }
 #endif
 
+THREAD_LOCAL_VARIABLE bool disallowSourceInfo = false;
+
+SourceInfo getSourceInfo(KConstRef stackTrace, int index) {
+  return disallowSourceInfo
+      ? SourceInfo { .fileName = nullptr, .lineNumber = -1, .column = -1 }
+      : Kotlin_getSourceInfo(*PrimitiveArrayAddressOfElementAt<KNativePtr>(stackTrace->array(), index));
+}
+
 }  // namespace
 
 extern "C" {
@@ -172,7 +180,7 @@ OBJ_GETTER(GetStackTraceStrings, KConstRef stackTrace) {
     RuntimeCheck(symbols != nullptr, "Not enough memory to retrieve the stacktrace");
 
     for (int index = 0; index < size; ++index) {
-      auto sourceInfo = Kotlin_getSourceInfo(*PrimitiveArrayAddressOfElementAt<KNativePtr>(stackTrace->array(), index));
+      auto sourceInfo = getSourceInfo(stackTrace, index);
       const char* symbol = symbols[index];
       const char* result;
       char line[1024];
@@ -302,4 +310,8 @@ void SetKonanTerminateHandler() {
 
 #endif // KONAN_OBJC_INTEROP
 
-} // extern "C"
+} // extern "C"
+
+void DisallowSourceInfo() {
+  disallowSourceInfo = true;
+}

runtime/src/main/cpp/Exceptions.h

@@ -67,4 +67,7 @@ void PrintThrowable(KRef);
 } // extern "C"
 #endif
 
+// It's not always safe to extract SourceInfo during unhandled exception termination.
+void DisallowSourceInfo();
+
 #endif // RUNTIME_NAMES_H

runtime/src/main/cpp/ObjCInterop.mm

@@ -102,17 +102,14 @@ BOOL _tryRetainImp(id self, SEL _cmd) {
     return getBackRef(self)->tryAddRef();
   } catch (ExceptionObjHolder& e) {
     // TODO: check for IncorrectDereferenceException and possible weak property access
-    @try {
-      // try and catch with objc_terminate: this is a workaround to terminate immediately
-      // with libc default_terminate_handler() to be called instead of custom `TerminateWithUnhandledException`.
-      // See `KonanTerminateHandler`.
-      // TerminateWithUnhandledException shall not be used here because in debug mode it uses
-      // CoreSymbolication framework (CSSymbolOwnerGetSymbolWithAddress) which fails at recursive retain lock.
-      [NSException raise:NSGenericException
-                  format:@"Possible illegal attempt to access weak property from non-owning thread"];
-    } @catch (...) {
-      objc_terminate();
-    }
+    // Cannot use SourceInfo here, because CoreSymbolication framework (CSSymbolOwnerGetSymbolWithAddress)
+    // fails at recursive retain lock. Similarly, cannot use objc exception here, because its unhandled
+    // exception handler might fail at recursive retain lock too.
+    // TODO: Refactor to be more explicit. Instead of relying on an unhandled exception termination
+    // (and effectively setting a global to alter its behavior), just call an appropriate termination
+    // function by hand.
+    DisallowSourceInfo();
+    std::terminate();
   }
 }