AccessEnforcement: Fix analysis to include mayReleases as potentially

executing unknown code

This means we have to claw back some performance by recognizing harmless
releases.

Such as releases on types we known don't call a deinit with unknown
side-effects.

rdar://143497196
rdar://143141695

Author: aschwaighofer

include/swift/AST/KnownStdlibTypes.def

@@ -50,6 +50,8 @@ KNOWN_STDLIB_TYPE_DECL(StaticString, NominalTypeDecl, 0)
 KNOWN_STDLIB_TYPE_DECL(Substring, NominalTypeDecl, 0)
 KNOWN_STDLIB_TYPE_DECL(Array, NominalTypeDecl, 1)
 KNOWN_STDLIB_TYPE_DECL(ArraySlice, NominalTypeDecl, 1)
+KNOWN_STDLIB_TYPE_DECL(_ArrayBuffer, NominalTypeDecl, 1)
+KNOWN_STDLIB_TYPE_DECL(_ContiguousArrayBuffer, NominalTypeDecl, 1)
 KNOWN_STDLIB_TYPE_DECL(_ContiguousArrayStorage, ClassDecl, 1)
 KNOWN_STDLIB_TYPE_DECL(Set, NominalTypeDecl, 1)
 KNOWN_STDLIB_TYPE_DECL(Sequence, NominalTypeDecl, 1)

include/swift/SILOptimizer/Analysis/AccessStorageAnalysis.h

@@ -31,6 +31,7 @@
 namespace swift {
 
 class BasicCalleeAnalysis;
+class DestructorAnalysis;
 
 /// Information about a formal access within a function pertaining to a
 /// particular AccessStorage location.
@@ -202,7 +203,7 @@ class AccessStorageResult {
 
   /// Record any access scopes entered by the given single SIL instruction. 'I'
   /// must not be a FullApply; use mergeFromApply instead.
-  void analyzeInstruction(SILInstruction *I);
+  void analyzeInstruction(SILInstruction *I, DestructorAnalysis *DA);
 
   void print(raw_ostream &os) const;
   void dump() const;
@@ -317,8 +318,8 @@ class FunctionAccessStorage {
   /// Analyze the side-effects of a single SIL instruction \p I.
   /// Visited callees are added to \p BottomUpOrder until \p RecursionDepth
   /// reaches MaxRecursionDepth.
-  void analyzeInstruction(SILInstruction *I) {
-    accessResult.analyzeInstruction(I);
+  void analyzeInstruction(SILInstruction *I, DestructorAnalysis *DA) {
+    accessResult.analyzeInstruction(I, DA);
   }
 
   void print(raw_ostream &os) const { accessResult.print(os); }
@@ -380,6 +381,10 @@ class AccessStorageAnalysis : public BottomUpIPAnalysis {
   /// Callee analysis, used for determining the callees at call sites.
   BasicCalleeAnalysis *BCA;
 
+  /// Destructor analysis, used for determined which releases are harmless wrt
+  /// to their side-effects.
+  DestructorAnalysis *DA;
+
 public:
   AccessStorageAnalysis()
       : BottomUpIPAnalysis(SILAnalysisKind::AccessStorage) {}
@@ -412,6 +417,8 @@ class AccessStorageAnalysis : public BottomUpIPAnalysis {
 
   BasicCalleeAnalysis *getBasicCalleeAnalysis() { return BCA; }
 
+  DestructorAnalysis *getDestructorAnalysis() { return DA; }
+
   virtual void initialize(SILPassManager *PM) override;
 
   /// Invalidate all information in this analysis.

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -35,6 +35,7 @@ namespace swift {
 class DominanceInfo;
 class DeadEndBlocks;
 class BasicCalleeAnalysis;
+class DestructorAnalysis;
 template <class T> class NullablePtr;
 
 /// Transform a Use Range (Operand*) into a User Range (SILInstruction *)
@@ -626,6 +627,9 @@ bool findUnreferenceableStorage(StructDecl *decl, SILType structType,
 
 SILValue getInitOfTemporaryAllocStack(AllocStackInst *asi);
 
+bool isDestructorSideEffectFree(SILInstruction *mayRelease,
+                                DestructorAnalysis *DA);
+
 } // end namespace swift
 
 #endif // SWIFT_SILOPTIMIZER_UTILS_INSTOPTUTILS_H

lib/SILOptimizer/Analysis/AccessStorageAnalysis.cpp

@@ -15,7 +15,9 @@
 #include "swift/Basic/Assertions.h"
 #include "swift/SILOptimizer/Analysis/AccessStorageAnalysis.h"
 #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
+#include "swift/SILOptimizer/Analysis/DestructorAnalysis.h"
 #include "swift/SILOptimizer/Analysis/FunctionOrder.h"
+#include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "swift/SILOptimizer/PassManager/PassManager.h"
 
 using namespace swift;
@@ -313,13 +315,16 @@ void AccessStorageResult::visitBeginAccess(B *beginAccess) {
     result.first->mergeFrom(storageAccess);
 }
 
-void AccessStorageResult::analyzeInstruction(SILInstruction *I) {
+void AccessStorageResult::analyzeInstruction(SILInstruction *I,
+                                             DestructorAnalysis *DA) {
   assert(!FullApplySite::isa(I) && "caller should merge");
 
   if (auto *BAI = dyn_cast<BeginAccessInst>(I))
     visitBeginAccess(BAI);
   else if (auto *BUAI = dyn_cast<BeginUnpairedAccessInst>(I))
     visitBeginAccess(BUAI);
+  else if (I->mayRelease() && !isDestructorSideEffectFree(I, DA))
+    setWorstEffects();
 }
 
 void StorageAccessInfo::print(raw_ostream &os) const {
@@ -393,6 +398,7 @@ bool FunctionAccessStorage::summarizeCall(FullApplySite fullApply) {
 void AccessStorageAnalysis::initialize(
     SILPassManager *PM) {
   BCA = PM->getAnalysis<BasicCalleeAnalysis>();
+  DA = PM->getAnalysis<DestructorAnalysis>();
 }
 
 void AccessStorageAnalysis::invalidate() {
@@ -449,7 +455,7 @@ void AccessStorageAnalysis::analyzeFunction(
       if (auto fullApply = FullApplySite::isa(&I))
         analyzeCall(functionInfo, fullApply, bottomUpOrder, recursionDepth);
       else
-        functionInfo->functionEffects.analyzeInstruction(&I);
+        functionInfo->functionEffects.analyzeInstruction(&I, DA);
     }
   }
   LLVM_DEBUG(llvm::dbgs() << "  << finished " << F->getName() << '\n');

lib/SILOptimizer/Analysis/DestructorAnalysis.cpp

@@ -43,6 +43,13 @@ bool DestructorAnalysis::cacheResult(CanType Type, bool Result) {
   return Result;
 }
 
+static bool isKnownSafeStdlibContainerType(CanType Ty) {
+  return Ty->isArray() ||
+    Ty->is_ArrayBuffer() ||
+    Ty->is_ContiguousArrayBuffer() ||
+    Ty->isDictionary();
+}
+
 bool DestructorAnalysis::isSafeType(CanType Ty) {
   // Don't visit types twice.
   auto CachedRes = Cached.find(Ty);
@@ -68,7 +75,8 @@ bool DestructorAnalysis::isSafeType(CanType Ty) {
   //   * or all stored properties are safe types.
   if (auto *Struct = Ty->getStructOrBoundGenericStruct()) {
 
-    if (implementsDestructorSafeContainerProtocol(Struct) &&
+    if ((implementsDestructorSafeContainerProtocol(Struct) ||
+         isKnownSafeStdlibContainerType(Ty)) &&
         areTypeParametersSafe(Ty))
       return cacheResult(Ty, true);
 

lib/SILOptimizer/Transforms/AccessEnforcementOpts.cpp

@@ -88,6 +88,7 @@
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SILOptimizer/Analysis/LoopRegionAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "swift/SILOptimizer/Utils/InstructionDeleter.h"
 #include "swift/SILOptimizer/Utils/OwnershipOptUtils.h"
 #include "llvm/ADT/MapVector.h"
@@ -633,7 +634,7 @@ void AccessConflictAndMergeAnalysis::propagateAccessSetsBottomUp(
           }
           // FIXME: Treat may-release conservatively in the analysis itself by
           // adding a mayRelease flag, in addition to the unidentified flag.
-          accessResult.analyzeInstruction(&instr);
+          accessResult.analyzeInstruction(&instr, ASA->getDestructorAnalysis());
         }
       }
     }
@@ -838,7 +839,8 @@ void AccessConflictAndMergeAnalysis::localDataFlowInBlock(RegionState &state,
       visitFullApply(fullApply, state);
       continue;
     }
-    if (instr.mayRelease()) {
+    if (instr.mayRelease() &&
+        !isDestructorSideEffectFree(&instr, ASA->getDestructorAnalysis())) {
       visitMayRelease(&instr, state);
     }
   }

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -35,6 +35,7 @@
 #include "swift/SILOptimizer/Analysis/ArraySemantic.h"
 #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
+#include "swift/SILOptimizer/Analysis/DestructorAnalysis.h"
 #include "swift/SILOptimizer/OptimizerBridging.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
 #include "swift/SILOptimizer/Utils/DebugOptUtils.h"
@@ -2470,3 +2471,130 @@ SILValue swift::getInitOfTemporaryAllocStack(AllocStackInst *asi) {
     return findRootValueForTupleTempAllocation(asi, state);
   return findRootValueForNonTupleTempAllocation(asi, state);
 }
+
+SILType getTypeOfLoadOfArrayOperandStorage(SILValue val) {
+  // The projection should look something like this:
+  // %29 = struct_element_addr %28 : $*Array<UInt8>, #Array._buffer
+  // %30 = struct_element_addr %29 : $*_ArrayBuffer<UInt8>, #_ArrayBuffer._storage
+  // %31 = struct_element_addr %30 : $*_BridgeStorage<__ContiguousArrayStorageBase>, #_BridgeStorage.rawValue
+  // %32 = load %31 : $*Builtin.BridgeObject
+
+  // We can strip casts and init_existential_ref leading to a load.
+  if (auto initExistRef = dyn_cast<InitExistentialRefInst>(val))
+    val = initExistRef->getOperand();
+  auto ld = dyn_cast<LoadInst>(stripCasts(val));
+  if (!ld)
+    return SILType();
+
+  auto opd = ld->getOperand();
+  auto opdTy = opd->getType();
+  if (opdTy.getObjectType() !=
+      SILType::getBridgeObjectType(opdTy.getASTContext()))
+    return SILType();
+
+  auto bridgedStoragePrj = dyn_cast<StructElementAddrInst>(opd);
+  if (!bridgedStoragePrj)
+    return SILType();
+
+  auto arrayBufferStoragePrj =
+    dyn_cast<StructElementAddrInst>(bridgedStoragePrj->getOperand());
+  if (!arrayBufferStoragePrj)
+    return SILType();
+
+  // If successfull return _ArrayBuffer<UInt8>.
+  return arrayBufferStoragePrj->getOperand()->getType().getObjectType();
+}
+
+static bool isBoxTypeWithoutSideEffectsOnRelease(SILFunction *f,
+                                                 DestructorAnalysis *DA,
+                                                 SILType ty) {
+  auto silBoxedTy = ty.getSILBoxFieldType(f);
+  if (silBoxedTy && !DA->mayStoreToMemoryOnDestruction(silBoxedTy))
+   return true;
+  return false;
+}
+
+
+static bool isReleaseOfClosureWithoutSideffects(SILFunction *f,
+                                                DestructorAnalysis *DA,
+                                                SILValue opd) {
+  auto fnTy = dyn_cast<SILFunctionType>(opd->getType().getASTType());
+  if (!fnTy)
+    return false;
+
+  if (fnTy->isNoEscape() &&
+      fnTy->getRepresentation() == SILFunctionType::Representation::Thick)
+    return true;
+
+  auto pa = dyn_cast<PartialApplyInst>(lookThroughOwnershipInsts(opd));
+  if (!pa)
+    return false;
+
+  // Check that all captured argument types are "trivial".
+  for (auto &opd: pa->getArgumentOperands()) {
+    auto OpdTy = opd.get()->getType().getObjectType();
+    if (!DA->mayStoreToMemoryOnDestruction(OpdTy))
+      continue;
+    if (isBoxTypeWithoutSideEffectsOnRelease(f, DA, OpdTy))
+      continue;
+    return false;
+  }
+
+  return true;
+}
+
+bool swift::isDestructorSideEffectFree(SILInstruction *mayRelease,
+                                       DestructorAnalysis *DA) {
+  switch (mayRelease->getKind()) {
+  case SILInstructionKind::DestroyValueInst:
+  case SILInstructionKind::StrongReleaseInst:
+  case SILInstructionKind::ReleaseValueInst: {
+    auto opd = mayRelease->getOperand(0);
+    auto opdTy = opd->getType();
+    if (!DA->mayStoreToMemoryOnDestruction(opdTy))
+      return true;
+
+    auto arrayTy = getTypeOfLoadOfArrayOperandStorage(opd);
+    if (arrayTy &&  !DA->mayStoreToMemoryOnDestruction(arrayTy))
+      return true;
+
+    if (isReleaseOfClosureWithoutSideffects(mayRelease->getFunction(), DA, opd))
+      return true;
+
+    if (isBoxTypeWithoutSideEffectsOnRelease(mayRelease->getFunction(), DA,
+                                             opdTy))
+      return true;
+
+    return false;
+  }
+  case SILInstructionKind::BuiltinInst: {
+    auto *builtin = cast<BuiltinInst>(mayRelease);
+    switch (builtin->getBuiltinInfo().ID) {
+    case BuiltinValueKind::CopyArray:
+    case BuiltinValueKind::TakeArrayNoAlias:
+    case BuiltinValueKind::TakeArrayFrontToBack:
+    case BuiltinValueKind::TakeArrayBackToFront:
+      return true; // nothing is released, harmless regardless of type
+    case BuiltinValueKind::AssignCopyArrayNoAlias:
+    case BuiltinValueKind::AssignCopyArrayFrontToBack:
+    case BuiltinValueKind::AssignCopyArrayBackToFront:
+    case BuiltinValueKind::AssignTakeArray:
+    case BuiltinValueKind::DestroyArray: {
+      SubstitutionMap substitutions = builtin->getSubstitutions();
+      auto eltTy = substitutions.getReplacementTypes()[0];
+      return !DA->mayStoreToMemoryOnDestruction(
+        builtin->getFunction()->getLoweredType(eltTy));
+      // Only harmless if the array element type destruction is harmless.
+    }
+    default:
+      break;
+    }
+    return false;
+  }
+  // Unhandled instruction.
+  default:
+    return false;
+  }
+
+  return false;
+}

test/SILOptimizer/access_enforcement_opts.sil

@@ -976,17 +976,17 @@ bb0:
 
 
 // public func testOldToNewMapReadMayRelease() {
-// Checks merging 2 out of 3 scopes resulting in a larger read scope
-// Due to a MayRelease instruction before the 3rd scope
+// Checks merging 3 of 3 scopes resulting in a larger read scope
+// There is a MayRelease instruction but we recognize it to be harmless
 //
 // CHECK-LABEL: sil @testOldToNewMapReadMayRelease : $@convention(thin) () -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
 // CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
-// CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-NEXT: strong_release
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [[BEGIN]] : $*X
+// CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-LABEL: } // end sil function 'testOldToNewMapReadMayRelease'
 sil @testOldToNewMapReadMayRelease : $@convention(thin) () -> () {
 bb0:
@@ -1711,3 +1711,32 @@ bb0(%0 : $RefElemNoConflictClass, %1 : $any Error):
   %10 = tuple ()
   return %10 : $()
 }
+
+public class MayHaveDeinit {
+  deinit
+}
+
+sil @releaseArg : $@convention(thin) (@owned MayHaveDeinit) -> () {
+bb0(%0 : $MayHaveDeinit):
+  release_value %0 : $MayHaveDeinit
+  %2 = tuple ()
+  return %2 : $()
+}
+
+sil @testSummarizeFunction : $@convention(method) (@guaranteed TestClass, @owned MayHaveDeinit) -> () {
+bb0(%0 : $TestClass, %1: $MayHaveDeinit):
+  %2 = ref_element_addr %0 : $TestClass, #TestClass.flags
+  %3 = begin_access [modify] [dynamic] %2 : $*Int64
+  %4 = integer_literal $Builtin.Int64, 3
+  %5 = struct $Int64 (%4 : $Builtin.Int64)
+  store %5 to %3 : $*Int64
+  %6 = function_ref @releaseArg : $@convention(thin) (@owned MayHaveDeinit) -> ()
+  %7 = apply %6(%1) : $@convention(thin) (@owned MayHaveDeinit) -> ()
+  end_access %3 : $*Int64
+  %12 = tuple ()
+  return %12 : $()
+}
+
+// CHECK-LABEL: sil @testSummarizeFunction
+// CHECK-NOT: begin_access {{.*}}no_nested_conflict
+// CHECK: } // end sil function 'testSummarizeFunction'

test/SILOptimizer/access_enforcement_opts_ossa.sil

@@ -1006,9 +1006,9 @@ bb0:
 // CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [trivial] [[BEGIN]] : $*X
 // CHECK-NEXT: load [trivial] [[BEGIN]] : $*X
-// CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-NEXT: destroy_value
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [trivial] [[BEGIN]] : $*X
+// CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-LABEL: } // end sil function 'testOldToNewMapReadMayRelease'
 sil [ossa] @testOldToNewMapReadMayRelease : $@convention(thin) () -> () {
 bb0: