Issue 17927 - [scope] `scope inout` parameter value can be escaped via return
Summary: [scope] `scope inout` parameter value can be escaped via return
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P4 normal
Assignee: Walter Bright
URL:
Keywords: safe
Depends on:
Blocks:
 
Reported: 2017-10-22 17:16 UTC by Martin Nowak
Modified: 2021-06-15 23:22 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 Martin Nowak 2017-10-22 17:16:42 UTC
cat > bug.d << CODE
struct String
{
pure nothrow @nogc:
    inout(char)[] opSlice() inout scope @trusted
    {
        return ptr[0 .. len];
    }

    char *ptr;
    size_t len;
}

void escape(const char[] s) nothrow @safe @nogc
{
    static const(char)[] cache;
    cache = s;
}

///
nothrow @safe
unittest
{
    auto s = String(&"Hello".dup[0], 5);
    escape(s[]);
}
CODE
dmd -c -unittest -dip1000 bug.d
----

Should error with `scope variable this.ptr may not be returned`.

workaround:
----
    char[] opSlice() scope @trusted
    {
        return ptr[0 .. len];
    }

    const(char)[] opSlice() const scope @trusted
    {
        return ptr[0 .. len];
    }
----
Comment 1 Martin Nowak 2017-10-22 17:22:14 UTC
My bad, there are two bugs.

All of the above opSlice methods should fail to compile, but on the mutable and cost method do, the inout silently compiles, even though it escapes a field.

All of the methods should and do compile with return scope (even the inout one).
The compiler doesn't infer scope for their returned slice and thus allows escaping that.
Comment 2 Walter Bright 2017-10-23 02:28:55 UTC
Changing the @trusted to @safe makes the first example fail to compile with:

  test.d(6): Error: pointer slicing not allowed in safe functions

Changing String to:

  struct String {
    inout(char)[] opSlice() inout scope @safe {
        return ptr[];
    }

    char[] ptr;
  }

And it now compiles, as it should. Will look at the rest.
Comment 3 Walter Bright 2017-10-23 04:57:54 UTC
Back to the process of stripping things down to the essentials:

--------------------
const(char)* foo1(scope const(char)* ptr) @safe { return ptr; }

inout(char)* foo2(scope inout(char)* ptr) @safe { return ptr; }
--------------------

Produces the expected error messages:

  test.d(1): Error: scope variable ptr may not be returned
  test.d(3): Error: scope variable ptr may not be returned

So add in a bit of complexity:

--------------
struct String {
    const(char)* mem1() const scope @safe { return ptr; }

    inout(char)* mem2() inout scope @safe { return ptr; }

    char* ptr;
}
--------------

Produces:

  test.d(2): Error: scope variable this may not be returned

The message for mem2() is not generated, so the issue is with the 'inout' on the 'this' parameter.
Comment 4 Walter Bright 2017-10-23 05:35:17 UTC
It turns out that:

  struct String {
    inout(char)* mem2() inout scope @safe { return ptr; }
    char* ptr;
  }

not issuing an error is actually correct, because a parameter that is `ref inout` is inferred to be `return`, and the `this` parameter for `mem2` is `ref inout`.
Comment 5 Walter Bright 2017-10-23 05:40:17 UTC
For the unittest, the `dup` does not create data with limited lifetime, so `s` is not inferred as `scope`, and `escape` is free to escape it.

It's complicated, but the compiler is working as it is supposed to.
Comment 6 Walter Bright 2017-10-23 05:52:23 UTC
https://github.com/dlang/dmd/pull/7235
Comment 7 Walter Bright 2017-10-23 08:01:16 UTC
Spec pull: https://github.com/dlang/dlang.org/pull/1914
Comment 8 github-bugzilla 2017-10-26 12:07:47 UTC
Commits pushed to master at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/3f7544f355eacc0ad390a89b1bc07ca2dbcf835e
fix Issue 17927 - [scope]  parameter value can be escaped via return

https://github.com/dlang/dmd/commit/b46ac59c637723877b52b98ed50167e0f68aca5d
Merge pull request #7235 from WalterBright/fix17927

fix Issue 17927 - [scope]  'scope inout' parameter value can be escaped via return
Comment 9 github-bugzilla 2017-12-18 22:55:27 UTC
Commits pushed to stable at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/3f7544f355eacc0ad390a89b1bc07ca2dbcf835e
fix Issue 17927 - [scope]  parameter value can be escaped via return

https://github.com/dlang/dmd/commit/b46ac59c637723877b52b98ed50167e0f68aca5d
Merge pull request #7235 from WalterBright/fix17927
Comment 10 Martin Nowak 2018-01-04 00:52:21 UTC
Thanks for investigating, so now it's reduced to the old problem that the scope system does not allow to define entry points.
Of course in real life this is using malloc instead of GC'ed dup, but it's not possible to contain the former.
Comment 11 Atila Neves 2018-08-16 18:29:16 UTC
I don't understand how it's possible that making it `inout` is correct inference. This allows for code that looks @safe but isn't. This really shouldn't compile:

@safe:

const(int)* gInt;

void main() {
    auto s = Struct();
    gInt = s.ptr;  // ARGH!
}

struct Struct {

    int* ints;

    this(int size) {
        import core.stdc.stdlib;
        ints = () @trusted { return cast(int*) malloc(size); }();
    }

    ~this() {
        import core.stdc.stdlib;
        () @trusted { free(ints); }();
    }

    scope inout(int)* ptr() inout {
        return ints;
    }
}



And yet it does. I guess I'll have to define 3 methods for mutable, const and immutable if I want to not crash.
Comment 12 Steven Schveighoffer 2018-08-20 14:04:03 UTC
(In reply to Walter Bright from comment #4)
> It turns out that:
> 
>   struct String {
>     inout(char)* mem2() inout scope @safe { return ptr; }
>     char* ptr;
>   }
> 
> not issuing an error is actually correct, because a parameter that is `ref
> inout` is inferred to be `return`, and the `this` parameter for `mem2` is
> `ref inout`.


What? ref inout should NOT be inferred as return. inout is a pattern match on the mutability of the parameters, it does not necessarily imply that it is part of the return type.

This can be handy when trying avoid code duplication when the const/immutable is nested under several indirections (including ref).

Reopening, the original problem is not fixed. The error case added tests for compiling the functions, but doesn't test that the result of the inout function is scope (it should be).
Comment 13 Mike Franklin 2019-09-04 13:18:08 UTC
Inferring `return` on `this` for anything marked with `inout` appears to be the cause of issue 20149.
Comment 14 Walter Bright 2020-03-04 10:01:21 UTC
(In reply to Steven Schveighoffer from comment #12)
> ref inout should NOT be inferred as return. inout is a pattern match
> on the mutability of the parameters, it does not necessarily imply that it
> is part of the return type.

inout is deliberately inferred as return. It's the way the language currently works. To change it please make an enhancement request, as such should be discussed on its own merits.
Comment 15 Steven Schveighoffer 2020-03-04 23:07:17 UTC
Fixing the resolution, as the original bug was not invalid. If I have time, I'll try to remember what this was about and open another enhancement request.