From 87f200da86ae7d671b1340b6ceddd48d6dc3d382 Mon Sep 17 00:00:00 2001 From: Troels Bjerre Lund Date: Wed, 10 Jan 2024 12:27:16 +0100 Subject: [PATCH 1/6] Disable signal handling of JVM GC signals The JVM uses signals as an efficient way to do GC safe-pointing. When clang is loaded as a native library on the JVM, the LLVM signal handler ends up getting triggered by the signals generated by the GC mechanism. This problem is addressed by libjsig, but that approach requires preloading the library on JVM startup, which cannot be ensured when using Gradle daemons. Furthermore, libjsig does not work on newer versions of MacOS. --- llvm/lib/Support/CrashRecoveryContext.cpp | 6 +++++- llvm/lib/Support/Unix/Signals.inc | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Support/CrashRecoveryContext.cpp b/llvm/lib/Support/CrashRecoveryContext.cpp index f53aea177d612..ee9385d08d0d0 100644 --- a/llvm/lib/Support/CrashRecoveryContext.cpp +++ b/llvm/lib/Support/CrashRecoveryContext.cpp @@ -344,8 +344,12 @@ static void uninstallExceptionOrSignalHandlers() { #include +/// KOTLIN/NATIVE SPECIFIC HACK +/// +/// We need to disable handling of signals that can originate from the JVM GC, +/// since they should not trigger the LLVM signal handler. static const int Signals[] = - { SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGTRAP }; + { SIGABRT, /* SIGBUS, SIGFPE, SIGILL, SIGSEGV, */ SIGTRAP }; static const unsigned NumSignals = std::size(Signals); static struct sigaction PrevActions[NumSignals]; diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc index 298fde1a387cc..0cd0b483cc06b 100644 --- a/llvm/lib/Support/Unix/Signals.inc +++ b/llvm/lib/Support/Unix/Signals.inc @@ -303,6 +303,17 @@ static void RegisterHandlers() { // Not signal-safe. enum class SignalKind { IsKill, IsInfo }; auto registerHandler = [&](int Signal, SignalKind Kind) { + /// KOTLIN/NATIVE SPECIFIC HACK + /// + /// We need to disable handling of signals that can originate from the JVM GC, + /// since they should not trigger the LLVM signal handler. + + static const int JvmSigs[] = { + SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGUSR1, SIGUSR2 + }; + + for (auto S : JvmSigs) if (Signal == S) return; + unsigned Index = NumRegisteredSignals.load(); assert(Index < std::size(RegisteredSignalInfo) && "Out of space for signal handlers!"); From 06a3fc2e5d6e5a50037332893d933412f3aa1927 Mon Sep 17 00:00:00 2001 From: Igor Chevdar Date: Wed, 14 Aug 2024 16:43:56 +0300 Subject: [PATCH 2/6] Added an option to configure MaxDevirtIterations (#3) * Added an option to configure MaxDevirtIterations * fixup! Added an option to configure MaxDevirtIterations (cherry picked from commit 74a4177743f3ffa2a04f4c3443860c345f68e6a2) --- llvm/include/llvm-c/Transforms/PassBuilder.h | 3 +++ llvm/include/llvm/Passes/PassBuilder.h | 3 +++ llvm/lib/Passes/PassBuilderBindings.cpp | 5 +++++ llvm/lib/Passes/PassBuilderPipelines.cpp | 4 +++- 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm-c/Transforms/PassBuilder.h b/llvm/include/llvm-c/Transforms/PassBuilder.h index d0466dd7fc0a1..de054bb01b226 100644 --- a/llvm/include/llvm-c/Transforms/PassBuilder.h +++ b/llvm/include/llvm-c/Transforms/PassBuilder.h @@ -102,6 +102,9 @@ void LLVMPassBuilderOptionsSetMergeFunctions(LLVMPassBuilderOptionsRef Options, void LLVMPassBuilderOptionsSetInlinerThreshold( LLVMPassBuilderOptionsRef Options, int Threshold); +void LLVMPassBuilderOptionsSetMaxDevirtIterations( + LLVMPassBuilderOptionsRef Options, int Iterations); + /** * Dispose of a heap-allocated PassBuilderOptions instance */ diff --git a/llvm/include/llvm/Passes/PassBuilder.h b/llvm/include/llvm/Passes/PassBuilder.h index 50da57c6854cd..ac331695f47b2 100644 --- a/llvm/include/llvm/Passes/PassBuilder.h +++ b/llvm/include/llvm/Passes/PassBuilder.h @@ -96,6 +96,9 @@ class PipelineTuningOptions { // analyses after various module->function or cgscc->function adaptors in the // default pipelines. bool EagerlyInvalidateAnalyses; + + /// Tuning option to override default devirtualization iterations. + int MaxDevirtIterations; }; /// This class provides access to building LLVM's passes. diff --git a/llvm/lib/Passes/PassBuilderBindings.cpp b/llvm/lib/Passes/PassBuilderBindings.cpp index b80dc0231ed5f..cc36c97f83c6e 100644 --- a/llvm/lib/Passes/PassBuilderBindings.cpp +++ b/llvm/lib/Passes/PassBuilderBindings.cpp @@ -145,6 +145,11 @@ void LLVMPassBuilderOptionsSetInlinerThreshold( unwrap(Options)->PTO.InlinerThreshold = Threshold; } +void LLVMPassBuilderOptionsSetMaxDevirtIterations( + LLVMPassBuilderOptionsRef Options, int Iterations) { + unwrap(Options)->PTO.MaxDevirtIterations = Iterations; +} + void LLVMDisposePassBuilderOptions(LLVMPassBuilderOptionsRef Options) { delete unwrap(Options); } diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index bc595869ab869..8bbc2cb6965e4 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -321,6 +321,7 @@ PipelineTuningOptions::PipelineTuningOptions() { UnifiedLTO = false; MergeFunctions = EnableMergeFunctions; InlinerThreshold = -1; + MaxDevirtIterations = -1; EagerlyInvalidateAnalyses = EnableEagerlyInvalidateAnalyses; } @@ -912,9 +913,10 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level, if (PGOOpt) IP.EnableDeferral = EnablePGOInlineDeferral; + int MDI = PTO.MaxDevirtIterations == -1 ? MaxDevirtIterations : PTO.MaxDevirtIterations; ModuleInlinerWrapperPass MIWP(IP, PerformMandatoryInliningsFirst, InlineContext{Phase, InlinePass::CGSCCInliner}, - UseInlineAdvisor, MaxDevirtIterations); + UseInlineAdvisor, MDI); // Require the GlobalsAA analysis for the module so we can query it within // the CGSCC pipeline. From dbe60fe4f1a4e9fd2c583e766c2c5e0e559101d2 Mon Sep 17 00:00:00 2001 From: "Aleksei.Glushko" Date: Wed, 5 Mar 2025 18:43:15 +0100 Subject: [PATCH 3/6] Revert "[ClangModuleManager] Extend the ext4 file system workaround" The reverted commit causes a PCH handling bug on Linux (KT-75560). This reverts commit 8c74a9adceb45ef90e1256513afdb3004c68508b. --- clang/lib/Basic/FileManager.cpp | 2 +- clang/lib/Frontend/CompilerInstance.cpp | 9 +-------- .../lib/Frontend/Rewrite/FrontendActions.cpp | 8 +------- clang/lib/Serialization/ModuleManager.cpp | 19 +++---------------- clang/test/Modules/explicit-build-relpath.cpp | 3 --- 5 files changed, 6 insertions(+), 35 deletions(-) diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index a8988d75ea2ea..375e1dc693341 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -477,7 +477,7 @@ OptionalFileEntryRef FileManager::getBypassFile(FileEntryRef VF) { // If we've already bypassed just use the existing one. auto Insertion = SeenBypassFileEntries->insert( - {VF.getNameAsRequested(), std::errc::no_such_file_or_directory}); + {VF.getName(), std::errc::no_such_file_or_directory}); if (!Insertion.second) return FileEntryRef(*Insertion.first); diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 7c8fd631913b8..10e5364b0ddea 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -2076,16 +2076,9 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( // Check whether M refers to the file in the prebuilt module path. if (M && M->getASTFile()) - if (auto ModuleFile = FileMgr->getOptionalFileRef(ModuleFilename)) { + if (auto ModuleFile = FileMgr->getFile(ModuleFilename)) if (*ModuleFile == M->getASTFile()) return M; -#if !defined(__APPLE__) - // Workaround for ext4 file system. Also check bypass file if exists. - if (auto Bypass = FileMgr->getBypassFile(*ModuleFile)) - if (*Bypass == M->getASTFile()) - return M; -#endif - } getDiagnostics().Report(ModuleNameLoc, diag::err_module_prebuilt) << ModuleName; diff --git a/clang/lib/Frontend/Rewrite/FrontendActions.cpp b/clang/lib/Frontend/Rewrite/FrontendActions.cpp index 2ee97a5b0f648..cf5a9437e89e6 100644 --- a/clang/lib/Frontend/Rewrite/FrontendActions.cpp +++ b/clang/lib/Frontend/Rewrite/FrontendActions.cpp @@ -213,15 +213,9 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener { void visitModuleFile(StringRef Filename, serialization::ModuleKind Kind) override { - auto File = CI.getFileManager().getOptionalFileRef(Filename); + auto File = CI.getFileManager().getFile(Filename); assert(File && "missing file for loaded module?"); -#if !defined(__APPLE__) - // Workaround for ext4 file system. - if (auto Bypass = CI.getFileManager().getBypassFile(*File)) - File = *Bypass; -#endif - // Only rewrite each module file once. if (!Rewritten.insert(*File).second) return; diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 6c852130ad93b..48f622713d24a 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -42,18 +42,8 @@ using namespace clang; using namespace serialization; ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const { - auto Entry = FileMgr.getOptionalFileRef(Name, /*OpenFile=*/false, - /*CacheFailure=*/false); -#if !defined(__APPLE__) - if (Entry) { - // On Linux ext4 FileManager's inode caching system does not - // provide us correct behaviour for ModuleCache directories. - // inode can be reused after PCM delete resulting in cache misleading. - if (auto BypassFile = FileMgr.getBypassFile(*Entry)) - Entry = *BypassFile; - } -#endif - + auto Entry = FileMgr.getFile(Name, /*OpenFile=*/false, + /*CacheFailure=*/false); if (Entry) return lookup(*Entry); @@ -460,10 +450,7 @@ bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize, // On Linux ext4 FileManager's inode caching system does not // provide us correct behaviour for ModuleCache directories. // inode can be reused after PCM delete resulting in cache misleading. - // Only use the bypass file if bypass succeed in case the underlying file - // system doesn't support bypass (thus there is no need for the workaround). - if (auto Bypass = FileMgr.getBypassFile(*File)) - File = *Bypass; + File = FileMgr.getBypassFile(*File); } #endif diff --git a/clang/test/Modules/explicit-build-relpath.cpp b/clang/test/Modules/explicit-build-relpath.cpp index 0f431b1d8ad07..708c9fd208b81 100644 --- a/clang/test/Modules/explicit-build-relpath.cpp +++ b/clang/test/Modules/explicit-build-relpath.cpp @@ -1,6 +1,3 @@ -/// Due to module bypass workaround in https://reviews.llvm.org/D97850 for ext4 FS, relative and absolute path do not match after bypass. -// REQUIRES: system-darwin - // RUN: rm -rf %t // RUN: mkdir %t // RUN: cd %t From 17d6535699ccc6a37782b7f3353634f2e2bff182 Mon Sep 17 00:00:00 2001 From: "Aleksei.Glushko" Date: Wed, 5 Mar 2025 18:53:23 +0100 Subject: [PATCH 4/6] Revert "Merge pull request #3221 from apple/eng/D97850-next" The reverted commit causes a PCH handling bug on Linux (KT-75560). This reverts commit b72d76f8d5758d75a5076e9cfe578dd96fdccb81, reversing changes made to 80d1c852e2e09a92202aca4a8c539a2d65f03278. --- clang/lib/Serialization/ModuleManager.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 48f622713d24a..3e1298f60e18e 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -445,15 +445,6 @@ bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize, File = FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true, /*CacheFailure=*/false); -#if !defined(__APPLE__) - if (File) { - // On Linux ext4 FileManager's inode caching system does not - // provide us correct behaviour for ModuleCache directories. - // inode can be reused after PCM delete resulting in cache misleading. - File = FileMgr.getBypassFile(*File); - } -#endif - // If the file is known to the module cache but not in the filesystem, it is // a memory buffer. Create a virtual file for it. if (!File) { From 8f7f9e84a4865c98752c38744042bda0b2595b8c Mon Sep 17 00:00:00 2001 From: "Aleksei.Glushko" Date: Tue, 18 Mar 2025 13:13:03 +0100 Subject: [PATCH 5/6] Workaround MSVC (14.29.30133) compilation failure --- llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp index 0ad5f93291c69..6d76f75e03b58 100644 --- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp @@ -1783,7 +1783,7 @@ void ThinLTOCodeGenerator::run() { }; struct ModuleInfo { std::unique_ptr Entry; - std::atomic State = ModuleState::MS_Empty; + std::atomic State = 0b0000; std::unique_ptr ComputedBuffer; std::unique_ptr CachedBuffer; }; From 50d9c1849b5cab35d108962dfa896423fd43f428 Mon Sep 17 00:00:00 2001 From: "Aleksei.Glushko" Date: Tue, 8 Apr 2025 16:05:37 +0200 Subject: [PATCH 6/6] Mute SEGFAULT recovery tests. Several tests in CrashRecoveryTest become invalid after the commit of "Disable signal handling of JVM GC signals". --- llvm/unittests/Support/CrashRecoveryTest.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp index be67c9afdc48b..c63145c733827 100644 --- a/llvm/unittests/Support/CrashRecoveryTest.cpp +++ b/llvm/unittests/Support/CrashRecoveryTest.cpp @@ -39,7 +39,12 @@ static void incrementGlobal() { ++GlobalInt; } static void llvmTrap() { LLVM_BUILTIN_TRAP; } static void incrementGlobalWithParam(void *) { ++GlobalInt; } +#define SKIP_WITH_KOTLIN_PATCH() \ + GTEST_SKIP() << "Crash recovery from SEGFAULT disabled by Kotlin patch" + TEST(CrashRecoveryTest, Basic) { + SKIP_WITH_KOTLIN_PATCH(); + llvm::CrashRecoveryContext::Enable(); GlobalInt = 0; EXPECT_TRUE(CrashRecoveryContext().RunSafely(incrementGlobal)); @@ -57,6 +62,8 @@ struct IncrementGlobalCleanup : CrashRecoveryContextCleanup { static void noop() {} TEST(CrashRecoveryTest, Cleanup) { + SKIP_WITH_KOTLIN_PATCH(); + llvm::CrashRecoveryContext::Enable(); GlobalInt = 0; { @@ -77,6 +84,8 @@ TEST(CrashRecoveryTest, Cleanup) { } TEST(CrashRecoveryTest, DumpStackCleanup) { + SKIP_WITH_KOTLIN_PATCH(); + SmallString<128> Filename; std::error_code EC = sys::fs::createTemporaryFile("crash", "test", Filename); EXPECT_FALSE(EC);