Skip to content

Commit 8fb28e4

Browse files
kbeylsaaupov
andauthored
[BOLT] Fix data race in MCPlusBuilder::getOrCreateAnnotationIndex (#67004)
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? --------- Co-authored-by: Amir Ayupov <[email protected]>
1 parent 3769aaa commit 8fb28e4

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "llvm/Support/Allocator.h"
3030
#include "llvm/Support/ErrorHandling.h"
3131
#include "llvm/Support/ErrorOr.h"
32+
#include "llvm/Support/RWMutex.h"
3233
#include <cassert>
3334
#include <cstdint>
3435
#include <map>
@@ -166,6 +167,10 @@ class MCPlusBuilder {
166167
/// Names of non-standard annotations.
167168
SmallVector<std::string, 8> AnnotationNames;
168169

170+
/// A mutex that is used to control parallel accesses to
171+
/// AnnotationNameIndexMap and AnnotationsNames.
172+
mutable llvm::sys::RWMutex AnnotationNameMutex;
173+
169174
/// Allocate the TailCall annotation value. Clients of the target-specific
170175
/// MCPlusBuilder classes must use convert/lower/create* interfaces instead.
171176
void setTailCall(MCInst &Inst);
@@ -1775,6 +1780,7 @@ class MCPlusBuilder {
17751780

17761781
/// Return annotation index matching the \p Name.
17771782
std::optional<unsigned> getAnnotationIndex(StringRef Name) const {
1783+
std::shared_lock<llvm::sys::RWMutex> Lock(AnnotationNameMutex);
17781784
auto AI = AnnotationNameIndexMap.find(Name);
17791785
if (AI != AnnotationNameIndexMap.end())
17801786
return AI->second;
@@ -1784,10 +1790,10 @@ class MCPlusBuilder {
17841790
/// Return annotation index matching the \p Name. Create a new index if the
17851791
/// \p Name wasn't registered previously.
17861792
unsigned getOrCreateAnnotationIndex(StringRef Name) {
1787-
auto AI = AnnotationNameIndexMap.find(Name);
1788-
if (AI != AnnotationNameIndexMap.end())
1789-
return AI->second;
1793+
if (std::optional<unsigned> Index = getAnnotationIndex(Name))
1794+
return *Index;
17901795

1796+
std::unique_lock<llvm::sys::RWMutex> Lock(AnnotationNameMutex);
17911797
const unsigned Index =
17921798
AnnotationNameIndexMap.size() + MCPlus::MCAnnotation::kGeneric;
17931799
AnnotationNameIndexMap.insert(std::make_pair(Name, Index));

0 commit comments

Comments
 (0)