Skip to content

Commit

Permalink
Merge pull request #4033 from MartinNowak/fix12979
Browse files Browse the repository at this point in the history
fix Issue 12979 - Nothrow violation error is hidden by inline assembler
  • Loading branch information
9rnsr committed Oct 6, 2014
2 parents d66e62d + ea3237b commit a1ec5c7
Show file tree
Hide file tree
Showing 15 changed files with 217 additions and 17 deletions.
9 changes: 3 additions & 6 deletions src/func.c
Expand Up @@ -1776,24 +1776,21 @@ void FuncDeclaration::semantic3(Scope *sc)
}
else if (!hasReturnExp && f->next->ty != Tvoid)
error("has no return statement, but is expected to return a value of type %s", f->next->toChars());
else if (hasReturnExp & 8) // if inline asm
{
flags &= ~FUNCFLAGnothrowInprocess;
}
else
{
// Check for errors related to 'nothrow'.
unsigned int nothrowErrors = global.errors;
int blockexit = fbody->blockExit(this, f->isnothrow);
if (f->isnothrow && (global.errors != nothrowErrors) )
if (f->isnothrow && (global.errors != nothrowErrors))
::error(loc, "%s '%s' is nothrow yet may throw", kind(), toPrettyChars());
if (flags & FUNCFLAGnothrowInprocess)
{
if (type == f) f = (TypeFunction *)f->copy();
f->isnothrow = !(blockexit & BEthrow);
}

if ((blockexit & BEfallthru) && f->next->ty != Tvoid)
const bool inlineAsm = hasReturnExp & 8;
if ((blockexit & BEfallthru) && f->next->ty != Tvoid && !inlineAsm)
{
Expression *e;
error("no return exp; or assert(0); at end of function");
Expand Down
5 changes: 0 additions & 5 deletions src/iasm.c
Expand Up @@ -4616,10 +4616,6 @@ Statement* asmSemantic(AsmStatement *s, Scope *sc)
{
//printf("AsmStatement::semantic()\n");

assert(sc->func);
if (sc->func->setUnsafe())
s->error("inline assembler not allowed in @safe function %s", sc->func->toChars());

OP *o;
OPND *o1 = NULL,*o2 = NULL, *o3 = NULL, *o4 = NULL;
PTRNTAB ptb;
Expand Down Expand Up @@ -4806,4 +4802,3 @@ Statement* asmSemantic(AsmStatement *s, Scope *sc)
}

#endif

6 changes: 5 additions & 1 deletion src/parse.c
Expand Up @@ -5397,6 +5397,10 @@ Statement *Parser::parseStatement(int flags, const utf8_t** endPtr)
Loc labelloc;

nextToken();
StorageClass stc = parsePostfix(STCundefined, NULL);
if (stc & (STCconst | STCimmutable | STCshared | STCwild))
error("const/immutable/shared/inout attributes are not allowed on asm blocks");

check(TOKlcurly);
Token *toklist = NULL;
Token **ptoklist = &toklist;
Expand Down Expand Up @@ -5477,7 +5481,7 @@ Statement *Parser::parseStatement(int flags, const utf8_t** endPtr)
}
break;
}
s = new CompoundStatement(loc, statements);
s = new CompoundAsmStatement(loc, statements, stc);
nextToken();
break;
}
Expand Down
56 changes: 52 additions & 4 deletions src/statement.c
Expand Up @@ -689,12 +689,14 @@ int Statement::blockExit(FuncDeclaration *func, bool mustNotThrow)
result = s->statement ? s->statement->blockExit(func, mustNotThrow) : BEfallthru;
}

void visit(AsmStatement *s)
void visit(CompoundAsmStatement *s)
{
if (mustNotThrow)
s->error("asm statements are assumed to throw", s->toChars());
if (mustNotThrow && !(s->stc & STCnothrow))
s->deprecation("asm statement is assumed to throw - mark it with 'nothrow' if it does not");

// Assume the worst
result = BEfallthru | BEthrow | BEreturn | BEgoto | BEhalt;
result = BEfallthru | BEreturn | BEgoto | BEhalt;
if (!(s->stc & STCnothrow)) result |= BEthrow;
}

void visit(ImportStatement *s)
Expand Down Expand Up @@ -5027,6 +5029,52 @@ Statement *AsmStatement::syntaxCopy()
return new AsmStatement(loc, tokens);
}

/************************ CompoundAsmStatement ***************************************/

CompoundAsmStatement::CompoundAsmStatement(Loc loc, Statements *s, StorageClass stc)
: CompoundStatement(loc, s)
{
this->stc = stc;
}

CompoundAsmStatement *CompoundAsmStatement::syntaxCopy()
{
Statements *a = new Statements();
a->setDim(statements->dim);
for (size_t i = 0; i < statements->dim; i++)
{
Statement *s = (*statements)[i];
(*a)[i] = s ? s->syntaxCopy() : NULL;
}
return new CompoundAsmStatement(loc, a, stc);
}

Statements *CompoundAsmStatement::flatten(Scope *sc)
{
return NULL;
}

CompoundAsmStatement *CompoundAsmStatement::semantic(Scope *sc)
{
for (size_t i = 0; i < statements->dim; i++)
{
Statement *s = (*statements)[i];
(*statements)[i] = s ? s->semantic(sc) : NULL;
}

assert(sc->func);
// use setImpure/setGC when the deprecation cycle is over
PURE pure;
if (!(stc & STCpure) && (pure = sc->func->isPureBypassingInference()) != PUREimpure && pure != PUREfwdref)
deprecation("asm statement is assumed to be impure - mark it with 'pure' if it is not");
if (!(stc & STCnogc) && sc->func->isNogcBypassingInference())
deprecation("asm statement is assumed to use the GC - mark it with '@nogc' if it does not");
if (!(stc & (STCtrusted|STCsafe)) && sc->func->setUnsafe())
error("asm statement is assumed to be @system - mark it with '@trusted' if it is not");

return this;
}

/************************ ImportStatement ***************************************/

ImportStatement::ImportStatement(Loc loc, Dsymbols *imports)
Expand Down
14 changes: 14 additions & 0 deletions src/statement.h
Expand Up @@ -747,6 +747,20 @@ class AsmStatement : public Statement
void accept(Visitor *v) { v->visit(this); }
};

// a complete asm {} block
class CompoundAsmStatement : public CompoundStatement
{
public:
StorageClass stc; // postfix attributes like nothrow/pure/@trusted

CompoundAsmStatement(Loc loc, Statements *s, StorageClass stc);
CompoundAsmStatement *syntaxCopy();
CompoundAsmStatement *semantic(Scope *sc);
Statements *flatten(Scope *sc);

void accept(Visitor *v) { v->visit(this); }
};

class ImportStatement : public Statement
{
public:
Expand Down
2 changes: 2 additions & 0 deletions src/visitor.h
Expand Up @@ -52,6 +52,7 @@ class DebugStatement;
class GotoStatement;
class LabelStatement;
class AsmStatement;
class CompoundAsmStatement;
class ImportStatement;

class Type;
Expand Down Expand Up @@ -338,6 +339,7 @@ class Visitor
virtual void visit(GotoStatement *s) { visit((Statement *)s); }
virtual void visit(LabelStatement *s) { visit((Statement *)s); }
virtual void visit(AsmStatement *s) { visit((Statement *)s); }
virtual void visit(CompoundAsmStatement *s) { visit((CompoundStatement *)s); }
virtual void visit(ImportStatement *s) { visit((Statement *)s); }

virtual void visit(Type *) { assert(0); }
Expand Down
17 changes: 17 additions & 0 deletions test/compilable/deprecate12979a.d
@@ -0,0 +1,17 @@
// REQUIRED_ARGS: -dw
// PERMUTE_ARGS:

/*
TEST_OUTPUT:
---
compilable/deprecate12979a.d(13): Deprecation: asm statement is assumed to throw - mark it with 'nothrow' if it does not
---
*/

void foo() nothrow
{
asm
{
ret;
}
}
5 changes: 5 additions & 0 deletions test/compilable/test12979a.d
@@ -0,0 +1,5 @@
void parse()
{
asm pure nothrow @nogc @trusted {}
asm @safe {}
}
34 changes: 34 additions & 0 deletions test/compilable/test12979b.d
@@ -0,0 +1,34 @@
// REQUIRED_ARGS: -w -de

void foo() pure nothrow @nogc @safe
{
asm pure nothrow @nogc @trusted
{
ret;
}
}

void bar()()
{
asm pure nothrow @nogc @trusted
{
ret;
}
}

static assert(__traits(compiles, () pure nothrow @nogc @safe => bar()));

void baz()()
{
asm
{
ret;
}
}

// wait for deprecation of asm pure inference
// static assert(!__traits(compiles, () pure => baz()));
static assert(!__traits(compiles, () nothrow => baz()));
// wait for deprecation of asm @nogc inference
// static assert(!__traits(compiles, () @nogc => baz()));
static assert(!__traits(compiles, () @safe => baz()));
18 changes: 18 additions & 0 deletions test/fail_compilation/deprecate12979a.d
@@ -0,0 +1,18 @@
// REQUIRED_ARGS: -de
// PERMUTE_ARGS:

/*
TEST_OUTPUT:
---
fail_compilation/deprecate12979a.d(14): Deprecation: asm statement is assumed to throw - mark it with 'nothrow' if it does not
fail_compilation/deprecate12979a.d(12): Error: function 'deprecate12979a.foo' is nothrow yet may throw
---
*/

void foo() nothrow
{
asm
{
ret;
}
}
17 changes: 17 additions & 0 deletions test/fail_compilation/deprecate12979b.d
@@ -0,0 +1,17 @@
// REQUIRED_ARGS: -de
// PERMUTE_ARGS:

/*
TEST_OUTPUT:
---
fail_compilation/deprecate12979b.d(13): Deprecation: asm statement is assumed to be impure - mark it with 'pure' if it is not
---
*/

void foo() pure
{
asm
{
ret;
}
}
17 changes: 17 additions & 0 deletions test/fail_compilation/deprecate12979c.d
@@ -0,0 +1,17 @@
// REQUIRED_ARGS: -de
// PERMUTE_ARGS:

/*
TEST_OUTPUT:
---
fail_compilation/deprecate12979c.d(13): Deprecation: asm statement is assumed to use the GC - mark it with '@nogc' if it does not
---
*/

void foo() @nogc
{
asm
{
ret;
}
}
16 changes: 16 additions & 0 deletions test/fail_compilation/deprecate12979d.d
@@ -0,0 +1,16 @@
// PERMUTE_ARGS:

/*
TEST_OUTPUT:
---
fail_compilation/deprecate12979d.d(12): Error: asm statement is assumed to be @system - mark it with '@trusted' if it is not
---
*/

void foo() @safe
{
asm
{
ret;
}
}
2 changes: 1 addition & 1 deletion test/fail_compilation/fail327.d
@@ -1,7 +1,7 @@
/*
TEST_OUTPUT:
---
fail_compilation/fail327.d(10): Error: inline assembler not allowed in @safe function foo
fail_compilation/fail327.d(10): Error: asm statement is assumed to be @system - mark it with '@trusted' if it is not
---
*/

Expand Down
16 changes: 16 additions & 0 deletions test/fail_compilation/test12979.d
@@ -0,0 +1,16 @@
// PERMUTE_ARGS:

/*
TEST_OUTPUT:
---
fail_compilation/test12979.d(13): Error: const/immutable/shared/inout attributes are not allowed on asm blocks
---
*/

void foo()
{
asm const shared
{
ret;
}
}

0 comments on commit a1ec5c7

Please sign in to comment.