fix Issue 14730 - Wrong closure var access with -inline

dlang · Feb 7, 2016 · 6f0f6db · 6f0f6db
1 parent b583111
commit 6f0f6db
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 72 deletions.
diff --git a/src/declaration.h b/src/declaration.h
@@ -594,6 +594,9 @@ class FuncDeclaration : public Declaration
 
     unsigned flags;                     // FUNCFLAGxxxxx
 
+    // toObjFile() these nested functions after this one
+    FuncDeclarations deferredNested;
+
     FuncDeclaration(Loc loc, Loc endloc, Identifier *id, StorageClass storage_class, Type *type);
     Dsymbol *syntaxCopy(Dsymbol *);
     void semantic(Scope *sc);
@@ -837,9 +840,6 @@ class UnitTestDeclaration : public FuncDeclaration
 public:
     char *codedoc; /** For documented unittest. */
 
-    // toObjFile() these nested functions after this one
-    FuncDeclarations deferredNested;
-
     UnitTestDeclaration(Loc loc, Loc endloc, StorageClass stc, char *codedoc);
     Dsymbol *syntaxCopy(Dsymbol *);
     void semantic(Scope *sc);

diff --git a/src/func.d b/src/func.d
@@ -486,6 +486,9 @@ public:
 
     uint flags;                         // FUNCFLAGxxxxx
 
+    // toObjFile() these nested functions after this one
+    FuncDeclarations deferredNested;
+
     final extern (D) this(Loc loc, Loc endloc, Identifier id, StorageClass storage_class, Type type)
     {
         super(id);
@@ -5202,9 +5205,6 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration
 public:
     char* codedoc;      // for documented unittest
 
-    // toObjFile() these nested functions after this one
-    FuncDeclarations deferredNested;
-
     extern (D) this(Loc loc, Loc endloc, StorageClass stc, char* codedoc)
     {
         super(loc, endloc, unitTestId(loc), stc, null);

diff --git a/src/toir.c b/src/toir.c
@@ -671,7 +671,7 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
          *        ~this() { call destructor }
          *    }
          */
-        //printf("FuncDeclaration::buildClosure() %s\n", toChars());
+        //printf("FuncDeclaration::buildClosure() %s\n", fd->toChars());
 
         /* Generate type name for closure struct */
         const char *name1 = "CLOSURE.";
@@ -690,64 +690,12 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
         symbol_add(sclosure);
         irs->sclosure = sclosure;
 
-        unsigned offset = Target::ptrsize;      // leave room for previous sthis
+        assert(fd->closureVars.dim);
+        assert(fd->closureVars[0]->offset >= Target::ptrsize);  // leave room for previous sthis
         for (size_t i = 0; i < fd->closureVars.dim; i++)
         {
             VarDeclaration *v = fd->closureVars[i];
             //printf("closure var %s\n", v->toChars());
-            assert(v->isVarDeclaration());
-
-            if (v->needsAutoDtor())
-            {
-                /* Because the value needs to survive the end of the scope!
-                 */
-                v->error("has scoped destruction, cannot build closure");
-            }
-            if (v->isargptr)
-            {
-                /* See Bugzilla 2479
-                 * This is actually a bug, but better to produce a nice
-                 * message at compile time rather than memory corruption at runtime
-                 */
-                v->error("cannot reference variadic arguments from closure");
-            }
-            /* Align and allocate space for v in the closure
-             * just like AggregateDeclaration::addField() does.
-             */
-            unsigned memsize;
-            unsigned memalignsize;
-            structalign_t xalign;
-            if (v->storage_class & STClazy)
-            {
-                /* Lazy variables are really delegates,
-                 * so give same answers that TypeDelegate would
-                 */
-                memsize = Target::ptrsize * 2;
-                memalignsize = memsize;
-                xalign = STRUCTALIGN_DEFAULT;
-            }
-            else if (ISWIN64REF(v))
-            {
-                memsize = v->type->size();
-                memalignsize = v->type->alignsize();
-                xalign = v->alignment;
-            }
-            else if (ISREF(v, NULL))
-            {
-                // reference parameters are just pointers
-                memsize = Target::ptrsize;
-                memalignsize = memsize;
-                xalign = STRUCTALIGN_DEFAULT;
-            }
-            else
-            {
-                memsize = v->type->size();
-                memalignsize = v->type->alignsize();
-                xalign = v->alignment;
-            }
-            AggregateDeclaration::alignmember(xalign, memalignsize, &offset);
-            v->offset = offset;
-            offset += memsize;
 
             /* Set Sscope to closure */
             Symbol *vsym = toSymbol(v);
@@ -757,20 +705,23 @@ void buildClosure(FuncDeclaration *fd, IRState *irs)
             /* Add variable as closure type member */
             symbol_struct_addField(Closstru->Ttag, vsym->Sident, vsym->Stype, v->offset);
             //printf("closure field %s: memalignsize: %i, offset: %i\n", vsym->Sident, memalignsize, v->offset);
-
-            /* Can't do nrvo if the variable is put in a closure, since
-             * what the shidden points to may no longer exist.
-             */
-            if (fd->nrvo_can && fd->nrvo_var == v)
-            {
-                fd->nrvo_can = 0;
-            }
         }
-        // offset is now the size of the closure
-        Closstru->Ttag->Sstruct->Sstructsize = offset;
+
+        // Calculate the size of the closure
+        VarDeclaration *vlast = fd->closureVars[fd->closureVars.dim - 1];
+        unsigned structsize;
+        if (vlast->storage_class & STClazy)
+            structsize = vlast->offset + Target::ptrsize * 2;
+        else if (vlast->isRef() || vlast->isOut())
+            structsize = vlast->offset + Target::ptrsize;
+        else
+            structsize = vlast->offset + vlast->type->size();
+        //printf("structsize = %d\n", structsize);
+
+        Closstru->Ttag->Sstruct->Sstructsize = structsize;
 
         // Allocate memory for the closure
-        elem *e = el_long(TYsize_t, offset);
+        elem *e = el_long(TYsize_t, structsize);
         e = el_bin(OPcall, TYnptr, el_var(getRtlsym(RTLSYM_ALLOCMEMORY)), e);
         toTraceGC(irs, e, &fd->loc);
 

diff --git a/test/runnable/closure.d b/test/runnable/closure.d
@@ -897,6 +897,27 @@ void test12406()
     }
 }
 
+/************************************/
+// 14730
+
+void test14730()
+{
+    static auto makeS(int x)
+    {
+        struct S
+        {
+            int n;
+            int get() { return x; }     // x will be a closure variable
+        }
+        return S(x);
+    }
+    auto s = makeS(1);
+    assert(s.get() == 1);
+    // By inlining get() function call, it's rewritten to:
+    // assert(*(s.tupleof[$-1] + x.offset) == 1);
+    // --> In DotVarExp::toElem(), x->offset should be already nonzero.
+}
+
 /************************************/
 
 int main()
@@ -929,6 +950,7 @@ int main()
     test9685a();
     test9685b();
     test12406();
+    test14730();
 
     printf("Success\n");
     return 0;