D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 22539 - [dip1000] slicing of returned ref scope static array should not be allowed
Summary: [dip1000] slicing of returned ref scope static array should not be allowed
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: pull
Depends on:
Blocks:
 
Reported: 2021-11-23 19:38 UTC by Dennis
Modified: 2022-03-10 08:34 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Dennis 2021-11-23 19:38:16 UTC
Because there is no transitive scope, taking the address of a scope variable is not allowed. There are 4 cases:

- ref param &pointer: rejected with `checkAddresVar`
- ref param slice[]: rejected with `checkAddresVar`
- ref return &pointer: too restrictive (issue 22519)
- ref return slice[]: too lenient (this issue)

This allows you to escape scope pointers:
```
// REQUIRED_ARGS -preview=dip1000
@safe:
ref int*[1] identity(ref return scope int*[1] x)
{
	return x;
}

int* escape()
{
	int stackVar = 0xFF;
	scope int*[1] x = [&stackVar];
	int*[] y = identity(x)[];
	return y[0];
}

void main()
{
	int* dangling = escape();
}
```
Comment 1 Dlang Bot 2021-11-26 17:17:00 UTC
@dkorpel created dlang/dmd pull request #13362 "Fix issue 22539 - slicing of returned ref scope static array should not be allowed" fixing this issue:

- Fix issue 22539 - slicing of returned ref scope static array should not be allowed

https://github.com/dlang/dmd/pull/13362
Comment 2 Walter Bright 2021-11-27 19:57:20 UTC
I suspect the problem is actually this line:

    scope int*[1] x = [&stackVar];

Since scope is not transitive, allowing this causes the compiler to lose track of stack addresses, and will allow escapes.
Comment 3 Dennis 2021-11-27 20:08:37 UTC
(In reply to Walter Bright from comment #2)
> I suspect the problem is actually this line:
> 
>     scope int*[1] x = [&stackVar];
> 
> Since scope is not transitive, allowing this causes the compiler to lose
> track of stack addresses, and will allow escapes.

Only if x were a slice, but since it's a static array with dimension 1 it is identical to:

scope int* x = &stackVar;

So it is correctly permitted.
Comment 4 Walter Bright 2021-11-27 20:15:07 UTC
Add @safe to escape() and it will fail to compile.
Comment 5 Walter Bright 2021-11-27 20:19:19 UTC
P.S. many times I've chased rabbits only to discover I forgot to add @safe. @safe really should be the default.
Comment 6 Dennis 2021-11-27 20:19:36 UTC
(In reply to Walter Bright from comment #4)
> Add @safe to escape() and it will fail to compile.

`escape` is already under `@safe:`. I added `@safe` to escape explicitly to double check, it still compiles on my end. What error do you get?
Comment 7 Dennis 2021-11-27 20:23:14 UTC
(In reply to Walter Bright from comment #5)
> P.S. many times I've chased rabbits only to discover I forgot to add @safe.
> @safe really should be the default.

I didn't forget @safe, I added @safe: on top (as I usually do).
Comment 8 Walter Bright 2021-11-27 20:26:43 UTC
My bad, adding -dip1000 removes the error message. I had neglected to do that.
Comment 9 Dennis 2021-11-27 20:30:24 UTC
(In reply to Walter Bright from comment #8)
> My bad, adding -dip1000 removes the error message. I had neglected to do
> that.

I guess -dip1000 really should be the default :)
Comment 10 Walter Bright 2021-11-27 21:29:00 UTC
Moving on from my previous mistakes:

    @safe:
    ref int* identity(ref return scope int* x) { return x; }

    int* escape() {
        int stackVar;
        scope int* x = &stackVar;
        int* y = identity(x);
        return y; // error: scope variable `y` may not be returned
    }

Good. Next,

    @safe:
    ref int*[1] identity(ref return scope int*[1] x) { return x; }

    int*[1] escape() {
        int stackVar;
        scope int*[1] x = [&stackVar];
        int*[1] y = identity(x);
        return y; // error: scope variable `y` may not be returned
    }

Good.

    int* escape() {
        int stackVar;
        scope int*[1] x = [&stackVar];
        int*[1] y = identity(x);
        return y[0]; // error: scope variable `y` may not be returned
    }

Good.

    int* escape() {
        int stackVar;
        scope int*[1] x = [&stackVar];
        int*[1] y = identity(x);
        int*[] z = y[]; // error: cannot take address of `scope` local `y`
        return z[0];
    }

Good. Here, the slicing of `y` is properly detected. But if we put the [] after the identity call:

    int* escape() {
        int stackVar;
        scope int*[1] x = [&stackVar];
        int*[] y = identity(x)[];
        return y[0];
    }

No error; bad. The slicing of the return value of identity() is not properly propagating the scope-ness of its argument x. This would be a problem in escape.d.
Comment 11 Dennis 2021-11-27 22:41:19 UTC
(In reply to Walter Bright from comment #10)
> No error; bad. The slicing of the return value of identity() is not properly
> propagating the scope-ness of its argument x. This would be a problem in
> escape.d.

Yes, the scope-ness is lost because there is no transitive scope, so the slicing shouldn't be allowed in @safe code. It's essentially the same as:
https://issues.dlang.org/show_bug.cgi?id=20691

Which you fixed with:
https://github.com/dlang/dmd/pull/10951

But that fix is limited to variables, not return values, so my pull request extends it by also considering slicing a CallExp:
https://github.com/dlang/dmd/pull/13362

I don't understand what your comment is getting at, do you have any objections to this?
Comment 12 Walter Bright 2021-11-28 02:41:21 UTC
Objections? Not at the moment, I'm just trying to thoroughly understand the problem.
Comment 13 Walter Bright 2021-11-29 00:07:54 UTC
(In reply to Walter Bright from comment #12)
> Objections? Not at the moment, I'm just trying to thoroughly understand the
> problem.

See the PR for my remaining objection. Thanks for your patience with this.
Comment 14 Dlang Bot 2022-03-10 08:34:11 UTC
dlang/dmd pull request #13362 "Fix issue 22539 - slicing of returned ref scope static array should not be allowed" was merged into master:

- 8f3f72f93a6b6241342c495c569f294c4ea0238f by dkorpel:
  Fix issue 22539 - slicing of returned ref scope static array should not be allowed

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