Merge pull request #37935 from slavapestov/ban-reference-to-uninitialized-box

SILGen: Don't allow referencing the 'var' box before it's fully initialized

Author: slavapestov

lib/SILGen/SILGenDecl.cpp

@@ -290,13 +290,13 @@ class DestroyLocalVariable : public Cleanup {
 namespace {
 /// Cleanup to destroy an uninitialized local variable.
 class DeallocateUninitializedLocalVariable : public Cleanup {
-  VarDecl *Var;
+  SILValue Box;
 public:
-  DeallocateUninitializedLocalVariable(VarDecl *var) : Var(var) {}
+  DeallocateUninitializedLocalVariable(SILValue box) : Box(box) {}
 
   void emit(SILGenFunction &SGF, CleanupLocation l,
             ForUnwind_t forUnwind) override {
-    SGF.deallocateUninitializedLocalVariable(l, Var);
+    SGF.B.createDeallocBox(l, Box);
   }
 
   void dump(SILGenFunction &) const override {
@@ -315,7 +315,12 @@ namespace {
 class LocalVariableInitialization : public SingleBufferInitialization {
   /// The local variable decl being initialized.
   VarDecl *decl;
-  SILGenFunction &SGF;
+
+  /// The alloc_box instruction.
+  SILValue Box;
+
+  /// The projected address.
+  SILValue Addr;
 
   /// The cleanup we pushed to deallocate the local variable before it
   /// gets initialized.
@@ -332,7 +337,7 @@ class LocalVariableInitialization : public SingleBufferInitialization {
   LocalVariableInitialization(VarDecl *decl,
                               Optional<MarkUninitializedInst::Kind> kind,
                               uint16_t ArgNo, SILGenFunction &SGF)
-      : decl(decl), SGF(SGF) {
+      : decl(decl) {
     assert(decl->getDeclContext()->isLocalContext() &&
            "can't emit a local var for a non-local var decl");
     assert(decl->hasStorage() && "can't emit storage for a computed variable");
@@ -348,26 +353,24 @@ class LocalVariableInitialization : public SingleBufferInitialization {
     // The variable may have its lifetime extended by a closure, heap-allocate
     // it using a box.
     SILDebugVariable DbgVar(decl->isLet(), ArgNo);
-    SILValue allocBox = SGF.B.createAllocBox(decl, boxType, DbgVar);
+    Box = SGF.B.createAllocBox(decl, boxType, DbgVar);
 
     // Mark the memory as uninitialized, so DI will track it for us.
     if (kind)
-      allocBox = SGF.B.createMarkUninitialized(decl, allocBox, kind.getValue());
+      Box = SGF.B.createMarkUninitialized(decl, Box, kind.getValue());
 
-    SILValue addr = SGF.B.createProjectBox(decl, allocBox, 0);
-
-    /// Remember that this is the memory location that we're emitting the
-    /// decl to.
-    SGF.VarLocs[decl] = SILGenFunction::VarLoc::get(addr, allocBox);
+    Addr = SGF.B.createProjectBox(decl, Box, 0);
 
     // Push a cleanup to destroy the local variable.  This has to be
     // inactive until the variable is initialized.
     SGF.Cleanups.pushCleanupInState<DestroyLocalVariable>(CleanupState::Dormant,
                                                           decl);
     ReleaseCleanup = SGF.Cleanups.getTopCleanup();
 
-    // Push a cleanup to deallocate the local variable.
-    SGF.Cleanups.pushCleanup<DeallocateUninitializedLocalVariable>(decl);
+    // Push a cleanup to deallocate the local variable. This references the
+    // box directly since it might be activated before we update
+    // SGF.VarLocs.
+    SGF.Cleanups.pushCleanup<DeallocateUninitializedLocalVariable>(Box);
     DeallocCleanup = SGF.Cleanups.getTopCleanup();
   }
 
@@ -376,8 +379,7 @@ class LocalVariableInitialization : public SingleBufferInitialization {
   }
 
   SILValue getAddress() const {
-    assert(SGF.VarLocs.count(decl) && "did not emit var?!");
-    return SGF.VarLocs[decl].value;    
+    return Addr;
   }
 
   SILValue getAddressForInPlaceInitialization(SILGenFunction &SGF,
@@ -394,6 +396,11 @@ class LocalVariableInitialization : public SingleBufferInitialization {
   }
 
   void finishInitialization(SILGenFunction &SGF) override {
+    /// Remember that this is the memory location that we've emitted the
+    /// decl to.
+    assert(SGF.VarLocs.count(decl) == 0 && "Already emitted the local?");
+    SGF.VarLocs[decl] = SILGenFunction::VarLoc::get(Addr, Box);
+
     SingleBufferInitialization::finishInitialization(SGF);
     assert(!DidFinish &&
            "called LocalVariableInitialization::finishInitialization twice!");
@@ -1688,21 +1695,4 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
     B.emitDestroyValueOperation(silLoc, Val);
   else
     B.createDestroyAddr(silLoc, Val);
-}
-
-void SILGenFunction::deallocateUninitializedLocalVariable(SILLocation silLoc,
-                                                          VarDecl *vd) {
-  assert(vd->getDeclContext()->isLocalContext() &&
-         "can't emit a local var for a non-local var decl");
-  assert(vd->hasStorage() && "can't emit storage for a computed variable");
-
-  assert(VarLocs.count(vd) && "var decl wasn't emitted?!");
-
-  auto loc = VarLocs[vd];
-
-  // Ignore let values captured without a memory location.
-  if (!loc.value->getType().isAddress()) return;
-
-  assert(loc.box && "captured var should have been given a box");
-  B.createDeallocBox(silLoc, loc.box);
-}
+}

lib/SILGen/SILGenFunction.h

@@ -2072,9 +2072,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
 
   /// Destroy and deallocate an initialized local variable.
   void destroyLocalVariable(SILLocation L, VarDecl *D);
-  
-  /// Deallocate an uninitialized local variable.
-  void deallocateUninitializedLocalVariable(SILLocation L, VarDecl *D);
 
   /// Enter a cleanup to deallocate a stack variable.
   CleanupHandle enterDeallocStackCleanup(SILValue address);

test/SILGen/capture_order.swift

@@ -165,3 +165,23 @@ class rdar40600800 {
     }
   }
 }
+
+// Make sure we can't capture an uninitialized 'var' box, either.
+func SR14747() {
+  func g() -> Int { // expected-error {{closure captures 'r' before it is declared}}
+    _ = r // expected-note {{captured here}}
+    return 5
+  }
+  var r = g() // expected-note {{captured value declared here}}
+  // expected-warning@-1 {{variable 'r' was never mutated; consider changing to 'let' constant}}
+}
+
+class class77933460 {}
+
+func func77933460() {
+  var obj: class77933460 = { obj }()
+  // expected-error@-1 {{closure captures 'obj' before it is declared}}
+  // expected-note@-2 {{captured here}}
+  // expected-note@-3 {{captured value declared here}}
+  // expected-warning@-4 {{variable 'obj' was never mutated; consider changing to 'let' constant}}
+}