From d909b70e789e25397513cf7af634221fad44d2fe Mon Sep 17 00:00:00 2001 From: Nicholas Lindsay Wilson Date: Mon, 1 Jul 2019 14:50:47 +0800 Subject: [PATCH 1/3] add restrictiveshared preview flags. --- src/dmd/cli.d | 2 ++ src/dmd/globals.d | 1 + src/dmd/globals.h | 1 + 3 files changed, 4 insertions(+) diff --git a/src/dmd/cli.d b/src/dmd/cli.d index 973e5ac9c959..c36f5ec50536 100644 --- a/src/dmd/cli.d +++ b/src/dmd/cli.d @@ -721,6 +721,8 @@ dmd -cov -unittest myprog.d "destruct fields of partially constructed objects"), Feature("rvaluerefparam", "rvalueRefParam", "enable rvalue arguments to ref parameters"), + Feature("restrictiveshared", "restrictiveshared", + "implement a more restrictive shared", false), ]; } diff --git a/src/dmd/globals.d b/src/dmd/globals.d index ec0d610710ec..7a13425aa7a1 100644 --- a/src/dmd/globals.d +++ b/src/dmd/globals.d @@ -176,6 +176,7 @@ struct Param // https://issues.dlang.org/show_bug.cgi?id=14246 bool fieldwise; // do struct equality testing field-wise rather than by memcmp() bool rvalueRefParam; // allow rvalues to be arguments to ref parameters + bool restrictiveshared; // restrict the use of shared to casting & shared functions CppStdRevision cplusplus = CppStdRevision.cpp98; // version of C++ standard to support diff --git a/src/dmd/globals.h b/src/dmd/globals.h index 4593128d9541..291872fd9205 100644 --- a/src/dmd/globals.h +++ b/src/dmd/globals.h @@ -148,6 +148,7 @@ struct Param // https://issues.dlang.org/show_bug.cgi?id=14246 bool fieldwise; // do struct equality testing field-wise rather than by memcmp() bool rvalueRefParam; // allow rvalues to be arguments to ref parameters + bool restrictiveshared; // restrict the use of shared to casting & shared functions CppStdRevision cplusplus; // version of C++ name mangling to support bool markdown; // enable Markdown replacements in Ddoc bool vmarkdown; // list instances of Markdown replacements in Ddoc From f0f802ee5a2300ff306920982d0b12fc132b1cc4 Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Sat, 6 Jul 2019 09:51:16 +0200 Subject: [PATCH 2/3] Add preliminary shared protection --- src/dmd/dscope.d | 14 +++++ src/dmd/expressionsem.d | 91 +++++++++++++++++++++++++---- src/dmd/scope.h | 1 + test/compilable/test_shared.d | 22 +++++++ test/fail_compilation/fail_shared.d | 24 ++++++++ 5 files changed, 140 insertions(+), 12 deletions(-) create mode 100644 test/compilable/test_shared.d create mode 100644 test/fail_compilation/fail_shared.d diff --git a/src/dmd/dscope.d b/src/dmd/dscope.d index a0b2dc6cb14f..dd4a62628fc7 100644 --- a/src/dmd/dscope.d +++ b/src/dmd/dscope.d @@ -57,6 +57,7 @@ enum SCOPE ignoresymbolvisibility = 0x0200, /// ignore symbol visibility /// https://issues.dlang.org/show_bug.cgi?id=15907 onlysafeaccess = 0x0400, /// unsafe access is not allowed for @safe code + nosharedcheck = 0x0800, /// allow shared acsses for the sub-expressions free = 0x8000, /// is on free list fullinst = 0x10000, /// fully instantiate templates @@ -273,6 +274,19 @@ struct Scope } + extern (C++) Scope* startRelaxShared() + { + Scope* sc = this.push(); + sc.flags = this.flags | SCOPE.nosharedcheck; + return sc; + } + + extern (C++) Scope* endRelaxShared() + { + assert(flags & SCOPE.nosharedcheck); + return pop(); + } + /******************************* * Merge results of `ctorflow` into `this`. * Params: diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d index 7359e28c9408..91e942830f93 100644 --- a/src/dmd/expressionsem.d +++ b/src/dmd/expressionsem.d @@ -1288,7 +1288,7 @@ private Expression resolvePropertiesX(Scope* sc, Expression e1, Expression e2 = { VarExp ve = cast(VarExp)e1; VarDeclaration v = ve.var.isVarDeclaration(); - if (v && ve.checkPurity(sc, v)) + if (v && (ve.checkPurity(sc, v) || checkAccessShared(sc, ve.loc, v))) return new ErrorExp(); } if (e2) @@ -1605,6 +1605,27 @@ private bool preFunctionParameters(Scope* sc, Expressions* exps) return err; } +/******************************************** + * Issue an error if shared state is Accessed. + * Returns: + * true an error was issued + */ +private bool checkAccessShared(Scope* sc, Loc loc, VarDeclaration vd) +{ + if (!global.params.restrictiveshared) + { + return false; + } + + if (vd.type && vd.type.isShared() && !(sc.flags & SCOPE.nosharedcheck)) + { + loc.error("Trying to Access shared state `%s`", vd.toChars()); + return true; + } + + return false; +} + /******************************************** * Issue an error if default construction is disabled for type t. * Default construction is required for arrays and 'out' parameters. @@ -4011,8 +4032,45 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor return exp.expressionSemantic(sc); } + void checkAccessSharedCall(CallExp exp) + { + assert (exp.f); + auto params = exp.f.getParameterList().parameters; + if (exp.arguments && exp.arguments.dim && params && params.dim) + { + foreach (i, arg;*exp.arguments) + { + if (arg.op == TOK.variable) + { + auto ve = cast (VarExp) arg; + auto vd = ve.var.isVarDeclaration(); + if (vd && + vd.type && + vd.type.isShared() && + i < params.dim && + !(*params)[i].type.isShared) + { + checkAccessShared(sc, arg.loc, vd); + } + } + } + } + } + override void visit(CallExp exp) { + // @@@SHARED@@@ + // we need to set allow shared for overload resolution + sc = sc.startRelaxShared(); + // after resolution is done + scope (exit) + { + sc = sc.endRelaxShared(); + if (exp.f && !exp.f.semantic3Errors + && exp.f.type.isTypeFunction() !is null) + checkAccessSharedCall(exp); + } + static if (LOGSEMANTIC) { printf("CallExp::semantic() %s\n", exp.toChars()); @@ -6798,19 +6856,28 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor return; } - // for static alias this: https://issues.dlang.org/show_bug.cgi?id=17684 - if (exp.e1.op == TOK.type) - exp.e1 = resolveAliasThis(sc, exp.e1); - - auto e1x = resolveProperties(sc, exp.e1); - if (e1x.op == TOK.error) + // shared allowed for resolving this + // @@@SHARED@@@ { - result = e1x; - return; + sc = sc.startRelaxShared(); + + // for static alias this: https://issues.dlang.org/show_bug.cgi?id=17684 + if (exp.e1.op == TOK.type) + exp.e1 = resolveAliasThis(sc, exp.e1); + + auto e1x = resolveProperties(sc, exp.e1); + + if (e1x.op == TOK.error) + { + result = e1x; + return; + } + if (e1x.checkType()) + return setError(); + exp.e1 = e1x; + + sc = sc.endRelaxShared(); } - if (e1x.checkType()) - return setError(); - exp.e1 = e1x; if (!exp.e1.type) { diff --git a/src/dmd/scope.h b/src/dmd/scope.h index 9aba09181245..9df7a6afde7f 100644 --- a/src/dmd/scope.h +++ b/src/dmd/scope.h @@ -41,6 +41,7 @@ class CPPNamespaceDeclaration; #define SCOPEctor 0x0001 // constructor type #define SCOPEcondition 0x0004 // inside static if/assert condition #define SCOPEdebug 0x0008 // inside debug conditional +#define SCOPEnosharedcheck 0x0800 // allow shared access // Flags that would be inherited beyond scope nesting #define SCOPEnoaccesscheck 0x0002 // don't do access checks diff --git a/test/compilable/test_shared.d b/test/compilable/test_shared.d new file mode 100644 index 000000000000..e229c6bf6502 --- /dev/null +++ b/test/compilable/test_shared.d @@ -0,0 +1,22 @@ +/* +REQUIRED_ARGS:-preview=restrictiveshared +TEST_OUTPUT: +--- +--- +*/ + + +int f1(shared int x) +{ + auto r = sync(&x, 22); + return r; + +} + +int sync(shared int *x, int y) +{ + auto unshared_x = cast()x; + + return *unshared_x = y; +} + diff --git a/test/fail_compilation/fail_shared.d b/test/fail_compilation/fail_shared.d new file mode 100644 index 000000000000..1c91fb675afc --- /dev/null +++ b/test/fail_compilation/fail_shared.d @@ -0,0 +1,24 @@ +/* +REQUIRED_ARGS:-preview=restrictiveshared +TEST_OUTPUT: +--- +fail_compilation/fail_shared.d(15): Error: Trying to Access shared state `x` +fail_compilation/fail_shared.d(16): Error: Trying to Access shared state `x` +fail_compilation/fail_shared.d(17): Error: Trying to Access shared state `x` +fail_compilation/fail_shared.d(22): Error: Trying to Access shared state `x` +--- +*/ + + +int f1(shared int x) +{ + x += 7; // Error: Trying to accsess shared state x + x = 22; // Error: Trying to accsess shared state x + return x; // Error: Trying to accsess shared state x +} + +int sync(shared int *x, int y) +{ + return x = y; // Error: Trying to accsess shared state x +} + From b737eb840509c903d161589c6e3718eaf1e413bd Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Fri, 19 Jul 2019 17:51:00 +0200 Subject: [PATCH 3/3] some fixn' --- src/dmd/dscope.d | 10 +++++----- src/dmd/expressionsem.d | 21 +++++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/dmd/dscope.d b/src/dmd/dscope.d index dd4a62628fc7..ddddf3471f3b 100644 --- a/src/dmd/dscope.d +++ b/src/dmd/dscope.d @@ -57,7 +57,7 @@ enum SCOPE ignoresymbolvisibility = 0x0200, /// ignore symbol visibility /// https://issues.dlang.org/show_bug.cgi?id=15907 onlysafeaccess = 0x0400, /// unsafe access is not allowed for @safe code - nosharedcheck = 0x0800, /// allow shared acsses for the sub-expressions + allowsharedaccess = 0x0800, /// allow shared access for the sub-expressions free = 0x8000, /// is on free list fullinst = 0x10000, /// fully instantiate templates @@ -274,16 +274,16 @@ struct Scope } - extern (C++) Scope* startRelaxShared() + extern (C++) Scope* startAllowSharedAccess() { Scope* sc = this.push(); - sc.flags = this.flags | SCOPE.nosharedcheck; + sc.flags = this.flags | SCOPE.allowsharedaccess; return sc; } - extern (C++) Scope* endRelaxShared() + extern (C++) Scope* endAllowSharedAccess() { - assert(flags & SCOPE.nosharedcheck); + assert(flags & SCOPE.allowsharedaccess); return pop(); } diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d index 91e942830f93..38a0a2f97fbe 100644 --- a/src/dmd/expressionsem.d +++ b/src/dmd/expressionsem.d @@ -1617,9 +1617,9 @@ private bool checkAccessShared(Scope* sc, Loc loc, VarDeclaration vd) return false; } - if (vd.type && vd.type.isShared() && !(sc.flags & SCOPE.nosharedcheck)) + if (vd.type && vd.type.isShared() && !(sc.flags & SCOPE.allowsharedaccess)) { - loc.error("Trying to Access shared state `%s`", vd.toChars()); + loc.error("Invalid access to shared data `%s`", vd.toChars()); return true; } @@ -4036,7 +4036,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor { assert (exp.f); auto params = exp.f.getParameterList().parameters; - if (exp.arguments && exp.arguments.dim && params && params.dim) + if (!exp.arguments || !exp.arguments.dim || !params || !params.dim) + return ; + { foreach (i, arg;*exp.arguments) { @@ -4059,13 +4061,16 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor override void visit(CallExp exp) { + // @@@SHARED@@@ - // we need to set allow shared for overload resolution - sc = sc.startRelaxShared(); + // overload resolution calls `resolvePropertiesX` which + // checks for shared access. + // We need to set allow shared for overload resolution + sc = sc.startAllowSharedAccess(); // after resolution is done scope (exit) { - sc = sc.endRelaxShared(); + sc = sc.endAllowSharedAccess(); if (exp.f && !exp.f.semantic3Errors && exp.f.type.isTypeFunction() !is null) checkAccessSharedCall(exp); @@ -6859,7 +6864,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor // shared allowed for resolving this // @@@SHARED@@@ { - sc = sc.startRelaxShared(); + sc = sc.startAllowSharedAccess(); // for static alias this: https://issues.dlang.org/show_bug.cgi?id=17684 if (exp.e1.op == TOK.type) @@ -6876,7 +6881,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor return setError(); exp.e1 = e1x; - sc = sc.endRelaxShared(); + sc = sc.endAllowSharedAccess(); } if (!exp.e1.type)