[BOLT] Fix data race in MCPlusBuilder::getOrCreateAnnotationIndex #67004
Conversation
This file contains 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
MCPlusBuilder::getAnnotationIndex(Name) can be called from different threads, for example when making use of ParallelUtilities::runOnEachFunctionWithUniqueAllocId. The race occurs when an Index for a particular annotation Name needs to be created for the first time. For example, this can easily happen when multiple "copies" of an analysis pass run on different BinaryFunctions, and the analysis pass creates a new Annotation Index to be able to store analysis results as annotations. This was found by using the ThreadSanitizer. No regression test was added; I don't think there is good way to write regression tests that verify the absence of data races?
aaupov
reviewed
Sep 21, 2023
aaupov
approved these changes
Sep 21, 2023
Co-authored-by: Amir Ayupov <fads93@gmail.com>
|
Thanks for the quick review! |
Guzhu-AMD
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Sep 28, 2023
Local branch amd-gfx 345ce9d Merged main:7fcbb64fca5e into amd-gfx:b56d6b265611 Remote branch main 8fb28e4 [BOLT] Fix data race in MCPlusBuilder::getOrCreateAnnotationIndex (llvm#67004)
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.
MCPlusBuilder::getOrCreateAnnotationIndex(Name) can be called from different threads, for example when making use of
ParallelUtilities::runOnEachFunctionWithUniqueAllocId.
The race occurs when an Index for a particular annotation Name needs to be created for the first time.
For example, this can easily happen when multiple "copies" of an analysis pass run on different BinaryFunctions, and the analysis pass creates a new Annotation Index to be able to store analysis results as annotations.
This was found by using the ThreadSanitizer.
No regression test was added; I don't think there is good way to write regression tests that verify the absence of data races?