From c6058f9b764ae59ffe0e7cc47de9c39317028798 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 17 Jun 2024 10:11:52 +0100 Subject: [PATCH] =?UTF-8?q?Fix=20Bugzilla=2017148=20-=20Copying=20from=20c?= =?UTF-8?q?onst(void)[]=20to=20void[]=20breaks=20immu=E2=80=A6=20(#16583)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- changelog/dmd.copying-to-void-arrays.dd | 15 ++++++++++++ compiler/src/dmd/cli.d | 4 ++-- compiler/src/dmd/expressionsem.d | 27 +++++++++++++++++++--- compiler/test/compilable/previewhelp.d | 2 +- compiler/test/fail_compilation/test15660.d | 18 ++++++++++++++- druntime/src/core/lifetime.d | 6 ++--- druntime/src/core/thread/osthread.d | 2 +- druntime/src/core/thread/threadbase.d | 2 +- druntime/src/rt/aaA.d | 4 ++-- druntime/src/rt/lifetime.d | 4 ++-- 10 files changed, 68 insertions(+), 16 deletions(-) create mode 100644 changelog/dmd.copying-to-void-arrays.dd diff --git a/changelog/dmd.copying-to-void-arrays.dd b/changelog/dmd.copying-to-void-arrays.dd new file mode 100644 index 0000000000..e01cac5e2e --- /dev/null +++ b/changelog/dmd.copying-to-void-arrays.dd @@ -0,0 +1,15 @@ +Copying from `const(void)[]` to `void[]` is disallowed with `-preview=fixImmutableConv` + +If `const(void)[]` data contains tail `const` pointers, copying to `void[]` +can subsequently violate `const` data: +--- +void f(int*[] a, const int*[] b) +{ + void[] va = a; + const void[] vb = b; + va[] = vb[]; // fills `a` with pointers to const + *a[0] = 0; // const data mutated +} +--- +Copying `vb` data to `va` is no longer allowed with the +`-preview=fixImmutableConv` switch. diff --git a/compiler/src/dmd/cli.d b/compiler/src/dmd/cli.d index eac6ceb7c5..5e3ca3956d 100644 --- a/compiler/src/dmd/cli.d +++ b/compiler/src/dmd/cli.d @@ -957,8 +957,8 @@ dmd -cov -unittest myprog.d "allow use of => for methods and top-level functions in addition to lambdas", "https://dlang.org/spec/function.html#ShortenedFunctionBody", false, true), Feature("fixImmutableConv", "fixImmutableConv", - "disallow functions with a mutable `void[]` parameter to be strongly pure", - "https://dlang.org/changelog/2.101.0.html#dmd.fix-immutable-conv"), + "disallow `void[]` data from holding immutable data", + "https://dlang.org/changelog/2.101.0.html#dmd.fix-immutable-conv, https://issues.dlang.org/show_bug.cgi?id=17148"), Feature("systemVariables", "systemVariables", "disable access to variables marked '@system' from @safe code", "https://dlang.org/spec/attribute.html#system-variables"), diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index 78fc36190b..1b234a96fc 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -11431,16 +11431,37 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor { if (sc.setUnsafe(false, exp.loc, "cannot copy `%s` to `%s` in `@safe` code", t2, t1)) return setError(); + + if (global.params.fixImmutableConv && !t2.implicitConvTo(t1)) + { + error(exp.loc, "cannot copy `%s` to `%s`", t2.toChars(), t1.toChars()); + errorSupplemental(exp.loc, + "Source data has incompatible type qualifier(s)"); + errorSupplemental(exp.loc, "Use `cast(%s)` to force copy", t1.toChars()); + return setError(); + } } } else { if (exp.e1.op == EXP.slice && (t1.ty == Tarray || t1.ty == Tsarray) && - t1.nextOf().toBasetype().ty == Tvoid && - sc.setUnsafePreview(FeatureState.default_, false, exp.loc, + t1.nextOf().toBasetype().ty == Tvoid) + { + if (sc.setUnsafePreview(FeatureState.default_, false, exp.loc, "cannot copy `%s` to `%s` in `@safe` code", t2, t1)) - return setError(); + return setError(); + + if (global.params.fixImmutableConv && !t2.implicitConvTo(t1)) + { + error(exp.loc, "cannot copy `%s` to `%s`", + t2.toChars(), t1.toChars()); + errorSupplemental(exp.loc, + "Source data has incompatible type qualifier(s)"); + errorSupplemental(exp.loc, "Use `cast(%s)` to force copy", t1.toChars()); + return setError(); + } + } if (exp.op == EXP.blit) e2x = e2x.castTo(sc, exp.e1.type); diff --git a/compiler/test/compilable/previewhelp.d b/compiler/test/compilable/previewhelp.d index c50118a8a2..14d5781964 100644 --- a/compiler/test/compilable/previewhelp.d +++ b/compiler/test/compilable/previewhelp.d @@ -16,7 +16,7 @@ Upcoming language changes listed by -preview=name: =nosharedaccess disable access to shared memory objects (https://dlang.org/spec/const3.html#shared) =in `in` on parameters means `scope const [ref]` and accepts rvalues (https://dlang.org/spec/function.html#in-params) =inclusiveincontracts 'in' contracts of overridden methods must be a superset of parent contract (https://dlang.org/changelog/2.095.0.html#inclusive-incontracts) - =fixImmutableConv disallow functions with a mutable `void[]` parameter to be strongly pure (https://dlang.org/changelog/2.101.0.html#dmd.fix-immutable-conv) + =fixImmutableConv disallow `void[]` data from holding immutable data (https://dlang.org/changelog/2.101.0.html#dmd.fix-immutable-conv, https://issues.dlang.org/show_bug.cgi?id=17148) =systemVariables disable access to variables marked '@system' from @safe code (https://dlang.org/spec/attribute.html#system-variables) ---- */ diff --git a/compiler/test/fail_compilation/test15660.d b/compiler/test/fail_compilation/test15660.d index ae573b2af2..9bf05d1253 100644 --- a/compiler/test/fail_compilation/test15660.d +++ b/compiler/test/fail_compilation/test15660.d @@ -1,7 +1,13 @@ /* REQUIRED_ARGS: -preview=fixImmutableConv TEST_OUTPUT: --- -fail_compilation/test15660.d(20): Error: cannot implicitly convert expression `f(v)` of type `int[]` to `immutable(int[])` +fail_compilation/test15660.d(26): Error: cannot implicitly convert expression `f(v)` of type `int[]` to `immutable(int[])` +fail_compilation/test15660.d(34): Error: cannot copy `const(void)[]` to `void[]` +fail_compilation/test15660.d(34): Source data has incompatible type qualifier(s) +fail_compilation/test15660.d(34): Use `cast(void[])` to force copy +fail_compilation/test15660.d(36): Error: cannot copy `const(int*)[]` to `void[]` +fail_compilation/test15660.d(36): Source data has incompatible type qualifier(s) +fail_compilation/test15660.d(36): Use `cast(void[])` to force copy --- */ @@ -19,3 +25,13 @@ void main() void[] v; immutable x = f(v); } + +// https://issues.dlang.org/show_bug.cgi?id=17148 +void f(int*[] a, const int*[] b) @system +{ + void[] a1 = a; + const(void)[] b1 = b; + a1[] = b1[]; + *a[0] = 0; //modify const data + a1[] = new const(int*)[2]; +} diff --git a/druntime/src/core/lifetime.d b/druntime/src/core/lifetime.d index 9e563ad43a..f3dab3624a 100644 --- a/druntime/src/core/lifetime.d +++ b/druntime/src/core/lifetime.d @@ -104,7 +104,7 @@ T emplace(T, Args...)(T chunk, auto ref Args args) // Initialize the object in its pre-ctor state const initializer = __traits(initSymbol, T); - (() @trusted { (cast(void*) chunk)[0 .. initializer.length] = initializer[]; })(); + () @trusted { (cast(void*) chunk)[0 .. initializer.length] = cast(void[]) initializer[]; }(); static if (isInnerClass!T) { @@ -2683,7 +2683,7 @@ T _d_newThrowable(T)() @trusted debug(PRINTF) printf(" p = %p\n", p); // initialize it - p[0 .. init.length] = init[]; + p[0 .. init.length] = cast(void[]) init[]; import core.internal.traits : hasIndirections; if (hasIndirections!T) @@ -2776,7 +2776,7 @@ if (is(T == class)) } // initialize it - p[0 .. init.length] = init[]; + p[0 .. init.length] = cast(void[]) init[]; debug(PRINTF) printf("initialization done\n"); return cast(T) p; diff --git a/druntime/src/core/thread/osthread.d b/druntime/src/core/thread/osthread.d index 42f28f3458..8b48a943d7 100644 --- a/druntime/src/core/thread/osthread.d +++ b/druntime/src/core/thread/osthread.d @@ -2033,7 +2033,7 @@ extern (C) void thread_init() @nogc nothrow status = sem_init( &suspendCount, 0, 0 ); assert( status == 0 ); } - _mainThreadStore[] = __traits(initSymbol, Thread)[]; + _mainThreadStore[] = cast(void[]) __traits(initSymbol, Thread)[]; Thread.sm_main = attachThread((cast(Thread)_mainThreadStore.ptr).__ctor()); } diff --git a/druntime/src/core/thread/threadbase.d b/druntime/src/core/thread/threadbase.d index cd9f751dfb..118c03987f 100644 --- a/druntime/src/core/thread/threadbase.d +++ b/druntime/src/core/thread/threadbase.d @@ -774,7 +774,7 @@ package void thread_term_tpl(ThreadT, MainThreadStore)(ref MainThreadStore _main // destruct manually as object.destroy is not @nogc (cast(ThreadT) cast(void*) ThreadBase.sm_main).__dtor(); _d_monitordelete_nogc(ThreadBase.sm_main); - _mainThreadStore[] = __traits(initSymbol, ThreadT)[]; + _mainThreadStore[] = cast(void[]) __traits(initSymbol, ThreadT)[]; ThreadBase.sm_main = null; assert(ThreadBase.sm_tbeg && ThreadBase.sm_tlen == 1); diff --git a/druntime/src/rt/aaA.d b/druntime/src/rt/aaA.d index 5903d9cd75..26c16d3af0 100644 --- a/druntime/src/rt/aaA.d +++ b/druntime/src/rt/aaA.d @@ -688,7 +688,7 @@ extern (C) inout(void[]) _aaValues(inout AA aa, const size_t keysz, const size_t { if (!b.filled) continue; - pval[0 .. valsz] = b.entry[off .. valsz + off]; + pval[0 .. valsz] = cast(void[]) b.entry[off .. valsz + off]; pval += valsz; } // postblit is done in object.values @@ -710,7 +710,7 @@ extern (C) inout(void[]) _aaKeys(inout AA aa, const size_t keysz, const TypeInfo { if (!b.filled) continue; - pkey[0 .. keysz] = b.entry[0 .. keysz]; + pkey[0 .. keysz] = cast(void[]) b.entry[0 .. keysz]; pkey += keysz; } // postblit is done in object.keys diff --git a/druntime/src/rt/lifetime.d b/druntime/src/rt/lifetime.d index 4a071f3d81..676f88d5ae 100644 --- a/druntime/src/rt/lifetime.d +++ b/druntime/src/rt/lifetime.d @@ -129,7 +129,7 @@ extern (C) Object _d_newclass(const ClassInfo ci) @weak } // initialize it - p[0 .. init.length] = init[]; + p[0 .. init.length] = cast(void[]) init[]; debug(PRINTF) printf("initialization done\n"); return cast(Object) p; @@ -1294,7 +1294,7 @@ extern (C) void rt_finalize2(void* p, bool det = true, bool resetMemory = true) if (resetMemory) { auto w = (*pc).initializer; - p[0 .. w.length] = w[]; + p[0 .. w.length] = cast(void[]) w[]; } } catch (Exception e)