Bug 20809 – return statement might access memory from destructed temporary

Hi, See this reduced code: https://run.dlang.io/is/yoyHXC I would expect that foo() returns 2. My guess in foo is: The return value of val is saved locally as a ref int and then the destructor of S is called (the local cache is pointing to 0). Now the ref value is dereferenced and returned. The last working version of dmd was 2.089 Maybe this issue is related to https://issues.dlang.org/show_bug.cgi?id=14696 (?) - foerdi

Digger blames this PR: https://github.com/dlang/dmd/pull/10577

Please copy-n-paste the code in this bug, so that it won't get lost in the ether in the future.

Here, let me do it for you: ------ import std.stdio; @safe: struct S { @safe: int a; ~this() { a = 0; } ref val() { return a; } } S bar() { return S(2); } int foo() { return bar.val; // The return value of val is saved locally as ref int and then the destructor of S is called (set the local cache to 0). Now the ref value is dereferenced and returned } void main() { assert(foo() == 2); // foo() is 0 } ------ Just please keep in mind next time to post the entire body of code in the bug so that it will never get lost in the future.

(In reply to hsteoh from comment #3) > Just please keep in mind next time to post the entire body of code in the > bug so that it will never get lost in the future. Thank you, I will keep this in my mind for the next time.

Even more reduced test case ---------------------------- struct S { int a; ~this() { a = 0; } } void main() { assert(foo() == 2); // foo() is 0 } int foo() { //bug is here return S(2).a; // destructor of the struct is called in this scope and sets a to 0 }

This function works as expected int foo() { S test = S(2); return test.a; }

While this is a regression, the underlying bug existed already in 2.089.1: @safe: struct S{ @safe: int[8] a; ~this(){ a[] = 0; } ref val(){ return a; } } S bar(){ return S([2,2,2,2,2,2,2,2]); } int[8] foo(){ return bar.val; } void main(){ assert(foo() == [2,2,2,2,2,2,2,2]); } // error I guess this is why review did not catch the problem (the pull request just generalized an already existing wrong transformation).

This is important for making Phobos's tempCString @safe: ``` // $(RED WARNING): $(RED Incorrect usage) auto pInvalid1 = str.tempCString().ptr; const char* pInvalid2 = str.tempCString(); // Both pointers refer to invalid memory here as // returned values aren't assigned to a variable and // both primary expressions are ended. ```

Recently I discovered an interesting fact: this error or regression has also affected the std RefCounter. https://run.dlang.io/is/PMyy0W ```d import std; int foo() { return refCounted(2); //(alias this) .refCountedPayload has ref return } void main() { writeln(foo()); // foo() is corrupted assert(foo() == 2); // foo() is corrupted } ``` I am surprised that this issue has not been more noticeable.

(In reply to Dennis from comment #8) > This is important for making Phobos's tempCString @safe: > > ``` > // $(RED WARNING): $(RED Incorrect usage) > auto pInvalid1 = str.tempCString().ptr; > const char* pInvalid2 = str.tempCString(); > // Both pointers refer to invalid memory here as > // returned values aren't assigned to a variable and > // both primary expressions are ended. > ``` That usage won't be affected by fixing this bug? The issue in the example arises because the pointer is extracted due to implicit conversion while the temporary buffer is destructed at the end of the statement. That could only work if the temporary was kept alive over multiple statements (which breaks other language rules w.r.t. temporaries)

(In reply to moonlightsentinel from comment #10) > That usage won't be affected by fixing this bug? You're right, I was looking into an accepts-invalid dip1000 issue and this looked like a duplicate, but since it's returning by value here it's actually different.

Most likely this is a backend issue since I cannot reproduce this with ldc.

This bug still exists and it depends on existence of dtor in struct: ```d struct Foo{ int i; ~this()@safe { i = 0; } } struct Bar{ int i; //no dtor! } void main()@safe{ scope int* foo = &Foo(1).i; //Compile OK but shouldn't. scope int* bar = &Bar(1).i; //Error: `Bar(2).i` is not an lvalue and cannot be modified } ```

(In reply to vitamin from comment #13) > This bug still exists and it depends on existence of dtor in struct: It now produces the errors: test.d(19): Error: address of variable `__slFoo3` assigned to `foo` with longer lifetime test.d(20): Error: `Bar(1).i` is not an lvalue and cannot be modified But the other manifestations of the problem still compile without error.

@WalterBright created dlang/dmd pull request #14358 "fix Issue 20809 - return statement might access memory from destructe…" fixing this issue: - fix Issue 20809 - return statement might access memory from destructed temporary https://github.com/dlang/dmd/pull/14358

dlang/dmd pull request #14358 "fix Issue 20809 - return statement might access memory from destructe…" was merged into master: - 35af96b308228cf6df9dcb409b4fee0ee2ddc3e6 by Walter Bright: fix Issue 20809 - return statement might access memory from destructed temporary https://github.com/dlang/dmd/pull/14358