Bug 18071 – [REG2.078] byKey, byValue and byKeyValue are now a hole for unsafe code
Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
druntime
Product
D
Version
D2
Platform
x86_64
OS
All
Creation time
2017-12-13T08:52:30Z
Last change time
2017-12-18T22:54:59Z
Assigned to
No Owner
Creator
Seb
Comments
Comment #0 by greensunny12 — 2017-12-13T08:52:30Z
```
int main() @safe {
auto a = ["foo": 1];
auto r = a.byValue;
r.popFront;
return r.front; // oops
}
```
and another one:
```
struct Foo {
int[int] aa;
auto opCast() pure nothrow @nogc { *cast(uint*)0xdeadbeef = 0xcafebabe; return null; }
alias aa this;
}
int main() @safe {
Foo f;
return !f.byKey.empty;
}
```
While 2.077.1 correctly emits warnings that this isn't `@safe`, the code (after https://github.com/dlang/druntime/pull/1944) segfaults.
Comment #1 by jack — 2017-12-13T13:56:00Z
The first example can be fixed by having `byValue.front` and `byValue.popFront` do this at the beginning of the function:
if (this.empty)
assert(0, "accessing front on an empty byValue");
Comment #2 by schveiguy — 2017-12-13T15:41:02Z
(In reply to Jack Stouffer from comment #1)
> The first example can be fixed by having `byValue.front` and
> `byValue.popFront` do this at the beginning of the function:
>
> if (this.empty)
> assert(0, "accessing front on an empty byValue");
This will have the same effect, as druntime is compiled in release mode.
Note that the first example isn't actually unsafe. It's simply dereferencing a null pointer (which is fine in @safe code).
I have a PR in progress to fix the second problem. I'm not loving how it's working out, but I am not sure exactly how to do it in a correct way (will post something about it on the newsgroup).
Comment #3 by jack — 2017-12-13T16:04:58Z
(In reply to Steven Schveighoffer from comment #2)
> (In reply to Jack Stouffer from comment #1)
> > The first example can be fixed by having `byValue.front` and
> > `byValue.popFront` do this at the beginning of the function:
> >
> > if (this.empty)
> > assert(0, "accessing front on an empty byValue");
>
> This will have the same effect, as druntime is compiled in release mode.
assert(0) isn't removed in release mode
Comment #4 by schveiguy — 2017-12-13T16:24:21Z
(In reply to Jack Stouffer from comment #3)
> assert(0) isn't removed in release mode
Right, but it's implemented as a segfault, not throwing an AssertError with a nice message.
Comment #5 by github-bugzilla — 2017-12-14T21:20:17Z