Skip to content

Commit 554459a

Browse files
kbeylsmaksfb
andauthored
[BOLT] Fix runOnEachFunctionWithUniqueAllocId (llvm#90039)
When runOnEachFunctionWithUniqueAllocId is invoked with ForceSequential=true, then the current implementation runs the function with AllocId==0, which is the Id for the shared, non-unique, default AnnotationAllocator. However, the documentation for runOnEachFunctionWithUniqueAllocId states: ``` /// Perform the work on each BinaryFunction except those that are rejected /// by SkipPredicate, and create a unique annotation allocator for each /// task. This should be used whenever the work function creates annotations to /// allow thread-safe annotation creation. ``` Therefore, even when ForceSequential==true, a unique AllocId should be used, i.e. different from 0. In the current upstream BOLT this is presumably not depended on, but it is needed to reduce memory usage for analyses that use a lot of memory/annotations. Examples are the pac-ret and stack-clash analyses that currently have prototype implementations as described in https://discourse.llvm.org/t/rfc-bolt-based-binary-analysis-tool-to-verify-correctness-of-security-hardening/78148 These analyses use the DataFlowAnalysis framework to sometimes store quite a lot of information on each MCInst. They run in parallel on each function. When the dataflow analysis is finished, the annotations on each MCInst can be removed, hugely saving on memory consumption. The only annotations that need to remain are those that indicate some unexpected properties somewhere in the binary. Fixing this bug enables implementing the deletion of the memory used by those huge number of DataFlowAnalysis annotations (by invoking BC.MIB->freeValuesAllocator(AllocatorId)), even when run with --no-threads. Without this bug fixed, the invocation of BC.MIB->freeValuesAllocator(AllocatorId) results in also the memory for all other annotations to be deleted, as AllocatorId is 0. --------- Co-authored-by: Maksim Panchenko <maks@meta.com>
1 parent 1343e68 commit 554459a

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

bolt/lib/Core/ParallelUtilities.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,20 @@ void runOnEachFunctionWithUniqueAllocId(
188188
LLVM_DEBUG(T.stopTimer());
189189
};
190190

191+
unsigned AllocId = 1;
192+
auto EnsureAllocatorExists = [&BC](unsigned AllocId) {
193+
if (!BC.MIB->checkAllocatorExists(AllocId)) {
194+
MCPlusBuilder::AllocatorIdTy Id =
195+
BC.MIB->initializeNewAnnotationAllocator();
196+
(void)Id;
197+
assert(AllocId == Id && "unexpected allocator id created");
198+
}
199+
};
200+
191201
if (opts::NoThreads || ForceSequential) {
192-
runBlock(BC.getBinaryFunctions().begin(), BC.getBinaryFunctions().end(), 0);
202+
EnsureAllocatorExists(AllocId);
203+
runBlock(BC.getBinaryFunctions().begin(), BC.getBinaryFunctions().end(),
204+
AllocId);
193205
return;
194206
}
195207
// This lock is used to postpone task execution
@@ -205,19 +217,13 @@ void runOnEachFunctionWithUniqueAllocId(
205217
ThreadPoolInterface &Pool = getThreadPool();
206218
auto BlockBegin = BC.getBinaryFunctions().begin();
207219
unsigned CurrentCost = 0;
208-
unsigned AllocId = 1;
209220
for (auto It = BC.getBinaryFunctions().begin();
210221
It != BC.getBinaryFunctions().end(); ++It) {
211222
BinaryFunction &BF = It->second;
212223
CurrentCost += computeCostFor(BF, SkipPredicate, SchedPolicy);
213224

214225
if (CurrentCost >= BlockCost) {
215-
if (!BC.MIB->checkAllocatorExists(AllocId)) {
216-
MCPlusBuilder::AllocatorIdTy Id =
217-
BC.MIB->initializeNewAnnotationAllocator();
218-
(void)Id;
219-
assert(AllocId == Id && "unexpected allocator id created");
220-
}
226+
EnsureAllocatorExists(AllocId);
221227
Pool.async(runBlock, BlockBegin, std::next(It), AllocId);
222228
AllocId++;
223229
BlockBegin = std::next(It);

0 commit comments

Comments
 (0)