Merge pull request #3423 from 9rnsr/fix11777

[REG2.065] Issue 11777 - [ICE] dmd memory corruption as `Scope::pop` `free`s `fieldinit` used also in `enclosing`
dlang · Apr 5, 2014 · fe52711 · fe52711
2 parents 497b168 + 684b84b
commit fe52711
Show file tree

Hide file tree

Showing 11 changed files with 62 additions and 28 deletions.
diff --git a/src/attrib.c b/src/attrib.c
@@ -92,7 +92,7 @@ void AttribDeclaration::setScopeNewSc(Scope *sc,
             structalign != sc->structalign)
         {
             // create new one for changes
-            newsc = new Scope(*sc);
+            newsc = sc->copy();
             newsc->flags &= ~SCOPEfree;
             newsc->stc = stc;
             newsc->linkage = linkage;
@@ -127,7 +127,7 @@ void AttribDeclaration::semanticNewSc(Scope *sc,
             structalign != sc->structalign)
         {
             // create new one for changes
-            newsc = new Scope(*sc);
+            newsc = sc->copy();
             newsc->flags &= ~SCOPEfree;
             newsc->stc = stc;
             newsc->linkage = linkage;
@@ -677,7 +677,7 @@ void ProtDeclaration::importAll(Scope *sc)
            sc->explicitProtection != 1)
         {
            // create new one for changes
-           newsc = new Scope(*sc);
+           newsc = sc->copy();
            newsc->flags &= ~SCOPEfree;
            newsc->protection = protection;
            newsc->explicitProtection = 1;

diff --git a/src/class.c b/src/class.c
@@ -393,7 +393,7 @@ void ClassDeclaration::semantic(Scope *sc)
                     //error("forward reference of base class %s", baseClass->toChars());
                     // Forward reference of base class, try again later
                     //printf("\ttry later, forward reference of base class %s\n", tc->sym->toChars());
-                    scope = scx ? scx : new Scope(*sc);
+                    scope = scx ? scx : sc->copy();
                     scope->setNoFree();
                     if (tc->sym->scope)
                         tc->sym->scope->module->addDeferredSemantic(tc->sym);
@@ -457,7 +457,7 @@ void ClassDeclaration::semantic(Scope *sc)
                 //error("forward reference of base class %s", baseClass->toChars());
                 // Forward reference of base, try again later
                 //printf("\ttry later, forward reference of base %s\n", baseClass->toChars());
-                scope = scx ? scx : new Scope(*sc);
+                scope = scx ? scx : sc->copy();
                 scope->setNoFree();
                 if (tc->sym->scope)
                     tc->sym->scope->module->addDeferredSemantic(tc->sym);
@@ -676,7 +676,7 @@ void ClassDeclaration::semantic(Scope *sc)
 
         sc = sc->pop();
 
-        scope = scx ? scx : new Scope(*sc);
+        scope = scx ? scx : sc->copy();
         scope->setNoFree();
         scope->module->addDeferredSemantic(this);
 
@@ -1377,7 +1377,7 @@ void InterfaceDeclaration::semantic(Scope *sc)
                 //error("forward reference of base class %s", baseClass->toChars());
                 // Forward reference of base, try again later
                 //printf("\ttry later, forward reference of base %s\n", b->base->toChars());
-                scope = scx ? scx : new Scope(*sc);
+                scope = scx ? scx : sc->copy();
                 scope->setNoFree();
                 scope->module->addDeferredSemantic(this);
                 return;

diff --git a/src/declaration.c b/src/declaration.c
@@ -1429,7 +1429,7 @@ void VarDeclaration::semantic(Scope *sc)
         }
         else if (parent->isAggregateDeclaration())
         {
-            scope = scx ? scx : new Scope(*sc);
+            scope = scx ? scx : sc->copy();
             scope->setNoFree();
         }
         else if (storage_class & (STCconst | STCimmutable | STCmanifest) ||
@@ -1497,7 +1497,7 @@ void VarDeclaration::semantic(Scope *sc)
             }
             else
             {
-                scope = scx ? scx : new Scope(*sc);
+                scope = scx ? scx : sc->copy();
                 scope->setNoFree();
             }
         }

diff --git a/src/enum.c b/src/enum.c
@@ -171,7 +171,7 @@ void EnumDeclaration::semantic(Scope *sc)
             if (!sym->memtype || !sym->members || !sym->symtab || sym->scope)
             {
                 // memtype is forward referenced, so try again later
-                scope = scx ? scx : new Scope(*sc);
+                scope = scx ? scx : sc->copy();
                 scope->setNoFree();
                 scope->module->addDeferredSemantic(this);
                 Module::dprogress = dprogress_save;

diff --git a/src/expression.c b/src/expression.c
@@ -5629,7 +5629,8 @@ Expression *FuncExp::semantic(Scope *sc)
         {
             if (fd->type && fd->type->ty == Tfunction && !fd->type->nextOf())
                 ((TypeFunction *)fd->type)->next = Type::terror;
-            return new ErrorExp();
+            e = new ErrorExp();
+            goto Ldone;
         }
 
         // Type is a "delegate to" or "pointer to" the function literal

diff --git a/src/func.c b/src/func.c
@@ -1158,7 +1158,7 @@ void FuncDeclaration::semantic(Scope *sc)
     /* Save scope for possible later use (if we need the
      * function internals)
      */
-    scope = new Scope(*sc);
+    scope = sc->copy();
     scope->setNoFree();
 
     static bool printedMain = false;  // semantic might run more than once
@@ -1555,12 +1555,14 @@ void FuncDeclaration::semantic3(Scope *sc)
             sc2 = sc2->push(sym);
 
             AggregateDeclaration *ad2 = isAggregateMember2();
+            unsigned *fieldinit = NULL;
 
             /* If this is a class constructor
              */
             if (ad2 && isCtorDeclaration())
             {
-                sc2->fieldinit = (unsigned *)mem.malloc(sizeof(unsigned) * ad2->fields.dim);
+                fieldinit = (unsigned *)mem.malloc(sizeof(unsigned) * ad2->fields.dim);
+                sc2->fieldinit = fieldinit;
                 sc2->fieldinit_dim = ad2->fields.dim;
                 for (size_t i = 0; i < ad2->fields.dim; i++)
                 {
@@ -1596,7 +1598,8 @@ void FuncDeclaration::semantic3(Scope *sc)
             assert(type == f);
 
             if (isStaticCtorDeclaration())
-            {   /* It's a static constructor. Ensure that all
+            {
+                /* It's a static constructor. Ensure that all
                  * ctor consts were initialized.
                  */
 
@@ -1609,16 +1612,16 @@ void FuncDeclaration::semantic3(Scope *sc)
                 else
                 {
                     for (size_t i = 0; i < pd->members->dim; i++)
-                    {   Dsymbol *s = (*pd->members)[i];
-
+                    {
+                        Dsymbol *s = (*pd->members)[i];
                         s->checkCtorConstInit();
                     }
                 }
             }
 
             if (fbody->isErrorStatement())
                 ;
-            else if (isCtorDeclaration() && ad2)
+            else if (ad2 && isCtorDeclaration())
             {
                 ClassDeclaration *cd = ad2->isClassDeclaration();
 
@@ -1654,7 +1657,6 @@ void FuncDeclaration::semantic3(Scope *sc)
                         }
                     }
                 }
-                mem.free(sc2->fieldinit);
                 sc2->fieldinit = NULL;
                 sc2->fieldinit_dim = 0;
 
@@ -1704,7 +1706,8 @@ void FuncDeclaration::semantic3(Scope *sc)
                 }
             }
             else if (fes)
-            {   // For foreach(){} body, append a return 0;
+            {
+                // For foreach(){} body, append a return 0;
                 Expression *e = new IntegerExp(0);
                 Statement *s = new ReturnStatement(Loc(), e);
                 fbody = new CompoundStatement(Loc(), fbody, s);
@@ -1733,11 +1736,13 @@ void FuncDeclaration::semantic3(Scope *sc)
                 if (type->nextOf()->ty != Tvoid)
                 {
                     if (offend)
-                    {   Expression *e;
+                    {
+                        Expression *e;
                         error("no return exp; or assert(0); at end of function");
                         if (global.params.useAssert &&
                             !global.params.useInline)
-                        {   /* Add an assert(0, msg); where the missing return
+                        {
+                            /* Add an assert(0, msg); where the missing return
                              * should be.
                              */
                             e = new AssertExp(
@@ -1756,6 +1761,8 @@ void FuncDeclaration::semantic3(Scope *sc)
                 }
             }
 
+            if (fieldinit)
+                mem.free(fieldinit);
             sc2 = sc2->pop();
         }
 

diff --git a/src/scope.c b/src/scope.c
@@ -141,6 +141,17 @@ Scope::Scope(Scope *enclosing)
     assert(this != enclosing);
 }
 
+Scope *Scope::copy()
+{
+    Scope *sc = new Scope(*this);
+
+    /* Bugzilla 11777: The copied scope should not inherit fieldinit.
+     */
+    sc->fieldinit = NULL;
+
+    return sc;
+}
+
 Scope *Scope::createGlobal(Module *module)
 {
     Scope *sc;
@@ -189,13 +200,12 @@ Scope *Scope::pop()
         enclosing->callSuper |= callSuper;
         if (enclosing->fieldinit && fieldinit)
         {
+            assert(fieldinit != enclosing->fieldinit);
+
             size_t dim = fieldinit_dim;
             for (size_t i = 0; i < dim; i++)
                 enclosing->fieldinit[i] |= fieldinit[i];
-            /* Workaround regression @@@BUG11777@@@.
-            Probably this memory is used in future.
             mem.free(fieldinit);
-            */
             fieldinit = NULL;
         }
     }

diff --git a/src/scope.h b/src/scope.h
@@ -121,6 +121,8 @@ struct Scope
     Scope();
     Scope(Scope *enclosing);
 
+    Scope *copy();
+
     Scope *push();
     Scope *push(ScopeDsymbol *ss);
     Scope *pop();

diff --git a/src/struct.c b/src/struct.c
@@ -775,7 +775,7 @@ void StructDeclaration::semantic(Scope *sc)
         alignsize = 0;
 //        structalign = 0;
 
-        scope = scx ? scx : new Scope(*sc);
+        scope = scx ? scx : sc->copy();
         scope->setNoFree();
         scope->module->addDeferredSemantic(this);
 

diff --git a/src/template.c b/src/template.c
@@ -533,7 +533,7 @@ void TemplateDeclaration::semantic(Scope *sc)
      */
     if (!this->scope)
     {
-        this->scope = new Scope(*sc);
+        this->scope = sc->copy();
         this->scope->setNoFree();
     }
 
@@ -7773,7 +7773,7 @@ void TemplateMixin::semantic(Scope *sc)
             {
                 // Forward reference
                 //printf("forward reference - deferring\n");
-                scope = scx ? scx : new Scope(*sc);
+                scope = scx ? scx : sc->copy();
                 scope->setNoFree();
                 scope->module->addDeferredSemantic(this);
             }
@@ -7978,7 +7978,7 @@ void TemplateMixin::semantic(Scope *sc)
         {
             // Forward reference
             //printf("forward reference - deferring\n");
-            scope = scx ? scx : new Scope(*sc);
+            scope = scx ? scx : sc->copy();
             scope->setNoFree();
             scope->module->addDeferredSemantic(this);
         }

diff --git a/test/compilable/ice11777.d b/test/compilable/ice11777.d
@@ -0,0 +1,14 @@
+void f(void delegate(int)) {}
+
+class C
+{
+    int i;
+    this()
+    {
+        f((a){});
+        /* (a){} is a template lambda, so FuncExp::semantic -> TemplateDeclaration::semantic
+         * will save the scope in TemplateDeclaration::scope with fieldinit. Later push/pop
+         * of the scope for template lambda body semantics will violate the assertion in Scope::pop().
+         */
+    }
+}