Issue 8838 - Slicing static arrays should be considered unsafe (@system)
Summary: Slicing static arrays should be considered unsafe (@system)
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 major
Assignee: No Owner
URL:
Keywords: safe
: 6844 7087 (view as issue list)
Depends on:
Blocks:
 
Reported: 2012-10-17 13:45 UTC by Jonathan M Davis
Modified: 2017-01-16 23:26 UTC (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Jonathan M Davis 2012-10-17 13:45:53 UTC
This code compiles just fine

int[] foo() @safe
{
    int[5] a;
    return a[];
}

void main()
{}

It really shouldn't. What it's doing is _not_ memory safe. And while implementing issue# 7087 would fix this particular case, it doesn't fix the problem in general, because all it takes is adding another function to the mix, and the compiler can't catch it:

int[] foo() @safe
{
    int[5] a;
    return bar(a);
}

int[] bar(int[] a) @safe
{
    return a;
}

void main()
{}

Taking the slice of a static array is really no different from taking the address of a local variable, and that's already @system, so slicing a static array should be as well.

Honestly, I wish that static arrays didn't implicitly slice when being passed to functions taking dynamic arrays precisely because of how dangerous it is, and the fact that the implicit conversion makes it really easy to miss, but at least if it were marked @system, then it couldn't happen in @safe code, and it would be harder to have bugs like in the code above.
Comment 1 bearophile_hugs 2012-10-17 15:23:41 UTC
(In reply to comment #0)

> Taking the slice of a static array is really no different from taking the
> address of a local variable, and that's already @system, so slicing a static
> array should be as well.

This is a big change in D, so before going this route I suggest to think well about this topic.
Comment 2 Jonathan M Davis 2012-10-17 16:35:57 UTC
> This is a big change in D, so before going this route I suggest to think well
about this topic.

The thing is that it _isn't_ memory safe. There's no question of that. So, per the definition of @safe, it has no business being @safe. It needs to be @system. If it's not, then SafeD is broken. I don't see how anyone could argue otherwise.

Yes, it's breaking change in the cases where people actually use @safe, but there's no way around that, and honestly, I suspect that most people don't mark their code @safe anyway, and it's only applicable to where static arrays are sliced, so I don't know how much code will really be broken. For folks who use static arrays and @safe heavily, it'll break a lot. For most other people, probably nothing.

Regardless, I don't see how we can _not_ make this change given what @safe is supposed to do.
Comment 3 timon.gehr 2012-10-17 16:42:44 UTC
(In reply to comment #2)
> > This is a big change in D, so before going this route I suggest to think well
> about this topic.
> 
> The thing is that it _isn't_ memory safe. There's no question of that. So, per
> the definition of @safe, it has no business being @safe. It needs to be
> @system. If it's not, then SafeD is broken. I don't see how anyone could argue
> otherwise.
> ...

The code segment must be rejected, but what makes it unsafe is the escaping.
Banning the slicing is not very precise.
Comment 4 bearophile_hugs 2012-10-17 16:47:12 UTC
(In reply to comment #3)

> The code segment must be rejected, but what makes it unsafe is the escaping.
> Banning the slicing is not very precise.

Region analysis is not one of the design goals of D, unfortunately. But a little of such analysis will be useful in the D front-end.
Comment 5 Jonathan M Davis 2012-10-17 16:57:55 UTC
> The code segment must be rejected, but what makes it unsafe is the escaping.
> Banning the slicing is not very precise.

It's exactly what happens with taking the address of a local variable. It's an error if the compiler can determine that it's escaping, but it's @system regardless. And because the compiler _can't_ guarantee that the reference isn't escaping, it really has no choice but to make it @system to take the address or slice in the first place. Doing otherwise would mean that it's possible to have memory corruption issues when only using @safe code, which would be violating @safe.
Comment 6 yebblies 2012-12-26 07:24:46 UTC
*** Issue 6844 has been marked as a duplicate of this issue. ***
Comment 7 yebblies 2013-07-27 23:41:54 UTC
*** Issue 7087 has been marked as a duplicate of this issue. ***
Comment 8 yebblies 2013-11-24 02:41:27 UTC
This is not very easy, because the compiler lowers all static array assignment to slice assignment, making _every_ static array assignment unsafe.  That needs to be fixed first, and requires extensive changes in the interpreter.
Comment 9 Jonathan M Davis 2013-11-24 14:21:13 UTC
> This is not very easy, because the compiler lowers all static array assignment
> to slice assignment, making _every_ static array assignment unsafe.  That
> needs to be fixed first, and requires extensive changes in the interpreter.

Well, that's not good to hear, but unfortunately, I really don't think that the change is negotiable given what @safe is supposed to do and mean. So, even if it takes a while, it's something that needs to happen.
Comment 10 bearophile_hugs 2013-11-24 16:15:12 UTC
(In reply to comment #8)
> This is not very easy, because the compiler lowers all static array assignment
> to slice assignment, making _every_ static array assignment unsafe.  That needs
> to be fixed first, and requires extensive changes in the interpreter.

Perhaps related, for optimization, and equally important:
https://d.puremagic.com/issues/show_bug.cgi?id=10305
Comment 11 yebblies 2013-11-24 17:18:07 UTC
(In reply to comment #10)
> (In reply to comment #8)
> > This is not very easy, because the compiler lowers all static array assignment
> > to slice assignment, making _every_ static array assignment unsafe.  That needs
> > to be fixed first, and requires extensive changes in the interpreter.
> 
> Perhaps related, for optimization, and equally important:
> https://d.puremagic.com/issues/show_bug.cgi?id=10305

No, not related.  And not anywhere near as important.
Comment 12 Steven Schveighoffer 2014-06-06 18:13:31 UTC
One thing to note here. A workaround is to use the temporary @trusted delegate to do the slicing.
Comment 13 hsteoh 2014-07-15 00:21:58 UTC
Hmm. Seems like if we implement 'scope' according to some combination of current proposals, we might be able to address this by having slices of static arrays return a scoped slice with lifetime not longer than the static array itself.
Comment 14 Steven Schveighoffer 2014-11-03 16:22:35 UTC
(In reply to yebblies from comment #8)
> This is not very easy, because the compiler lowers all static array
> assignment to slice assignment, making _every_ static array assignment
> unsafe.  That needs to be fixed first, and requires extensive changes in the
> interpreter.

Can you just ban slicing that escapes the statement?
Comment 15 Jonathan M Davis 2016-04-16 04:34:45 UTC
Related: issue# 15932
Comment 16 Walter Bright 2016-06-07 07:12:25 UTC
The first case no longer compiles, but the second case remains.
Comment 17 Walter Bright 2016-08-28 10:02:09 UTC
Fixed by: https://github.com/dlang/dmd/pull/5972
Comment 18 github-bugzilla 2016-11-01 10:35:08 UTC
Commit pushed to scope at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/66187b5c1e1554cd6e417e3c762285d0234bb982
fix Issue 8838 - Slicing static arrays should be considered unsafe (@system)
Comment 19 github-bugzilla 2016-12-31 18:15:35 UTC
Commit pushed to master at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/66187b5c1e1554cd6e417e3c762285d0234bb982
fix Issue 8838 - Slicing static arrays should be considered unsafe (@system)
Comment 20 github-bugzilla 2017-01-16 23:24:17 UTC
Commit pushed to newCTFE at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/66187b5c1e1554cd6e417e3c762285d0234bb982
fix Issue 8838 - Slicing static arrays should be considered unsafe (@system)