Bug 23175 – -preview=in silently adds possible stack memory escape

In the current specification, an `in` parameter is defined as equivalent to `const`, as long as you don't use the `-preview=in` switch. Consider this existing code, written with the assumption that `in` is `const`: ```d string foo(in string s) { return s; } ``` This code is perfectly valid, does not cause memory corruption, and is functionally equivalent to: ```d string foo(const string s) { return s; } ``` However, turn on `-preview=in`, and now the function implies that the `s` parameter is `scope`. Turn dip1000/dip25 on, and now, the compiler assumes that `s` is not returned, and so it can allocate an array literal on the stack: ```d auto s = foo(['a']); // s now points to temporary stack data that is out of scope. ``` However, without the preview switches, the compiler allocates the array on the heap. This can lead to memory corruption via dangling pointers in @system code, that wasn't present without the preview switches. If preview in is going to change the semantics of parameters in a way that allows the compiler to introduce memory corruption, this usage *must* be warned about, either in the current compiler, or with an enabled switch (possibly dip1000), before it is turned on by default.

Some historical context: Pn 2018-06-14, in commit 93411bed2 [], the spec was changed On 2020-08-16, in commit 52e211539 [], the spec was changed to its current wording, which documents `-preview=in` and says that `in` parameters "behave as though" they are `const scope`. []: https://github.com/dlang/dlang.org/commit/93411bed2 []: https://github.com/dlang/dlang.org/commit/52e211539

Apologies for the previous comment. It was submitted in error while still incomplete.

Some historical context: Prior to 2013-02-06, the spec defined `in` as a no-op attribute (likely a holdover from D1). On 2013-02-06, in commit 26d4aec9c [1], `in` was defined as equivalent to `const scope`. On 2018-01-25, in commit 71ad1b38d [2], the spec was changed to define `in` as equivalent to `const`. The commit message refers to issue 17928, which contains some relevant discussion. On 2018-06-14, in commit 93411bed2 [3], the spec was changed to define `in` as `const scope`, with a note that the current implementation treated it as equivalent to `const`. On 2020-08-16, in commit 52e211539 [4], the spec was changed to its current wording, which documents `-preview=in` and says that `in` parameters "behave as though" they are `const scope`. [1]: https://github.com/dlang/dlang.org/commit/26d4aec9c [2]: https://github.com/dlang/dlang.org/commit/71ad1b38d [3]: https://github.com/dlang/dlang.org/commit/93411bed2 [4]: https://github.com/dlang/dlang.org/commit/52e211539

My summary the historical information above: * Except for a few months in 2018, `in` has consistently been documented as `const scope` since 2013. * Until `-preview=in`, the compiler has consistently treated `in` as `const`. * There has been considerable confusion over whether the spec or the compiler's behavior should be treated as authoritative w.r.t. the meaning of `in`. In light of this information, I am inclined to agree with Jonathan M Davis's comment from issue 17928: > If we want in to mean scope const instead of just const, then we're going > to need a means of migrating to that without immediately breaking code

One further update, in Feb 2021, which is still there to this day, notes that the `in` storage class without the preview=in switch means `const`: https://github.com/dlang/dlang.org/commit/8f05cb00c99c90d88550972199c7941e2ea8ca21

@dkorpel created dlang/dmd pull request #14219 "Fix 23175 - -preview=in silently adds possible stack memory escape" fixing this issue: - Fix 23175 - -preview=in silently adds possible stack memory escape https://github.com/dlang/dmd/pull/14219

As was pointed out in the lengthy forum thread: - `scope` now leads to stack allocation; - `-preview=in` is supposed to make `in` means `scope const`; So I don't see how turning on a switch that is supposed to make `in` have the semantic of `scope` is "silently breaking code" ?? Hello, *the whole point of `-preview`* is to introduce potentially-breaking changes! This is working as intended.

Let's look at foo written like this: ```d string foo(const string s) { return s; } ``` Do you think there is any valid case where the compiler can be changed (even behind a switch) to make this start corrupting memory, yet not give any warning about it? It is the same here. `in` means `const`. It's that way in the implementation, it says that right in the spec. This is *valid* code, with an *expected* meaning, and you are *changing the meaning* without warning. You say that it's expected when you use -preview=in, because you specifically used that switch. Does this mean preview in is never meant to be used on existing code? Does this mean that at no time in the future will preview in be turned on by default? I must assume this is a "preview" of what is to come. If we never intend to make it a permanent semantic change, then we shouldn't call it a "preview". I do not think we need to change the semantics of preview=in, or to disable optimizations based on it. What we need is a warning to tell the user that what he wrote before, while valid with the previous compiler, has memory corruption implications now that the semantics have changed.

> I do not think we need to change the semantics of preview=in, or to disable optimizations based on it. What we need is a warning to tell the user that what he wrote before, while valid with the previous compiler, has memory corruption implications now that the semantics have changed. You should have led with this. That's what `-transition` is for. It lists all instances of `in` usage so you can inspect them.

(In reply to Mathias LANG from comment #9) > > I do not think we need to change the semantics of preview=in, or to disable optimizations based on it. What we need is a warning to tell the user that what he wrote before, while valid with the previous compiler, has memory corruption implications now that the semantics have changed. > > You should have led with this. That's what `-transition` is for. It lists > all instances of `in` usage so you can inspect them. ``` -transition=<name> help with language change identified by 'name' ``` That description could do with some improvement.

(In reply to Mathias LANG from comment #9) > You should have led with this. That's what `-transition` is for. It lists > all instances of `in` usage so you can inspect them. Thanks, I didn't know about this switch. However, it just lists all the places where `in` is a parameter, even ones where it is effectively const, i.e.: `void foo(in int x){}` It doesn't warn about the problems here. I also get 100 listings of `in` on parameters in `object.d`, probably all of which are fine. This isn't helpful. The compiler can do better, it knows when it might try this optimization, I don't.

The most straightforward fix would be to have the compiler emit a warning any time an `in` parameter escapes from a function in @system or @trusted code (i.e., the same thing it currently emits an error for in @safe code). If we don't want to make the warning part of `-preview=in` itself, having it enabled by `-transition=in` could also work (though I suspect the vast majority of users will never discover it there).

> Thanks, I didn't know about this switch. However, it just lists all the places where `in` is a parameter, even ones where it is effectively const, i.e.: The `-transition` switch was put in place because `in` also means `ref`, which one may find "breaks code" (hint: it doesn't). The compiler *will* tell you when you're doing something wrong, though: With DIP1000 and @safe. As the spec says it will.

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/20119 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB