Emit unenforced access markers in SILGen for accesses to local temporaries.

These accesses can't be recognized as obviously local temporaries in the
verification pass, so the only way to exhaustively verify exclusivity is by
added unenforced markers.

SILGen currently only emits unenforced markers under -verify-exlcusivity. Once
opaque values is the only supported SILGen mode, then we should turn the markers
on by default (SILGen should not have different modes of operation).

Author: atrick

lib/SILGen/FormalEvaluation.h

@@ -26,7 +26,7 @@ class LogicalPathComponent;
 
 class FormalAccess {
 public:
-  enum Kind { Shared, Exclusive, Owned };
+  enum Kind { Shared, Exclusive, Owned, Unenforced };
 
 private:
   unsigned allocatedSize;

lib/SILGen/LValue.h

@@ -535,6 +535,62 @@ struct LLVM_LIBRARY_VISIBILITY ExclusiveBorrowFormalAccess : FormalAccess {
   }
 };
 
+struct LLVM_LIBRARY_VISIBILITY UnenforcedAccess {
+  // Make sure someone called `endAccess` before destroying this.
+  struct DeleterCheck {
+    void operator()(BeginAccessInst *) {
+      llvm_unreachable("access scope must be ended");
+    }
+  };
+  typedef std::unique_ptr<BeginAccessInst, DeleterCheck> BeginAccessPtr;
+  BeginAccessPtr beginAccessPtr;
+
+  UnenforcedAccess() = default;
+  UnenforcedAccess(const UnenforcedAccess &other) = delete;
+  UnenforcedAccess(UnenforcedAccess &&other) = default;
+
+  UnenforcedAccess &operator=(const UnenforcedAccess &) = delete;
+  UnenforcedAccess &operator=(UnenforcedAccess &&other) = default;
+
+  // Return the a new begin_access if it was required, otherwise return the
+  // given `address`.
+  SILValue beginAccess(SILGenFunction &SGF, SILLocation loc, SILValue address,
+                       SILAccessKind kind);
+
+  // End the access and release beginAccessPtr.
+  void endAccess(SILGenFunction &SGF);
+
+  // Emit the end_access (on a branch) without marking this access as ended.
+  void emitEndAccess(SILGenFunction &SGF);
+};
+
+/// Pseudo-formal access that emits access markers but does not actually
+/// require enforcement. It may be used for access to formal memory that is
+/// exempt from exclusivity checking, such as initialization, or it may be used
+/// for accesses to local memory that are indistinguishable from formal access
+/// at the SIL level. Adding the access markers in these cases gives SIL address
+/// users a structural property that allows for exhaustive verification.
+struct LLVM_LIBRARY_VISIBILITY UnenforcedFormalAccess : FormalAccess {
+
+  static SILValue enter(SILGenFunction &SGF, SILLocation loc, SILValue address,
+                        SILAccessKind kind);
+
+  // access.beginAccessPtr is either the begin_access or null if no access was
+  // required.
+  UnenforcedAccess access;
+
+  UnenforcedFormalAccess(SILLocation loc, UnenforcedAccess &&access,
+                         CleanupHandle cleanup)
+      : FormalAccess(sizeof(*this), FormalAccess::Unenforced, loc, cleanup),
+        access(std::move(access)) {}
+
+  // Emit the end_access (on a branch) without marking this access as ended.
+  void emitEndAccess(SILGenFunction &SGF);
+
+  // Only called at the end formal evaluation scope. End this access.
+  void finishImpl(SILGenFunction &SGF) override;
+};
+
 } // namespace Lowering
 } // namespace swift
 

lib/SILGen/SILGenDecl.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "Initialization.h"
+#include "LValue.h"
 #include "RValue.h"
 #include "SILGen.h"
 #include "SILGenDynamicCast.h"
@@ -192,15 +193,27 @@ void SingleBufferInitialization::
 copyOrInitValueIntoSingleBuffer(SILGenFunction &SGF, SILLocation loc,
                                 ManagedValue value, bool isInit,
                                 SILValue destAddr) {
+  // Emit an unchecked access around initialization of the local buffer to
+  // silence access marker verification.
+  //
+  // FIXME: This is not a good place for FormalEvaluationScope +
+  // UnenforcedFormalAccess.  However, there's no way to identify the buffer
+  // initialization sequence after SILGen, and no easy way to wrap the
+  // Initialization in an access during top-level expression evaluation.
+  FormalEvaluationScope scope(SGF);
   if (!isInit) {
     assert(value.getValue() != destAddr && "copying in place?!");
-    value.copyInto(SGF, destAddr, loc);
+    SILValue accessAddr =
+      UnenforcedFormalAccess::enter(SGF, loc, destAddr, SILAccessKind::Modify);
+    value.copyInto(SGF, accessAddr, loc);
     return;
   }
 
   // If we didn't evaluate into the initialization buffer, do so now.
   if (value.getValue() != destAddr) {
-    value.forwardInto(SGF, loc, destAddr);
+    SILValue accessAddr =
+      UnenforcedFormalAccess::enter(SGF, loc, destAddr, SILAccessKind::Modify);
+    value.forwardInto(SGF, loc, accessAddr);
   } else {
     // If we did evaluate into the initialization buffer, disable the
     // cleanup.
@@ -828,10 +841,14 @@ void EnumElementPatternInitialization::emitEnumMatch(
           SILValue boxedValue = SGF.B.createProjectBox(loc, mv.getValue(), 0);
           auto &boxedTL = SGF.getTypeLowering(boxedValue->getType());
           // SEMANTIC ARC TODO: Revisit this when the verifier is enabled.
-          if (boxedTL.isLoadable() || !SGF.silConv.useLoweredAddresses())
-            boxedValue = boxedTL.emitLoad(SGF.B, loc, boxedValue,
+          if (boxedTL.isLoadable() || !SGF.silConv.useLoweredAddresses()) {
+            UnenforcedAccess access;
+            SILValue accessAddress =
+              access.beginAccess(SGF, loc, boxedValue, SILAccessKind::Read);
+            boxedValue = boxedTL.emitLoad(SGF.B, loc, accessAddress,
                                           LoadOwnershipQualifier::Take);
-
+            access.endAccess(SGF);
+          }
           // We must treat the boxed value as +0 since it may be shared. Copy it
           // if nontrivial.
           //

lib/SILGen/SILGenExpr.cpp

@@ -966,33 +966,56 @@ emitRValueForDecl(SILLocation loc, ConcreteDeclRef declRef, Type ncRefType,
       if (var->isLet())
         guaranteedValid = true;
 
+      // Protect the lvalue read with access markers. The !is<LValueType> assert
+      // above ensures that the "LValue" is actually immutable, so we use an
+      // unenforced access marker.
+      SILValue destAddr = result.getLValueAddress();
+      SILValue accessAddr = UnenforcedFormalAccess::enter(*this, loc, destAddr,
+                                                          SILAccessKind::Read);
+      auto propagateRValuePastAccess = [&](RValue &&rvalue) {
+        // Check if a new begin_access was emitted and returned as the
+        // RValue. This means that the load did not actually load. If so, then
+        // fix the rvalue to begin_access operand. The end_access cleanup
+        // doesn't change. FIXME: this can't happen with sil-opaque-values.
+        if (accessAddr != destAddr && rvalue.isComplete()
+            && rvalue.isPlusZero(*this) && !isa<TupleType>(rvalue.getType())) {
+          auto mv = std::move(rvalue).getScalarValue();
+          if (mv.getValue() == accessAddr)
+            mv = std::move(mv).transform(
+                cast<BeginAccessInst>(accessAddr)->getOperand());
+          return RValue(*this, loc, refType, mv);
+        }
+        return std::move(rvalue);
+      };
       // If we have self, see if we are in an 'init' delegation sequence. If so,
       // call out to the special delegation init routine. Otherwise, use the
       // normal RValue emission logic.
       if (var->getName() == getASTContext().Id_self &&
           SelfInitDelegationState != NormalSelf) {
-        return emitRValueForSelfInDelegationInit(loc, refType,
-                                                 result.getLValueAddress(), C);
+        auto rvalue =
+            emitRValueForSelfInDelegationInit(loc, refType, accessAddr, C);
+        return propagateRValuePastAccess(std::move(rvalue));
       }
 
       // Avoid computing an abstraction pattern for local variables.
       // This is a slight compile-time optimization, but more importantly
       // it avoids problems where locals don't always have interface types.
       if (var->getDeclContext()->isLocalContext()) {
-        return RValue(*this, loc, refType,
-                      emitLoad(loc, result.getLValueAddress(),
-                               getTypeLowering(refType), C, shouldTake,
-                               guaranteedValid));
+        auto rvalue = RValue(*this, loc, refType,
+                             emitLoad(loc, accessAddr, getTypeLowering(refType),
+                                      C, shouldTake, guaranteedValid));
+
+        return propagateRValuePastAccess(std::move(rvalue));
       }
 
       // Otherwise, do the full thing where we potentially bridge and
       // reabstract the declaration.
       auto origFormalType = SGM.Types.getAbstractionPattern(var);
-      return RValue(*this, loc, refType,
-                    emitLoad(loc, result.getLValueAddress(),
-                             origFormalType, refType,
-                             getTypeLowering(refType), C, shouldTake,
-                             guaranteedValid));
+      auto rvalue = RValue(*this, loc, refType,
+                           emitLoad(loc, accessAddr, origFormalType, refType,
+                                    getTypeLowering(refType), C, shouldTake,
+                                    guaranteedValid));
+      return propagateRValuePastAccess(std::move(rvalue));
     }
 
     // For local decls, use the address we allocated or the value if we have it.
@@ -1276,14 +1299,21 @@ RValue SILGenFunction::emitRValueForStorageLoad(
       result = result.copyUnmanaged(*this, loc);
     }
   } else {
+    // Create a tiny unenforced access scope around a load from local memory. No
+    // cleanup is necessary since we directly emit the load here. This will
+    // probably go away with opaque values.
+    UnenforcedAccess access;
+    SILValue accessAddress =
+      access.beginAccess(*this, loc, base.getValue(), SILAccessKind::Read);
+
     // For address-only sequences, the base is in memory.  Emit a
     // struct_element_addr to get to the field, and then load the element as an
     // rvalue.
-    SILValue ElementPtr =
-      B.createStructElementAddr(loc, base.getValue(), field);
+    SILValue ElementPtr = B.createStructElementAddr(loc, accessAddress, field);
 
     result = emitLoad(loc, ElementPtr, abstractedTL,
                       hasAbstractionChange ? SGFContext() : C, IsNotTake);
+    access.endAccess(*this);
   }
 
   // If we're accessing this member with an abstraction change, perform that

lib/SILGen/SILGenLValue.cpp

@@ -24,6 +24,7 @@
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/AST/DiagnosticsCommon.h"
 #include "swift/AST/GenericEnvironment.h"
+#include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/PrettyStackTrace.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILUndef.h"
@@ -52,7 +53,7 @@ struct LValueWritebackCleanup : Cleanup {
   void dump(SILGenFunction &) const override {
 #ifndef NDEBUG
     llvm::errs() << "LValueWritebackCleanup\n"
-                 << "State: " << getState() << "Depth: " << Depth.getDepth()
+                 << "State: " << getState() << " Depth: " << Depth.getDepth()
                  << "\n";
 #endif
   }
@@ -266,6 +267,13 @@ ManagedValue LogicalPathComponent::getMaterialized(SILGenFunction &SGF,
 
   ManagedValue temp = std::move(*this).materializeIntoTemporary(SGF, loc, base);
 
+  if (SGF.getOptions().VerifyExclusivity) {
+    // Begin an access of the temporary. It is unenforced because enforcement
+    // isn't required for RValues.
+    SILValue accessAddress = UnenforcedFormalAccess::enter(
+        SGF, loc, temp.getValue(), SILAccessKind::Modify);
+    temp = std::move(temp).transform(accessAddress);
+  }
   // Push a writeback for the temporary.
   pushWriteback(SGF, loc, std::move(clonedComponent), base,
                 MaterializedLValue(temp));
@@ -475,6 +483,93 @@ static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
   return addr;
 }
 
+// Find the base of the formal access at `address`. If the base requires an
+// access marker, then create a begin_access on `address`. Return the
+// address to be used for the access.
+//
+// FIXME: In order to generate more consistent and verifiable SIL patterns, or
+// subobject projections, create the access on the base address and recreate the
+// projection.
+SILValue UnenforcedAccess::beginAccess(SILGenFunction &SGF, SILLocation loc,
+                                       SILValue address, SILAccessKind kind) {
+  if (!SGF.getOptions().VerifyExclusivity)
+    return address;
+
+  SILValue base = findAccessedAddressBase(address);
+  if (!base || !isPossibleFormalAccessBase(base))
+    return address;
+
+  auto BAI =
+      SGF.B.createBeginAccess(loc, address, kind, SILAccessEnforcement::Unsafe);
+  beginAccessPtr = BeginAccessPtr(BAI, DeleterCheck());
+
+  return BAI;
+}
+
+void UnenforcedAccess::endAccess(SILGenFunction &SGF) {
+  emitEndAccess(SGF);
+  beginAccessPtr.release();
+}
+
+void UnenforcedAccess::emitEndAccess(SILGenFunction &SGF) {
+  if (!beginAccessPtr)
+    return;
+
+  SGF.B.createEndAccess(beginAccessPtr->getLoc(), beginAccessPtr.get(),
+                        /*abort*/ false);
+}
+
+// Emit an end_access marker when executing a cleanup (on a side branch).
+void UnenforcedFormalAccess::emitEndAccess(SILGenFunction &SGF) {
+  access.emitEndAccess(SGF);
+}
+
+// End the access when existing the FormalEvaluationScope.
+void UnenforcedFormalAccess::finishImpl(SILGenFunction &SGF) {
+  access.endAccess(SGF);
+}
+
+namespace {
+struct UnenforcedAccessCleanup : Cleanup {
+  FormalEvaluationContext::stable_iterator Depth;
+
+  UnenforcedAccessCleanup() : Depth() {}
+
+  void emit(SILGenFunction &SGF, CleanupLocation loc) override {
+    auto &evaluation = *SGF.FormalEvalContext.find(Depth);
+    assert(evaluation.getKind() == FormalAccess::Unenforced);
+    auto &formalAccess = static_cast<UnenforcedFormalAccess &>(evaluation);
+    formalAccess.emitEndAccess(SGF);
+  }
+
+  void dump(SILGenFunction &) const override {
+#ifndef NDEBUG
+    llvm::errs() << "UnenforcedAccessCleanup\n"
+                 << "State: " << getState() << " Depth: " << Depth.getDepth()
+                 << "\n";
+#endif
+  }
+};
+} // end anonymous namespace
+
+SILValue UnenforcedFormalAccess::enter(SILGenFunction &SGF, SILLocation loc,
+                                       SILValue address, SILAccessKind kind) {
+  assert(SGF.InFormalEvaluationScope);
+
+  UnenforcedAccess access;
+  SILValue accessAddress = access.beginAccess(SGF, loc, address, kind);
+  if (!access.beginAccessPtr)
+    return address;
+
+  auto &cleanup = SGF.Cleanups.pushCleanup<UnenforcedAccessCleanup>();
+  CleanupHandle handle = SGF.Cleanups.getTopCleanup();
+  auto &context = SGF.FormalEvalContext;
+  context.push<UnenforcedFormalAccess>(loc, std::move(access), handle);
+  cleanup.Depth = context.stable_begin();
+
+  return accessAddress;
+}
+
 namespace {
   class RefElementComponent : public PhysicalPathComponent {
     VarDecl *Field;
@@ -1259,6 +1354,7 @@ namespace {
         // indirectly.
         SILValue baseAddress;
         SILValue baseMetatype;
+        UnenforcedAccess baseAccess;
         if (base) {
           if (base.getType().isAddress()) {
             baseAddress = base.getValue();
@@ -1269,6 +1365,10 @@ namespace {
                                             baseFormalType);
 
             baseAddress = SGF.emitTemporaryAllocation(loc, base.getType());
+            // Create an unenforced formal access for the temporary base, which
+            // is passed @inout to the callback.
+            baseAddress = baseAccess.beginAccess(SGF, loc, baseAddress,
+                                                 SILAccessKind::Modify);
             if (base.getOwnershipKind() == ValueOwnershipKind::Guaranteed) {
               SGF.B.createStoreBorrow(loc, base.getValue(), baseAddress);
             } else {
@@ -1298,6 +1398,9 @@ namespace {
                             baseAddress,
                             baseMetatype
                           }, false);
+
+        if (baseAccess.beginAccessPtr)
+          baseAccess.endAccess(SGF);
       }
 
       // Continue.

lib/SILGen/SILGenPattern.cpp

@@ -14,6 +14,7 @@
 #include "Cleanup.h"
 #include "ExitableFullExpr.h"
 #include "Initialization.h"
+#include "LValue.h"
 #include "RValue.h"
 #include "SILGen.h"
 #include "Scope.h"
@@ -2156,9 +2157,13 @@ void PatternMatchEmission::emitEnumElementDispatch(
         SILValue boxedValue = SGF.B.createProjectBox(loc, origCMV.getValue(), 0);
         eltTL = &SGF.getTypeLowering(boxedValue->getType());
         if (eltTL->isLoadable()) {
+          UnenforcedAccess access;
+          SILValue accessAddress =
+            access.beginAccess(SGF, loc, boxedValue, SILAccessKind::Read);
           ManagedValue newLoadedBoxValue = SGF.B.createLoadBorrow(
-              loc, ManagedValue::forUnmanaged(boxedValue));
+            loc, ManagedValue::forUnmanaged(accessAddress));
           boxedValue = newLoadedBoxValue.getUnmanagedValue();
+          access.endAccess(SGF);
         }
 
         // The boxed value may be shared, so we always have to copy it.