Skip to content

Commit 3d5c496

Browse files
atrickatrick
committed
Safely implement strict TBAA rules.
This fixes type punning issues with unsafeBitCast. The optimizer is still too aggressive with UnsafePointer. To fix that, we first need an explicit API for circumventing type safety (rdar://23406272). I should be able to fix the following regressions by migrating the stdlib away from unsafeBitCast to unsafeReferenceCast (~2 weeks). Slowdowns: |.Benchmark.................|..Before.|...After.|.Speedup| |.ArrayInClass..............|...49.00.|...78.00.|.-37.2%.| |.Sim2DArray................|..471.00.|..549.00.|.-14.2%.| |.PrimeNum..................|.1876.00.|.1980.00.|..-5.3%.| Speedups: |.Benchmark.................|..Before.|...After.|.Speedup| |.HeapSort..................|.2962.00.|.2663.00.|..11.2%.| |.StdlibSort................|.2672.00.|.2537.00.|...5.3%.|
1 parent d326c8e commit 3d5c496

File tree

8 files changed

+142
-62
lines changed

8 files changed

+142
-62
lines changed

Diff for: ‎include/swift/SILAnalysis/AliasAnalysis.h

+3-4
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,9 @@ class AliasAnalysis : public SILAnalysis {
157157
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
158158
AliasAnalysis::AliasResult R);
159159

160-
/// Look at the origin/user ValueBase of V to see if any of them are
161-
/// TypedAccessOracle which enable one to ascertain via undefined behavior the
162-
/// "true" type of the instruction.
163-
SILType findTypedAccessType(SILValue V);
160+
/// If this value is an address that obeys strict TBAA, return the address type.
161+
/// Otherwise, return an empty type.
162+
SILType computeTBAAType(SILValue V);
164163

165164
/// Check if V points to a let-variable.
166165
bool isLetPointer(SILValue V);

Diff for: ‎lib/SIL/SILValue.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,9 @@ SILValue SILValue::stripCasts() {
9898
V = stripSinglePredecessorArgs(V);
9999

100100
auto K = V->getKind();
101-
if (isRCIdentityPreservingCast(K) ||
102-
K == ValueKind::UncheckedTrivialBitCastInst) {
101+
if (isRCIdentityPreservingCast(K)
102+
|| K == ValueKind::UncheckedTrivialBitCastInst
103+
|| K == ValueKind::MarkDependenceInst) {
103104
V = cast<SILInstruction>(V.getDef())->getOperand(0);
104105
continue;
105106
}

Diff for: ‎lib/SIL/Verifier.cpp

+20
Original file line numberDiff line numberDiff line change
@@ -2759,6 +2759,26 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
27592759
return true;
27602760
}),
27612761
"entry point argument types do not match function type");
2762+
2763+
2764+
// TBAA requirement for all address arguments.
2765+
require(std::equal(entry->bbarg_begin(), entry->bbarg_end(),
2766+
ti->getParameters().begin(),
2767+
[&](SILArgument *bbarg, SILParameterInfo paramInfo) {
2768+
if (!bbarg->getType().isAddress())
2769+
return true;
2770+
switch (paramInfo.getConvention()) {
2771+
default:
2772+
return false;
2773+
case ParameterConvention::Indirect_In:
2774+
case ParameterConvention::Indirect_Inout:
2775+
case ParameterConvention::Indirect_Out:
2776+
case ParameterConvention::Indirect_In_Guaranteed:
2777+
return true;
2778+
}
2779+
}),
2780+
"entry point address argument must have a nonaliasing calling "
2781+
"convention");
27622782
}
27632783

27642784
void verifyEpilogBlocks(SILFunction *F) {

Diff for: ‎lib/SILAnalysis/AliasAnalysis.cpp

+77
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,83 @@ AliasAnalysis::aliasAddressProjection(SILValue V1, SILValue V2,
344344
// TBAA
345345
//===----------------------------------------------------------------------===//
346346

347+
/// Is this an instruction that can act as a type "oracle" allowing typed access
348+
/// TBAA to know what the real types associated with the SILInstruction are.
349+
static bool isTypedAccessOracle(SILInstruction *I) {
350+
switch (I->getKind()) {
351+
case ValueKind::RefElementAddrInst:
352+
case ValueKind::StructElementAddrInst:
353+
case ValueKind::TupleElementAddrInst:
354+
case ValueKind::UncheckedTakeEnumDataAddrInst:
355+
case ValueKind::LoadInst:
356+
case ValueKind::StoreInst:
357+
case ValueKind::AllocStackInst:
358+
case ValueKind::AllocBoxInst:
359+
case ValueKind::DeallocStackInst:
360+
case ValueKind::DeallocBoxInst:
361+
return true;
362+
default:
363+
return false;
364+
}
365+
}
366+
367+
/// Return true if the given value is an instruction or block argument that is
368+
/// known to produce a nonaliasing address with respect to TBAA rules (i.e. the
369+
/// pointer is not type punned). The only way to produce an aliasing typed
370+
/// address is with pointer_to_address (via UnsafePointer) or
371+
/// unchecked_addr_cast (via Builtin.reinterpretCast). Consequently, if the
372+
/// given value is directly derived from a memory location, it cannot
373+
/// alias. Call arguments also cannot alias because they must follow @in, @out,
374+
/// @inout, or @in_guaranteed conventions.
375+
///
376+
/// FIXME: pointer_to_address should contain a flag that indicates whether the
377+
/// address is aliasing. Currently, we aggressively assume that
378+
/// pointer-to-address is never used for type punning, which is not yet
379+
/// clearly specified by our UnsafePointer API.
380+
static bool isAddressRootTBAASafe(SILValue V) {
381+
if (auto *Arg = dyn_cast<SILArgument>(V))
382+
return Arg->isFunctionArg();
383+
384+
switch (V->getKind()) {
385+
default:
386+
return false;
387+
case ValueKind::AllocStackInst:
388+
case ValueKind::AllocBoxInst:
389+
case ValueKind::PointerToAddressInst:
390+
return true;
391+
}
392+
}
393+
394+
/// Look at the origin/user ValueBase of V to see if any of them are
395+
/// TypedAccessOracle which enable one to ascertain via undefined behavior the
396+
/// "true" type of the instruction.
397+
static SILType findTypedAccessType(SILValue V) {
398+
// First look at the origin of V and see if we have any instruction that is a
399+
// typed oracle.
400+
if (auto *I = dyn_cast<SILInstruction>(V))
401+
if (isTypedAccessOracle(I))
402+
return V.getType();
403+
404+
// Then look at any uses of V that potentially could act as a typed access
405+
// oracle.
406+
for (auto Use : V.getUses())
407+
if (isTypedAccessOracle(Use->getUser()))
408+
return V.getType();
409+
410+
// Otherwise return an empty SILType
411+
return SILType();
412+
}
413+
414+
SILType swift::computeTBAAType(SILValue V) {
415+
if (isAddressRootTBAASafe(getUnderlyingObject(V)))
416+
return findTypedAccessType(V);
417+
418+
// FIXME: add ref_element_addr check here. TBAA says that objects cannot be
419+
// type punned.
420+
421+
return SILType();
422+
}
423+
347424
static bool typedAccessTBAABuiltinTypesMayAlias(SILType LTy, SILType RTy,
348425
SILModule &Mod) {
349426
assert(LTy != RTy && "LTy should have already been shown to not equal RTy to "

Diff for: ‎lib/SILAnalysis/MemoryBehavior.cpp

+7-51
Original file line numberDiff line numberDiff line change
@@ -20,50 +20,6 @@
2020

2121
using namespace swift;
2222

23-
//===----------------------------------------------------------------------===//
24-
// TBAA Utilities
25-
//===----------------------------------------------------------------------===//
26-
27-
/// Is this an instruction that can act as a type "oracle" allowing typed access
28-
/// TBAA to know what the real types associated with the SILInstruction are.
29-
static bool isTypedAccessOracle(SILInstruction *I) {
30-
switch (I->getKind()) {
31-
case ValueKind::RefElementAddrInst:
32-
case ValueKind::StructElementAddrInst:
33-
case ValueKind::TupleElementAddrInst:
34-
case ValueKind::UncheckedTakeEnumDataAddrInst:
35-
case ValueKind::LoadInst:
36-
case ValueKind::StoreInst:
37-
case ValueKind::AllocStackInst:
38-
case ValueKind::AllocBoxInst:
39-
case ValueKind::DeallocStackInst:
40-
case ValueKind::DeallocBoxInst:
41-
return true;
42-
default:
43-
return false;
44-
}
45-
}
46-
47-
/// Look at the origin/user ValueBase of V to see if any of them are
48-
/// TypedAccessOracle which enable one to ascertain via undefined behavior the
49-
/// "true" type of the instruction.
50-
SILType swift::findTypedAccessType(SILValue V) {
51-
// First look at the origin of V and see if we have any instruction that is a
52-
// typed oracle.
53-
if (auto *I = dyn_cast<SILInstruction>(V))
54-
if (isTypedAccessOracle(I))
55-
return V.getType();
56-
57-
// Then look at any uses of V that potentially could act as a typed access
58-
// oracle.
59-
for (auto Use : V.getUses())
60-
if (isTypedAccessOracle(Use->getUser()))
61-
return V.getType();
62-
63-
// Otherwise return an empty SILType
64-
return SILType();
65-
}
66-
6723
//===----------------------------------------------------------------------===//
6824
// Memory Behavior Implementation
6925
//===----------------------------------------------------------------------===//
@@ -96,9 +52,9 @@ class MemoryBehaviorVisitor
9652
RetainObserveKind IgnoreRefCountIncs)
9753
: AA(AA), V(V), IgnoreRefCountIncrements(IgnoreRefCountIncs) {}
9854

99-
SILType getTypedAccessType() {
55+
SILType getValueTBAAType() {
10056
if (!TypedAccessTy)
101-
TypedAccessTy = findTypedAccessType(V);
57+
TypedAccessTy = computeTBAAType(V);
10258
return *TypedAccessTy;
10359
}
10460

@@ -190,8 +146,8 @@ class MemoryBehaviorVisitor
190146
} // end anonymous namespace
191147

192148
MemBehavior MemoryBehaviorVisitor::visitLoadInst(LoadInst *LI) {
193-
if (AA.isNoAlias(LI->getOperand(), V, LI->getOperand().getType(),
194-
getTypedAccessType())) {
149+
if (AA.isNoAlias(LI->getOperand(), V, computeTBAAType(LI->getOperand()),
150+
getValueTBAAType())) {
195151
DEBUG(llvm::dbgs() << " Load Operand does not alias inst. Returning "
196152
"None.\n");
197153
return MemBehavior::None;
@@ -210,8 +166,8 @@ MemBehavior MemoryBehaviorVisitor::visitStoreInst(StoreInst *SI) {
210166

211167
// If the store dest cannot alias the pointer in question, then the
212168
// specified value can not be modified by the store.
213-
if (AA.isNoAlias(SI->getDest(), V, SI->getDest().getType(),
214-
getTypedAccessType())) {
169+
if (AA.isNoAlias(SI->getDest(), V, computeTBAAType(SI->getDest()),
170+
getValueTBAAType())) {
215171
DEBUG(llvm::dbgs() << " Store Dst does not alias inst. Returning "
216172
"None.\n");
217173
return MemBehavior::None;
@@ -292,7 +248,7 @@ MemBehavior MemoryBehaviorVisitor::visitApplyInst(ApplyInst *AI) {
292248
SILValue Arg = AI->getArgument(Idx);
293249
// We only consider the argument effects if the argument aliases V.
294250
if (!Arg.getType().isAddress() ||
295-
!AA.isNoAlias(Arg, V, Arg.getType(), getTypedAccessType())) {
251+
!AA.isNoAlias(Arg, V, computeTBAAType(Arg), getValueTBAAType())) {
296252
Behavior = ArgBehavior;
297253
}
298254
}

Diff for: ‎lib/SILAnalysis/ValueTracking.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ using namespace swift::PatternMatch;
2525
/// Strip off casts/indexing insts/address projections from V until there is
2626
/// nothing left to strip.
2727
/// FIXME: Maybe put this on SILValue?
28+
/// FIXME: Why don't we strip projections after stripping indexes?
2829
SILValue swift::getUnderlyingObject(SILValue V) {
2930
while (true) {
3031
SILValue V2 = V.stripCasts().stripAddressProjections().stripIndexingInsts();

Diff for: ‎lib/SILPasses/UtilityPasses/AADumper.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class SILAADumper : public SILFunctionTransform {
7676
auto V2 = Values[i2];
7777

7878
auto Result =
79-
AA->alias(V1, V2, findTypedAccessType(V1), findTypedAccessType(V2));
79+
AA->alias(V1, V2, computeTBAAType(V1), computeTBAAType(V2));
8080

8181
// Results should always be the same. But if they are different print it
8282
// out so we find the error. This should make our test results less

Diff for: ‎test/SILAnalysis/typed-access-tb-aa.sil

+30-4
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,9 @@ sil @baz_init : $@convention(thin) (@thick baz.Type) -> @owned baz
3333
sil @goo_init : $@convention(thin) (@thick goo.Type) -> @owned goo
3434

3535
// CHECK-LABEL: no_parent_child_relation_reftype_tests
36-
37-
// CHECK: PAIR #54.
38-
// CHECK: (0): %4 = apply %2(%3) : $@convention(thin) (@thick boo.Type) -> @owned boo
39-
// CHECK: (0): %8 = apply %6(%7) : $@convention(thin) (@thick baz.Type) -> @owned baz
36+
// CHECK: PAIR #13.
37+
// CHECK: (1): %0 = alloc_stack $boo
38+
// CHECK: (1): %1 = alloc_stack $baz
4039
// CHECK: NoAlias
4140
sil hidden @no_parent_child_relation_reftype_tests : $@convention(thin) () -> () {
4241
bb0:
@@ -900,3 +899,30 @@ sil @tuple_tests : $@convention(thin) () -> () {
900899
return %9999 : $()
901900
}
902901

902+
// Check that TBAA fails when the root is unchecked_addr_cast or
903+
// pointer_to_address.
904+
//
905+
// CHECK-LABEL: @tbaa_dump
906+
// CHECK: PAIR #2.
907+
// CHECK-NEXT: (0): %0 = argument of bb0 : $*Builtin.Int64
908+
// CHECK-NEXT: (0): %3 = unchecked_addr_cast %0 : $*Builtin.Int64 to $*Builtin.Int32
909+
// CHECK-NEXT: MayAlias
910+
// CHECK: PAIR #5.
911+
// CHECK-NEXT: (0): %0 = argument of bb0 : $*Builtin.Int64
912+
// CHECK-NEXT: (0): %7 = pointer_to_address %6 : $Builtin.RawPointer to $*Builtin.Int32
913+
// FIXME: This should be MayAlias unless the pointer_to_address instruction
914+
// comes with a "must not alias" flag.
915+
// CHECK-NEXT: NoAlias
916+
sil @tbaa_dump : $@convention(thin) (@in Builtin.Int64) -> Builtin.Int64 {
917+
bb0(%0 : $*Builtin.Int64):
918+
%1 = integer_literal $Builtin.Int64, 42
919+
store %1 to %0 : $*Builtin.Int64
920+
%3 = unchecked_addr_cast %0 : $*Builtin.Int64 to $*Builtin.Int32
921+
%4 = integer_literal $Builtin.Int32, 0
922+
store %4 to %3 : $*Builtin.Int32
923+
%6 = address_to_pointer %0 : $*Builtin.Int64 to $Builtin.RawPointer
924+
%7 = pointer_to_address %6 : $Builtin.RawPointer to $*Builtin.Int32
925+
store %4 to %7 : $*Builtin.Int32
926+
%8 = load %0 : $*Builtin.Int64
927+
return %8 : $Builtin.Int64
928+
}

0 commit comments

Comments
 (0)