averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

SILOptimier: Fix a miscompile in COWArrayOpt.

Browse Source 
If a make_mutable operation is done conditionally in a loop, the hoisting of this operation can cause an over-release of the array buffer in some cases.

rdar://problem/48906146
This commit is contained in:
Erik Eckstein
2019-03-18 14:56:22 -07:00
parent de65d30ef3
commit c2a8c71191
3 changed files with 91 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp
View File

@@ -410,8 +410,7 @@ class COWArrayOpt {
// analyzing.
SILValue CurrentArrayAddr;
public:
COWArrayOpt(RCIdentityFunctionInfo *RCIA, SILLoop *L,
DominanceAnalysis *DA)
COWArrayOpt(RCIdentityFunctionInfo *RCIA, SILLoop *L, DominanceAnalysis *DA)
: RCIA(RCIA), Function(L->getHeader()->getParent()), Loop(L),
Preheader(L->getLoopPreheader()), DomTree(DA->get(Function)),
ColdBlocks(DA), CachedSafeLoop(false, false) {}
@@ -427,7 +426,8 @@ protected:
bool checkSafeArrayValueUses(UserList &ArrayValueUsers);
bool checkSafeArrayElementUse(SILInstruction *UseInst, SILValue ArrayVal);
bool checkSafeElementValueUses(UserOperList &ElementValueUsers);
bool hoistMakeMutable(ArraySemanticsCall MakeMutable);
bool hoistMakeMutable(ArraySemanticsCall MakeMutable, bool dominatesExits);
bool dominatesExitingBlocks(SILBasicBlock *BB);
void hoistAddressProjections(Operand &ArrayOp);
bool hasLoopOnlyDestructorSafeArrayOperations();
SILValue getArrayAddressBase(SILValue V);
@@ -1037,7 +1037,8 @@ void COWArrayOpt::hoistAddressProjections(Operand &ArrayOp) {
/// Check if this call to "make_mutable" is hoistable, and move it, or delete it
/// if it's already hoisted.
bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable) {
bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable,
bool dominatesExits) {
LLVM_DEBUG(llvm::dbgs() << " Checking mutable array: " <<CurrentArrayAddr);
// We can hoist address projections (even if they are only conditionally
@@ -1056,7 +1057,12 @@ bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable) {
// Check whether we can hoist make_mutable based on the operations that are
// in the loop.
if (hasLoopOnlyDestructorSafeArrayOperations()) {
// Note that in this case we don't verify that the array buffer is not aliased
// and therefore we must be conservative if the make_mutable is executed
// conditionally (i.e. doesn't dominate all exit blocks).
// The test SILOptimizer/cowarray_opt.sil: dont_hoist_if_executed_conditionally
// shows the problem.
if (hasLoopOnlyDestructorSafeArrayOperations() && dominatesExits) {
// Done. We can hoist the make_mutable.
// We still need the array uses later to check if we can add loads to
// HoistableLoads.
@@ -1106,6 +1112,16 @@ bool COWArrayOpt::hoistMakeMutable(ArraySemanticsCall MakeMutable) {
return true;
}
bool COWArrayOpt::dominatesExitingBlocks(SILBasicBlock *BB) {
llvm::SmallVector<SILBasicBlock *, 8> ExitingBlocks;
Loop->getExitingBlocks(ExitingBlocks);
for (SILBasicBlock *Exiting : ExitingBlocks) {
if (!DomTree->dominates(BB, Exiting))
return false;
}
return true;
}
bool COWArrayOpt::run() {
LLVM_DEBUG(llvm::dbgs() << " Array Opts in Loop " << *Loop);
@@ -1123,6 +1139,7 @@ bool COWArrayOpt::run() {
for (auto *BB : Loop->getBlocks()) {
if (ColdBlocks.isCold(BB))
continue;
bool dominatesExits = dominatesExitingBlocks(BB);
for (auto II = BB->begin(), IE = BB->end(); II != IE;) {
// Inst may be moved by hoistMakeMutable.
SILInstruction *Inst = &*II;
@@ -1134,7 +1151,7 @@ bool COWArrayOpt::run() {
CurrentArrayAddr = MakeMutableCall.getSelf();
auto HoistedCallEntry = ArrayMakeMutableMap.find(CurrentArrayAddr);
if (HoistedCallEntry == ArrayMakeMutableMap.end()) {
if (!hoistMakeMutable(MakeMutableCall)) {
if (!hoistMakeMutable(MakeMutableCall, dominatesExits)) {
ArrayMakeMutableMap[CurrentArrayAddr] = nullptr;
continue;
}