May 27, 2019
On Friday, 24 May 2019 at 16:51:11 UTC, ag0aep6g wrote:
> On 24.05.19 18:19, Atila Neves wrote:
>> On Friday, 24 May 2019 at 13:30:05 UTC, ag0aep6g wrote:
> [...]
>>> My `puts`s might not do any harm, but they could just as well be buffer overflows.
>> 
>> Could you please give an example of how @system allocator code could do that?
>
> Sure. You just write beyond some buffer instead of calling `puts`:
>
> ----
> char[3] buf;
> char[3] foo = "foo";
> char[3] bar = "bar";
>
> struct UnsafeAllocator
> {
>     import std.experimental.allocator.mallocator: Mallocator;
>     static instance = UnsafeAllocator.init;
>     size_t i;
>     void deallocate(void[] bytes) @nogc @system
>     {
>         buf.ptr[i .. i + 3] = '!';
>         Mallocator.instance.deallocate(bytes);
>     }
>     void[] allocate(size_t sz) @nogc @system
>     {
>         buf.ptr[i .. i + 3] = '!';
>         return Mallocator.instance.allocate(sz);
>     }
> }
>
> void main() @safe @nogc
> {
>     {
>         import nogc: BUFFER_SIZE, text;
>         UnsafeAllocator.instance.i = 8;
>             /* greater than buf.length, whoops */
>         auto t = text!(BUFFER_SIZE, UnsafeAllocator)(42);
>         assert(foo == "foo"); /* fails */
>         UnsafeAllocator.instance.i = 16;
>             /* also greater than buf.length, whoops again */
>     }
>     assert(bar == "bar"); /* fails */
> }
> ----
>
> You just can't trust user-provided @system code. It doesn't matter if it's allocator code or whatever.

I don't see the problem here. This example would throw RangeError at runtime instead of actually overwriting memory unless bounds checking is turned off.

The other issue is that Mallocator has a @safe allocate function and a @system deallocate function since it's up to the user of the interface to supply a slice that was actually malloc'ed. It's clear that this interface is one that can be used @safely (and is by automem.vector.Vector). Likewise, reallocating is @system because there might be references to the old pointer, but it makes sense that a @trusted block could exist where the reviewer makes sure that there's only ever one reference to the allocated memory.

Then there's the fact that if a 3rd party library really does want to corrupt memory they can just tag all their functions with @trusted, and unless someone looks at their code nobody will be the wiser.
May 27, 2019
On Monday, 27 May 2019 at 08:54:45 UTC, Atila Neves wrote:
> On Friday, 24 May 2019 at 16:51:11 UTC, ag0aep6g wrote:

> Then there's the fact that if a 3rd party library really does want to corrupt memory they can just tag all their functions with @trusted, and unless someone looks at their code nobody will be the wiser.

... and a @safe conscious programmer will not touch that library ever with a 5 five meters pole.

I'm still not convinced that trusted code should accept generic system code ... can you elaborate more?

Thanks,
Paolo

May 27, 2019
On 27.05.19 10:54, Atila Neves wrote:
> I don't see the problem here. This example would throw RangeError at runtime instead of actually overwriting memory unless bounds checking is turned off.

No, it doesn't. It's a complete, runnable example. You can try it at home. It does overwrite `foo` and `bar`. It does not throw a RangeError.

> The other issue is that Mallocator has a @safe allocate function and a @system deallocate function since it's up to the user of the interface to supply a slice that was actually malloc'ed. It's clear that this interface is one that can be used @safely (and is by automem.vector.Vector). Likewise, reallocating is @system because there might be references to the old pointer, but it makes sense that a @trusted block could exist where the reviewer makes sure that there's only ever one reference to the allocated memory.

Yes, you can use @trusted to use Mallocator safely. And your code (probably) does that. But the allocator in my example isn't Mallocator, it's UnsafeAllocator. Your code doesn't use that one safely.

> Then there's the fact that if a 3rd party library really does want to corrupt memory they can just tag all their functions with @trusted, and unless someone looks at their code nobody will be the wiser.

In this thread, you're the author of that 3rd party library. You've got the bad @trusted functions that lead to memory corruption. I'm the guy who looked at it, noticed the problem, and tells you.
May 27, 2019
On Monday, 27 May 2019 at 09:07:48 UTC, Paolo Invernizzi wrote:
> On Monday, 27 May 2019 at 08:54:45 UTC, Atila Neves wrote:
>> On Friday, 24 May 2019 at 16:51:11 UTC, ag0aep6g wrote:
>
>> Then there's the fact that if a 3rd party library really does want to corrupt memory they can just tag all their functions with @trusted, and unless someone looks at their code nobody will be the wiser.
>
> ... and a @safe conscious programmer will not touch that library ever with a 5 five meters pole.
>
> I'm still not convinced that trusted code should accept generic system code ... can you elaborate more?

I'm not convinced either - I'm having a dialogue to figure out potential issues.


May 27, 2019
On Monday, 27 May 2019 at 09:48:27 UTC, ag0aep6g wrote:
> On 27.05.19 10:54, Atila Neves wrote:
>> I don't see the problem here. This example would throw RangeError at runtime instead of actually overwriting memory unless bounds checking is turned off.
>
> No, it doesn't. It's a complete, runnable example. You can try it at home. It does overwrite `foo` and `bar`. It does not throw a RangeError.

You're right - I should have run it first.

> Yes, you can use @trusted to use Mallocator safely. And your code (probably) does that. But the allocator in my example isn't Mallocator, it's UnsafeAllocator. Your code doesn't use that one safely.

No, and I guess it can't. I'm trying to figure out what the implications are. Can Vector only be @safe for Mallocator? Is it possible to write a @safe Vector at all without having to force the allocator to be @safe?

> In this thread, you're the author of that 3rd party library. You've got the bad @trusted functions that lead to memory corruption. I'm the guy who looked at it, noticed the problem, and tells you.

Thanks for bringing it up.


May 27, 2019
On Monday, 27 May 2019 at 10:01:15 UTC, Atila Neves wrote:
> On Monday, 27 May 2019 at 09:07:48 UTC, Paolo Invernizzi wrote:
>> On Monday, 27 May 2019 at 08:54:45 UTC, Atila Neves wrote:
>>> On Friday, 24 May 2019 at 16:51:11 UTC, ag0aep6g wrote:
>>
>>> Then there's the fact that if a 3rd party library really does want to corrupt memory they can just tag all their functions with @trusted, and unless someone looks at their code nobody will be the wiser.
>>
>> ... and a @safe conscious programmer will not touch that library ever with a 5 five meters pole.
>>
>> I'm still not convinced that trusted code should accept generic system code ... can you elaborate more?
>
> I'm not convinced either - I'm having a dialogue to figure out potential issues.

:-)

My nice-try to reduce the problem is: trusted code block needs to by "manually verified" for safety by humans, so it should be "@safe pure", aka, if you can't perform the analysis looking only at the statements in the trusted block, that can't be marked trusted.


May 27, 2019
On 27.05.19 12:06, Atila Neves wrote:
> No, and I guess it can't. I'm trying to figure out what the implications are. Can Vector only be @safe for Mallocator? Is it possible to write a @safe Vector at all without having to force the allocator to be @safe?

For @safe allocators, Vector can be @safe.

For specific @system allocators, like Mallocator, you can make special @trusted cases in Vector.

For generic @system allocators, Vector cannot be @safe (or @trusted).
May 27, 2019
On Monday, 27 May 2019 at 10:31:10 UTC, ag0aep6g wrote:
> On 27.05.19 12:06, Atila Neves wrote:
>> No, and I guess it can't. I'm trying to figure out what the implications are. Can Vector only be @safe for Mallocator? Is it possible to write a @safe Vector at all without having to force the allocator to be @safe?
>
> For @safe allocators, Vector can be @safe.
>
> For specific @system allocators, like Mallocator, you can make special @trusted cases in Vector.
>
> For generic @system allocators, Vector cannot be @safe (or @trusted).

It's ugly but would work. Right now I don't think I can do any better than to follow your suggestion, but I predict many beard-stroking walks for me along Lake Geneva in the near future.

I'd be nice if I could detect at compile-time that it's not just Mallocator but an allocator that's built using it as well (e.g. FallBackAllocator).
May 27, 2019
On 27.05.19 15:34, Atila Neves wrote:
> It's ugly but would work. Right now I don't think I can do any better than to follow your suggestion, but I predict many beard-stroking walks for me along Lake Geneva in the near future.

Oh, yeah. Getting @trusted right is hard. Getting it right when user-provided types are involved is extra hard, because you can't even trust fundamental operations like assignment or copying.

Please note that the allocator stuff is just one of the three violations I had pointed out. You've already pushed a fix for the unsafe .stringz, but you haven't addressed unsafe copy constructors yet.

And my list wasn't meant to be complete. There may be other safety holes I didn't notice.
May 29, 2019
On Monday, 27 May 2019 at 14:26:16 UTC, ag0aep6g wrote:
> Oh, yeah. Getting @trusted right is hard. Getting it right when user-provided types are involved is extra hard, because you can't even trust fundamental operations like assignment or copying.

In my point of view @trusted means "I use pointer-related operations correctly. Also I am using all @system interfaces correctly". The code in question uses allocator interface correctly. User of this code has a contract to supply allocator that conforms to this interface. If a user supplies mallocator that is not correct, there are two possibilities:

- Allocator is buggy. Nothing to do with @trusted code.
- Allocator do not conforms to allocator interface. User has broken the contract. Nothing to do with @trusted code.

I think we should keep in mind not only technical aspects of @trusted and @system, but this contract too.