November 03, 2020 [Issue 21357] New: [REG2.093] postblit aliases old and new struct pointers | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=21357 Issue ID: 21357 Summary: [REG2.093] postblit aliases old and new struct pointers Product: D Version: D2 Hardware: All OS: All Status: NEW Severity: regression Priority: P1 Component: dmd Assignee: nobody@puremagic.com Reporter: johanengelen@weka.io Testcase: ``` struct BatchState { int[10] arr; // change size to 1 to remove the bug BatchState copy() { auto ret = BatchState(arr); arr[0] += 1; return ret; } } struct GrayArea { BatchState low; this(this) { low = low.copy; } } void main() { GrayArea a; a.low.arr[0] = 1; GrayArea b; b.low.arr[0] = 4; b = a; // calls the postblit import std.stdio; writeln(a.low.arr[0]); writeln(b.low.arr[0]); } ``` Old output: ❯ ~/dlang/dmd-2.092.1/linux/bin64/dmd -run bug.d 1 1 2.093 output: ❯ ~/dlang/dmd-2.093.1/linux/bin64/dmd -run bug.d 1 2 The problem is the following: 1. BatchState is a large struct, which means that `BatchState.copy()` is passed a pointer to a caller-allocated struct to store its return value (instead of returning a value through a register). 2. The call `low = low.copy` in `this(this)` _should_ be creating a temporary BatchState struct on the stack and pass a pointer to that to `BatchState.copy()` so it can store the return value there. That is, the call should be transformed to something like: `BatchState temp; BatchState.copy(&temp, &low); low = temp;`. Instead what happens since 2.093 is that the call becomes: `BatchState.copy(&low, &low);`. This is a very serious bug. Common code like `this(this) { member = member.dup; }` is now utterly broken. -- |
Copyright © 1999-2021 by the D Language Foundation