Bug 24162 – Another example of why @safe is broken
Status
RESOLVED
Resolution
INVALID
Severity
normal
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2023-09-26T06:36:37Z
Last change time
2023-10-02T09:02:38Z
Assigned to
No Owner
Creator
tomerfiliba
Comments
Comment #0 by tomerfiliba — 2023-09-26T06:36:37Z
have a look at this snippet - all @safe, uses GC and no pointers: https://run.dlang.io/is/KGbaEd
```
@safe:
import std.stdio: writeln;
void f(ref int x, ref int[] arr) {
x += 1; // this happens on arr[2]
arr ~= new int[900000]; // array is relocated
x += 1; // this now modifies random memory and will not be reflected on arr
}
void main() {
auto arr = new int[10];
f(arr[2], arr);
writeln(arr[0..10]);
}
```
it passes two mutable references to the array, which means the array can be relocated while a reference exists
Comment #1 by issues.dlang — 2023-09-26T07:43:04Z
Why do you think that it modifies random memory? If a dynamic array is reallocated, that has no effect whatsoever on the block of memory that it referred to before. A new block of memory is allocated, the portions of the old block of memory that the dynamic array refers to are copied into the new block of memory, and the dynamic array's pointer is changed to point to the new block of memory. However, the old block of memory is still around. The GC still knows about it, and it's still valid, so it won't be collected until nothing refers to it any longer. So, x can continue to refer to that block of memory and access it even though arr no longer does. Sure, you don't actually have access to what x changed once f returns, but nothing unsafe occurred.
Comment #2 by tomerfiliba — 2023-09-26T07:50:11Z
suppose this function runs in a long living thread. so what you're saying is, it will be leaked forever because the GC will see it's pointed by a stack location? fine. but surely that's undefined behavior? accessing this memory location no longer does what it was meant to do.
> but surely that's undefined behavior?
It's not undefined behavior. The GC only frees memory with no references to it, so keeping a reference to the old array keeps it alive.
> accessing this memory location no longer does what it was meant to do.
@safe is about preventing memory corruption, not bugs in general. The compiler can't know what the intention of memory is, it just looks at size and lifetime. Only if your reference becomes a dangling pointer or out of bounds in @safe code, it's a bug.
Comment #5 by maxhaton — 2023-09-27T02:21:08Z
This is a valid reason to attempt to ban mutable aliasing. It's not an uncommon thing to try and avoid with the type system in newer languages.
Whether its valid or not is basically a matter of definition although I will point out that in this particular example I don't know if the GC would actually spot the reference to it because there wouldn't be much on the stack for it to find.
Comment #6 by issues.dlang — 2023-09-27T03:35:53Z
All the GC needs is a pointer to an address in block of memory that it controls, and taking a reference to a member of a dynamic array that's a slice of GC-allocated memory will do that. There is absolutely nothing untoward going on here with regards to memory safety.
Yes, the caller might be very surprised if the result isn't reflected in arr[2] of the dynamic array after the function returns, but any time that you pass a dynamic array to a function that could choose to append to it, it could result in the dynamic array being reallocated. And if the function in question didn't append to the array, then having passed arr[2] wouldn't be a problem at all. But as for mutable aliasing in general, that's how dynamic arrays function in D. They're just slices of memory that they don't control, and you're allowed to slice those arrays as much as you'd like. And as long as they're slices of GC-allocated memory, it's all memory safe. So, trying to prevent mutable aliasing when it comes to dynamic arrays is not going to work. For better or worse, it's fundamental to how dynamic arrays work in D. And for the most part, it works just fine much as it can be confusing at first.
Comment #7 by dfj1esp02 — 2023-09-27T08:52:32Z
(In reply to tomerfiliba from comment #2)
> accessing this memory location no longer does what it was meant to do.
If this function doesn't reflect your intention, then write a function that reflects your intention, like:
@safe:
import std.stdio: writeln;
void f(int x, ref int[] arr) {
arr[x] += 1;
arr ~= new int[900000];
arr[x] += 1;
}
void main() {
auto arr = new int[10];
f(2, arr);
writeln(arr[0..10]);
}
Comment #8 by nick — 2023-10-02T09:02:38Z
I don't think we should ban passing a reference to an element and also a reference to the array. This can be perfectly valid, for example when the element reference is only modified before the array is resized. Also when the element reference is only read, even after the resize. Also when mutating the element reference is still accessible in another slice of the array even after the array reference is resized. So there are lots of valid situations we shouldn't break.