Skip to content

Commit

Permalink
Merge pull request #2905 from 9rnsr/fix11653
Browse files Browse the repository at this point in the history
Issue 11653 - No error when forgetting break with range cases
  • Loading branch information
MartinNowak committed Dec 1, 2013
2 parents b382519 + a805219 commit c8773c1
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/arraytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ typedef Array<struct File> Files;

typedef Array<class CaseStatement> CaseStatements;

typedef Array<class CompoundStatement> CompoundStatements;
typedef Array<class ScopeStatement> ScopeStatements;

typedef Array<class GotoCaseStatement> GotoCaseStatements;

Expand Down
73 changes: 41 additions & 32 deletions src/statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -595,25 +595,24 @@ Statement *CompoundStatement::syntaxCopy()


Statement *CompoundStatement::semantic(Scope *sc)
{ Statement *s;

{
//printf("CompoundStatement::semantic(this = %p, sc = %p)\n", this, sc);

#if 0
for (size_t i = 0; i < statements->dim; i++)
{
s = (*statements)[i];
Statement *s = (*statements)[i];
if (s)
printf("[%d]: %s", i, s->toChars());
}
#endif

for (size_t i = 0; i < statements->dim; )
{
s = (*statements)[i];
Statement *s = (*statements)[i];
if (s)
{ Statements *flt = s->flatten(sc);

{
Statements *flt = s->flatten(sc);
if (flt)
{
statements->remove(i);
Expand Down Expand Up @@ -715,12 +714,24 @@ Statement *CompoundStatement::semantic(Scope *sc)
}
for (size_t i = 0; i < statements->dim; ++i)
{
s = (*statements)[i];
if (s)
L1:
Statement *s = (*statements)[i];
if (!s)
continue;

Statement *se = s->isErrorStatement();
if (se)
return se;

/* Bugzilla 11653: 'semantic' may return another CompoundStatement
* (eg. CaseRangeStatement), so flatten it here.
*/
Statements *flt = s->flatten(sc);
if (flt)
{
Statement *se = s->isErrorStatement();
if (se)
return se;
statements->remove(i);
statements->insert(i, flt);
goto L1;

This comment has been minimized.

Copy link
@ibuclaw

ibuclaw Jan 14, 2014

Member

I think this cause a regression. The remove/insert causes the Array to go corrupt in certain scenarios. Deliberately ugly because I was trying to break the compiler. :-)

void main()
{
  try {
      try {
      L1: {}
      }
      finally {
      }
    }
  finally {
    }
  goto L1;
}

gdc error:

cc1d: malloc.c:2369: sysmalloc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.

This comment has been minimized.

Copy link
@ibuclaw

ibuclaw Jan 14, 2014

Member

Another example:

void main()
{
  switch (1) {
    case 1:
      goto L1;
      break;

    default:
      break;
  }
  try {
    L1: { }
  }
  finally {
  }
}

gdc error:

gdc: internal compiler error: Segmentation fault (program cc1d)

This comment has been minimized.

Copy link
@MartinNowak

MartinNowak Jan 17, 2014

Author Member

Please open a bug report (https://d.puremagic.com/issues/), this regression needs to be fixed before the next release.

This comment has been minimized.

Copy link
@9rnsr

9rnsr Jan 18, 2014

Contributor

He was filed the bug as https://d.puremagic.com/issues/show_bug.cgi?id=11925 (and then it is fixed in git-head).

}
}
if (statements->dim == 1)
Expand All @@ -740,7 +751,8 @@ ReturnStatement *CompoundStatement::isReturnStatement()
ReturnStatement *rs = NULL;

for (size_t i = 0; i < statements->dim; i++)
{ Statement *s = (*statements)[i];
{
Statement *s = (*statements)[i];
if (s)
{
rs = s->isReturnStatement();
Expand Down Expand Up @@ -2076,7 +2088,7 @@ Statement *ForeachStatement::semantic(Scope *sc)
}
tfld = new TypeFunction(args, Type::tint32, 0, LINKd);
cases = new Statements();
gotos = new CompoundStatements();
gotos = new ScopeStatements();
FuncLiteralDeclaration *fld = new FuncLiteralDeclaration(loc, Loc(), tfld, TOKdelegate, this);
fld->fbody = body;
Expression *flde = new FuncExp(loc, fld);
Expand All @@ -2085,14 +2097,14 @@ Statement *ForeachStatement::semantic(Scope *sc)

// Resolve any forward referenced goto's
for (size_t i = 0; i < gotos->dim; i++)
{ CompoundStatement *cs = (*gotos)[i];
GotoStatement *gs = (GotoStatement *)(*cs->statements)[0];

{
GotoStatement *gs = (GotoStatement *)(*gotos)[i]->statement;
if (!gs->label->statement)
{ // 'Promote' it to this scope, and replace with a return
{
// 'Promote' it to this scope, and replace with a return
cases->push(gs);
s = new ReturnStatement(Loc(), new IntegerExp(cases->dim + 1));
(*cs->statements)[0] = s;
(*gotos)[i]->statement = s;
}
}

Expand Down Expand Up @@ -5003,7 +5015,8 @@ Statements *DebugStatement::flatten(Scope *sc)
{
Statements *a = statement ? statement->flatten(sc) : NULL;
if (a)
{ for (size_t i = 0; i < a->dim; i++)
{
for (size_t i = 0; i < a->dim; i++)
{ Statement *s = (*a)[i];

s = new DebugStatement(loc, s);
Expand Down Expand Up @@ -5055,17 +5068,13 @@ Statement *GotoStatement::semantic(Scope *sc)
{
/* Either the goto label is forward referenced or it
* is in the function that the enclosing foreach is in.
* Can't know yet, so wrap the goto in a compound statement
* Can't know yet, so wrap the goto in a scope statement
* so we can patch it later, and add it to a 'look at this later'
* list.
*/
Statements *a = new Statements();
CompoundStatement *s;

a->push(this);
s = new CompoundStatement(loc, a);
sc->fes->gotos->push(s); // 'look at this later' list
return s;
ScopeStatement *ss = new ScopeStatement(loc, this);
sc->fes->gotos->push(ss); // 'look at this later' list
return ss;
}

// Add to fwdref list to check later
Expand Down Expand Up @@ -5153,14 +5162,14 @@ Statement *LabelStatement::syntaxCopy()
}

Statement *LabelStatement::semantic(Scope *sc)
{ LabelDsymbol *ls;
{
FuncDeclaration *fd = sc->parent->isFuncDeclaration();

this->lastVar = sc->lastVar;
//printf("LabelStatement::semantic()\n");
ident = fixupLabelName(sc, ident);

ls = fd->searchLabel(ident);
LabelDsymbol *ls = fd->searchLabel(ident);
if (ls->statement)
{
error("Label '%s' already defined", ls->toChars());
Expand Down Expand Up @@ -5198,10 +5207,10 @@ Statements *LabelStatement::flatten(Scope *sc)
{
a->push(new ExpStatement(loc, (Expression *)NULL));
}
Statement *s = (*a)[0];

s = new LabelStatement(loc, ident, s);
(*a)[0] = s;
// reuse 'this' LabelStatement
this->statement = (*a)[0];
(*a)[0] = this;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ class ForeachStatement : public Statement
FuncDeclaration *func; // function we're lexically in

Statements *cases; // put breaks, continues, gotos and returns here
CompoundStatements *gotos; // forward referenced goto's go here
ScopeStatements *gotos; // forward referenced goto's go here

ForeachStatement(Loc loc, TOK op, Parameters *arguments, Expression *aggr, Statement *body);
Statement *syntaxCopy();
Expand Down
24 changes: 24 additions & 0 deletions test/fail_compilation/fail11653.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// REQUIRED_ARGS: -w
/*
TEST_OUTPUT:
---
fail_compilation/fail11653.d(18): Error: switch case fallthrough - use 'goto case;' if intended
---
*/

void main()
{
int test = 12412;
int output = 0;
switch(test)
{
case 1:
output = 1;
//break; //Oops..
case 2: .. case 3:
output = 2;
break;
default:
output = 3;
}
}

0 comments on commit c8773c1

Please sign in to comment.