[clang] Enable making the module build stack thread-safe #137059
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesThis PR makes another piece of the In this PR, we prevent this by keeping the stack empty and moving the responsibility of cycle detection to the client. The client can recreate the same module build stack externally and ensure thread-safety by enforcing mutual exclusion. Full diff: https://github.com/llvm/llvm-project/pull/137059.diff 1 Files Affected:
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 8596dd03148e8..1526ea53add7d 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1240,11 +1240,15 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
Instance.createSourceManager(Instance.getFileManager());
SourceManager &SourceMgr = Instance.getSourceManager();
- // Note that this module is part of the module build stack, so that we
- // can detect cycles in the module graph.
- SourceMgr.setModuleBuildStack(getSourceManager().getModuleBuildStack());
- SourceMgr.pushModuleBuildStack(ModuleName,
- FullSourceLoc(ImportLoc, getSourceManager()));
+ if (ThreadSafeConfig) {
+ // Detecting cycles in the module graph is responsibility of the client.
+ } else {
+ // Note that this module is part of the module build stack, so that we
+ // can detect cycles in the module graph.
+ SourceMgr.setModuleBuildStack(getSourceManager().getModuleBuildStack());
+ SourceMgr.pushModuleBuildStack(
+ ModuleName, FullSourceLoc(ImportLoc, getSourceManager()));
+ }
// Make a copy for the new instance.
Instance.FailedModules = FailedModules;
|
cyndyishida
approved these changes
Apr 24, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/4080 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/18922 Here is the relevant piece of the build log for the reference |
jansvoboda11
added a commit
to jansvoboda11/llvm-project
that referenced
this pull request
Apr 24, 2025
…tor thread-safe The same principle as llvm#135473, llvm#135737, llvm#136178, llvm#136601 & llvm#137059.
jansvoboda11
added a commit
that referenced
this pull request
Apr 29, 2025
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This PR makes another piece of the `CompilerInstance::cloneForModuleCompile()` result thread-safe: the module build stack. This data structure is used to detect cyclic dependencies between modules. The problem is that it uses `FullSourceLoc` which refers to the `SourceManager` of the parent `CompilerInstance`: if two threads happen to execute `CompilerInstance`s cloned from the same parent concurrently, and both discover a dependency cycle, they may concurrently access the parent `SourceManager` when emitting the diagnostic, creating a data race. In this PR, we prevent this by keeping the stack empty and moving the responsibility of cycle detection to the client. The client can recreate the same module build stack externally and ensure thread-safety by enforcing mutual exclusion.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…tor thread-safe (llvm#137227) The same principle as llvm#135473, llvm#135737, llvm#136178, llvm#136601 & llvm#137059.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This PR makes another piece of the `CompilerInstance::cloneForModuleCompile()` result thread-safe: the module build stack. This data structure is used to detect cyclic dependencies between modules. The problem is that it uses `FullSourceLoc` which refers to the `SourceManager` of the parent `CompilerInstance`: if two threads happen to execute `CompilerInstance`s cloned from the same parent concurrently, and both discover a dependency cycle, they may concurrently access the parent `SourceManager` when emitting the diagnostic, creating a data race. In this PR, we prevent this by keeping the stack empty and moving the responsibility of cycle detection to the client. The client can recreate the same module build stack externally and ensure thread-safety by enforcing mutual exclusion.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…tor thread-safe (llvm#137227) The same principle as llvm#135473, llvm#135737, llvm#136178, llvm#136601 & llvm#137059.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This PR makes another piece of the `CompilerInstance::cloneForModuleCompile()` result thread-safe: the module build stack. This data structure is used to detect cyclic dependencies between modules. The problem is that it uses `FullSourceLoc` which refers to the `SourceManager` of the parent `CompilerInstance`: if two threads happen to execute `CompilerInstance`s cloned from the same parent concurrently, and both discover a dependency cycle, they may concurrently access the parent `SourceManager` when emitting the diagnostic, creating a data race. In this PR, we prevent this by keeping the stack empty and moving the responsibility of cycle detection to the client. The client can recreate the same module build stack externally and ensure thread-safety by enforcing mutual exclusion.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…tor thread-safe (llvm#137227) The same principle as llvm#135473, llvm#135737, llvm#136178, llvm#136601 & llvm#137059.
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
…tor thread-safe (llvm#137227) The same principle as llvm#135473, llvm#135737, llvm#136178, llvm#136601 & llvm#137059.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
This PR makes another piece of the `CompilerInstance::cloneForModuleCompile()` result thread-safe: the module build stack. This data structure is used to detect cyclic dependencies between modules. The problem is that it uses `FullSourceLoc` which refers to the `SourceManager` of the parent `CompilerInstance`: if two threads happen to execute `CompilerInstance`s cloned from the same parent concurrently, and both discover a dependency cycle, they may concurrently access the parent `SourceManager` when emitting the diagnostic, creating a data race. In this PR, we prevent this by keeping the stack empty and moving the responsibility of cycle detection to the client. The client can recreate the same module build stack externally and ensure thread-safety by enforcing mutual exclusion.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
…tor thread-safe (llvm#137227) The same principle as llvm#135473, llvm#135737, llvm#136178, llvm#136601 & llvm#137059.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes another piece of the
CompilerInstance::cloneForModuleCompile()result thread-safe: the module build stack. This data structure is used to detect cyclic dependencies between modules. The problem is that it usesFullSourceLocwhich refers to theSourceManagerof the parentCompilerInstance: if two threads happen to executeCompilerInstances cloned from the same parent concurrently, and both discover a dependency cycle, they may concurrently access the parentSourceManagerwhen emitting the diagnostic, creating a data race.In this PR, we prevent this by keeping the stack empty and moving the responsibility of cycle detection to the client. The client can recreate the same module build stack externally and ensure thread-safety by enforcing mutual exclusion.