Fix crashing core.thread.fiber unittest for AArch64. (#4648)

ldc-developers · May 10, 2024 · fa4f032 · fa4f032
1 parent b514c6b
commit fa4f032
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 5 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -49,8 +49,6 @@ common_steps_template: &COMMON_STEPS_TEMPLATE
       cd $CIRRUS_WORKING_DIR/../build
       excludes="dmd-testsuite|ldc2-unittest|lit-tests"
       if [[ "$CI_OS-$CI_ARCH" == "linux-aarch64" ]]; then
-        # FIXME: segfaults with enabled optimizations
-        excludes+='|^core.thread.fiber(-shared)?$'
         # FIXME: failing unittest(s)
         excludes+='|^std.internal.math.gammafunction'
         # FIXME: failing unittest(s) with enabled optimizations

diff --git a/.github/workflows/supported_llvm_versions.yml b/.github/workflows/supported_llvm_versions.yml
@@ -182,8 +182,6 @@ jobs:
             if [[ '${{ matrix.os }}' == macos-14 ]]; then
               # FIXME: crashes frequently with enabled optimizations on M1 runners
               excludes+='|^std.internal.math.gammafunction(-shared)?$'
-              # FIXME: sporadically segfaults with enabled optimizations
-              excludes+='|^core.thread.fiber(-shared)?$'
             fi
           else
             N=$(nproc)

diff --git a/runtime/druntime/src/core/thread/fiber.d b/runtime/druntime/src/core/thread/fiber.d
@@ -558,6 +558,8 @@ version (LDC)
 
     version (Android) version = CheckFiberMigration;
 
+    version (AArch64) version = CheckFiberMigration;
+
     // Fiber migration across threads is (probably) not possible with ASan fakestack enabled (different parts of the stack
     // will contain fakestack pointers that were created on different threads...)
     version (SupportSanitizers) version = CheckFiberMigration;
@@ -1132,6 +1134,16 @@ class Fiber
      */
     static Fiber getThis() @safe nothrow @nogc
     {
+        // LDC NOTE:
+        // Currently, it is not safe to migrate fibers across threads when they
+        // use TLS at all, as LLVM might cache the TLS address lookup across a
+        // context switch (see https://github.com/ldc-developers/ldc/issues/666).
+        // Preventing inlining of this function, as well as switch{In,Out}
+        // below, enables users to do this at least as long as they are very
+        // careful about accessing TLS data themselves (such as in the shared
+        // fiber unittest below, which tends to sporadically crash with enabled
+        // optimizations if this prevent-inlining workaround is removed).
+        version (LDC) pragma(inline, false);
         return sm_this;
     }
 
@@ -1951,6 +1963,7 @@ private:
     //
     final void switchIn() nothrow @nogc
     {
+        // see note in getThis()
         version (LDC) pragma(inline, false);
 
         ThreadBase  tobj = ThreadBase.getThis();
@@ -2044,6 +2057,7 @@ private:
     //
     final void switchOut() nothrow @nogc
     {
+        // see note in getThis()
         version (LDC) pragma(inline, false);
 
         ThreadBase  tobj = m_curThread;
@@ -2320,7 +2334,7 @@ unittest
                         fibs[idx].call();
                         cont |= fibs[idx].state != Fiber.State.TERM;
                     }
-                    locks[idx] = false;
+                    locks[idx].atomicStore(false);
                 }
                 else
                 {