D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 20881 - [DIP1000] scope inference turns return-ref into return-scope
Summary: [DIP1000] scope inference turns return-ref into return-scope
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 normal
Assignee: No Owner
URL:
Keywords: accepts-invalid, pull, safe
: 22528 (view as issue list)
Depends on:
Blocks:
 
Reported: 2020-05-29 21:12 UTC by Stanislav Blinov
Modified: 2022-03-21 17:27 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Stanislav Blinov 2020-05-29 21:12:23 UTC
// dmd -dip1000

@safe:

struct Correct {
    private int* ptr;
    int* get() return { return ptr; }
}

struct Faulty(T) {
    private T* ptr;
    T* get() return { return ptr; }
}

struct Workaround(T) {
    private T* ptr;
    T* get() return {
        return *&ptr; // workaround is the *&
    }
}

// fails to compile (as it should)
unittest {
    int* outlive;
    Correct c;
    outlive = c.get(); // error
}

// compiles (but shouldn't)
unittest {
    int* outlive;
    Faulty!int f;
    outlive = f.get(); // should be error
}

// fails to compile (as it should)
unittest {
    int* outlive;
    Workaround!int w;
    outlive = w.get(); // error
}
Comment 1 ag0aep6g 2020-06-03 15:27:16 UTC
Can we actually return a problematic pointer this way? As it is, the test case only returns `null`s.

It seems that without attribute inference, DMD assumes `get` might return a problematic pointer. But with attribute inference, DMD looks at the actual expression that is being returned.
Comment 2 Stanislav Blinov 2020-06-03 18:40:48 UTC
Of course we can. What ptr points to and how its value came to be is irrelevant for this test case, thus omitted.
Comment 3 ag0aep6g 2020-06-03 21:35:22 UTC
(In reply to Stanislav Blinov from comment #2)
> Of course we can. What ptr points to and how its value came to be is
> irrelevant for this test case, thus omitted.

Please show an example. When I tried returning a problematic pointer, DMD showed the same error as in the `Correct` case.
Comment 4 Stanislav Blinov 2020-06-07 11:42:26 UTC
(In reply to ag0aep6g from comment #3)

> Please show an example. When I tried returning a problematic pointer, DMD
> showed the same error as in the `Correct` case.

? malloc it in a constructor, free it in a destructor, disable copy and assignment
Comment 5 ag0aep6g 2020-06-07 19:27:27 UTC
(In reply to Stanislav Blinov from comment #4)
> ? malloc it in a constructor, free it in a destructor, disable copy and
> assignment

Makes sense. When I think about DIP 1000, I tend to think of pointers to the stack. But yeah, it's also supposed to enable malloc/free, isn't it.

For future readers, a full example:

----
import core.stdc.stdlib: free, malloc;
struct Faulty
{
    private int* ptr;
    this(int v) @trusted
    {
        ptr = cast(int*) malloc(int.sizeof);
        *ptr = v;
    }
    ~this() @trusted { if (ptr !is null) free(ptr); }
    @disable this(this);
    @disable void opAssign(Faulty);
    int* get1() @safe return { return ptr; }
    int* get2()() @safe return { return ptr; }
}
void main() @safe
{
    int* outlive;
    {
        auto f1 = Faulty(42);
        outlive = f1.get1(); /* error */
        outlive = f1.get2(); /* should be error */
    }
    /* `outlive` is invalid from here on */
}
----
Comment 6 ag0aep6g 2020-06-08 11:58:40 UTC
The method is being inferred as `return scope`. Oddly enough, that doesn't show up when printing with `pragma(msg, ...)`.

With this information, we can construct a test case that shows memory corruption even without malloc/free and @trusted:

----
struct Faulty
{
    private int x;
    private int* unused;
    int* get1() @safe return scope { return &x; }
}
int* f(int x) @safe
{
    auto f = Faulty(x);
    return f.get1(); /* Returns a pointer to the stack. */
}
void main() @safe
{
    int* p1 = f(42);
    int* p2 = f(13);
    import std.stdio;
    writeln(*p1); /* Prints "13". Whoopsie. */
}
----
Comment 7 Dennis 2021-11-24 15:00:06 UTC
*** Issue 22528 has been marked as a duplicate of this issue. ***
Comment 8 Dennis 2021-11-24 15:18:51 UTC
There's multiple things going on, so I'm going to limit this issue to "scope inference turns return-ref into return-scope", the issue that the compiler confuses `return scope` and `return ref` internally should be covered by issue 22108 and issue 22541.
Comment 9 Dlang Bot 2022-03-09 16:36:01 UTC
@dkorpel updated dlang/dmd pull request #13693 "Make consistent use of `STC.returnScope`" fixing this issue:

- Fix issue 20881, 22790 - make ref-return-scope consistent

https://github.com/dlang/dmd/pull/13693
Comment 10 Dlang Bot 2022-03-21 17:27:29 UTC
dlang/dmd pull request #13693 "Fix issue 20881, 22790 - make ref-return-scope consistent" was merged into master:

- fce2d6529550931dc7ed35f904bbc7a6ac0cd8f0 by dkorpel:
  Fix issue 20881, 22790 - make ref-return-scope consistent

https://github.com/dlang/dmd/pull/13693