Skip to content

Commit 846eb76

Browse files
committed
[BOLT][AArch64] Fix instrumentation deadloop
According to ARMv8-a architecture reference manual B2.10.5 software must avoid having any explicit memory accesses between exclusive load and associated store instruction. Otherwise exclusive monitor might clear the exclusivity without application-related cause which may result in the deadloop. Disable instrumentation for such functions, since between exclusive load and store there might be branches and we would insert instrumentation snippet which contains loads and stores. The better solution would be to analyze with BFS finding the exact BBs between load and store and not instrumenting them. Or even better to recognize such sequences and replace them with more complex one, e.g. loading value non exclusively, and for the brach where exclusive store is made make exclusive load and store sequentially, but for now just disable instrumentation for such functions completely. Differential Revision: https://reviews.llvm.org/D159520
1 parent 2aa4a32 commit 846eb76

File tree

4 files changed

+94
-0
lines changed

4 files changed

+94
-0
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,11 @@ class MCPlusBuilder {
626626
return Info->get(Inst.getOpcode()).mayStore();
627627
}
628628

629+
virtual bool isAArch64Exclusive(const MCInst &Inst) const {
630+
llvm_unreachable("not implemented");
631+
return false;
632+
}
633+
629634
virtual bool isCleanRegXOR(const MCInst &Inst) const {
630635
llvm_unreachable("not implemented");
631636
return false;

bolt/lib/Passes/Instrumentation.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "bolt/Passes/Instrumentation.h"
1414
#include "bolt/Core/ParallelUtilities.h"
1515
#include "bolt/RuntimeLibs/InstrumentationRuntimeLibrary.h"
16+
#include "bolt/Utils/CommandLineOpts.h"
1617
#include "bolt/Utils/Utils.h"
1718
#include "llvm/Support/CommandLine.h"
1819
#include "llvm/Support/RWMutex.h"
@@ -85,6 +86,24 @@ cl::opt<bool> InstrumentCalls("instrument-calls",
8586
namespace llvm {
8687
namespace bolt {
8788

89+
static bool hasAArch64ExclusiveMemop(BinaryFunction &Function) {
90+
// FIXME ARMv8-a architecture reference manual says that software must avoid
91+
// having any explicit memory accesses between exclusive load and associated
92+
// store instruction. So for now skip instrumentation for functions that have
93+
// these instructions, since it might lead to runtime deadlock.
94+
BinaryContext &BC = Function.getBinaryContext();
95+
for (const BinaryBasicBlock &BB : Function)
96+
for (const MCInst &Inst : BB)
97+
if (BC.MIB->isAArch64Exclusive(Inst)) {
98+
if (opts::Verbosity >= 1)
99+
outs() << "BOLT-INSTRUMENTER: Function " << Function
100+
<< " has exclusive instructions, skip instrumentation\n";
101+
return true;
102+
}
103+
104+
return false;
105+
}
106+
88107
uint32_t Instrumentation::getFunctionNameIndex(const BinaryFunction &Function) {
89108
auto Iter = FuncToStringIdx.find(&Function);
90109
if (Iter != FuncToStringIdx.end())
@@ -288,6 +307,9 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function,
288307
if (BC.isMachO() && Function.hasName("___GLOBAL_init_65535/1"))
289308
return;
290309

310+
if (BC.isAArch64() && hasAArch64ExclusiveMemop(Function))
311+
return;
312+
291313
SplitWorklistTy SplitWorklist;
292314
SplitInstrsTy SplitInstrs;
293315

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,34 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
272272
return isLDRB(Inst) || isLDRH(Inst) || isLDRW(Inst) || isLDRX(Inst);
273273
}
274274

275+
bool isAArch64Exclusive(const MCInst &Inst) const override {
276+
return (Inst.getOpcode() == AArch64::LDXPX ||
277+
Inst.getOpcode() == AArch64::LDXPW ||
278+
Inst.getOpcode() == AArch64::LDXRX ||
279+
Inst.getOpcode() == AArch64::LDXRW ||
280+
Inst.getOpcode() == AArch64::LDXRH ||
281+
Inst.getOpcode() == AArch64::LDXRB ||
282+
Inst.getOpcode() == AArch64::STXPX ||
283+
Inst.getOpcode() == AArch64::STXPW ||
284+
Inst.getOpcode() == AArch64::STXRX ||
285+
Inst.getOpcode() == AArch64::STXRW ||
286+
Inst.getOpcode() == AArch64::STXRH ||
287+
Inst.getOpcode() == AArch64::STXRB ||
288+
Inst.getOpcode() == AArch64::LDAXPX ||
289+
Inst.getOpcode() == AArch64::LDAXPW ||
290+
Inst.getOpcode() == AArch64::LDAXRX ||
291+
Inst.getOpcode() == AArch64::LDAXRW ||
292+
Inst.getOpcode() == AArch64::LDAXRH ||
293+
Inst.getOpcode() == AArch64::LDAXRB ||
294+
Inst.getOpcode() == AArch64::STLXPX ||
295+
Inst.getOpcode() == AArch64::STLXPW ||
296+
Inst.getOpcode() == AArch64::STLXRX ||
297+
Inst.getOpcode() == AArch64::STLXRW ||
298+
Inst.getOpcode() == AArch64::STLXRH ||
299+
Inst.getOpcode() == AArch64::STLXRB ||
300+
Inst.getOpcode() == AArch64::CLREX);
301+
}
302+
275303
bool isLoadFromStack(const MCInst &Inst) const {
276304
if (!mayLoad(Inst))
277305
return false;
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// This test checks that the foo function having exclusive memory access
2+
// instructions won't be instrumented.
3+
4+
// REQUIRES: system-linux,bolt-runtime,target=aarch64{{.*}}
5+
6+
// RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
7+
// RUN: %s -o %t.o
8+
// RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe -nostdlib -Wl,-q -Wl,-fini=dummy
9+
// RUN: llvm-bolt %t.exe -o %t.bolt -instrument -v=1 | FileCheck %s
10+
11+
// CHECK: Function foo has exclusive instructions, skip instrumentation
12+
13+
.global foo
14+
.type foo, %function
15+
foo:
16+
ldaxr w9, [x10]
17+
cbnz w9, .Lret
18+
stlxr w12, w11, [x9]
19+
cbz w12, foo
20+
clrex
21+
.Lret:
22+
ret
23+
.size foo, .-foo
24+
25+
.global _start
26+
.type _start, %function
27+
_start:
28+
cmp x0, #0
29+
b.eq .Lexit
30+
bl foo
31+
.Lexit:
32+
ret
33+
.size _start, .-_start
34+
35+
.global dummy
36+
.type dummy, %function
37+
dummy:
38+
ret
39+
.size dummy, .-dummy

0 commit comments

Comments
 (0)