On Sat, Feb 07, 2015 at 08:15:05PM -0800, Andrei Alexandrescu via Digitalmars-d wrote:
> On 2/7/15 8:00 PM, H. S. Teoh via Digitalmars-d wrote:
> >On Sat, Feb 07, 2015 at 06:19:19PM -0800, Andrei Alexandrescu via Digitalmars-d wrote: [...]
> >>private @system T[] mallocUninitializedArrayImpl(T)(size_t n)
> >>{
> >>     auto p = malloc(n * T.sizeof);
> >>     p || assert(0, "Not enough memory");
> >
> >This is a truly strange way of writing it... why not:
> >
> >	assert(p !is null, "Not enough memory");
> >
> >?
> 
> assert(0) is not removed in release mode. -- Andrei

Ah, right. But shouldn't it be enforce instead of assert, then? :-P


T

-- 
In theory, software is implemented according to the design that has been carefully worked out beforehand. In practice, design documents are written after the fact to describe the sorry mess that has gone on before.

On 2/7/15 8:21 PM, H. S. Teoh via Digitalmars-d wrote:
> Come to think of it, is there any point in making malloc @safe/@trusted
> at all? I don't think it's possible to make free() @safe, so what's the
> purpose of making malloc callable from @safe code? Unless you make a
> ref-counted wrapper of some sort around it, in which case you might as
> well use RefCounted instead.

Same goes about e.g. fopen vs. fclose. I'm thinking just of increasing the quantity of safe code. -- Andrei

On Sunday, 8 February 2015 at 04:45:55 UTC, Andrei Alexandrescu wrote:
> On 2/7/15 8:21 PM, H. S. Teoh via Digitalmars-d wrote:
>> Come to think of it, is there any point in making malloc @safe/@trusted
>> at all? I don't think it's possible to make free() @safe, so what's the
>> purpose of making malloc callable from @safe code? Unless you make a
>> ref-counted wrapper of some sort around it, in which case you might as
>> well use RefCounted instead.
>
> Same goes about e.g. fopen vs. fclose. I'm thinking just of increasing the quantity of safe code. -- Andrei

So are you going for this:

http://forum.dlang.org/thread/ovoarcbexpvrrceysnrs@forum.dlang.org

?

H. S. Teoh:

> Come to think of it, is there any point in making malloc @safe/@trusted at all?

I am not asking for a @trusted function. I'd like a @system template wrapper for malloc/calloc/free that is safer than the C functions (safer because it's type-aware).

Bye,
bearophile

> come to think of it, is there any point in making malloc @safe/@trusted
> at all? I don't think it's possible to make free() @safe, so what's the
> purpose of making malloc callable from @safe code?

At least two programs, widely used by folks here, never release their memory. Those could be made @safe.

Am Sat, 07 Feb 2015 15:50:53 -0800
schrieb Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org>:

> I was looking into ways to make core.stdc safer. That should be relatively easy to do by defining a few wrappers. For example:

This might be a good idea, but it might also be more difficult than you think:

> 
> int  setvbuf(FILE* stream, char* buf, int mode, size_t size);
> 
> is unsafe because there's no relationship between buf and size. But this is fine:
> 
> @trusted int setvbuf(T)(FILE* stream, T[] buf, int mode)
> if (is(T == char) || is(T == byte) || is(T == ubyte))
> {
>      return setvbuf(stream, cast(char*) buf.ptr, mode, buf.length);
> }
> 

This can still cause memory corruption if `buf` is GC-allocated. You'd have to pin the buffer which might not be easy in such a low-level wrapper. OTOH in a higher level wrapper (std.stdio.File) you can simply keep a reference to the buffer.

> Another example is:
> 
> int stat(in char*, stat_t*);
> 
> which may start reading through random memory if the string is not zero-terminated. Again, the solution is here to ensure the string does have a terminating zero:
> 
> @trusted int stat(in char[] name, stat_t* p)
> {
>      if (isZeroTerminated(name)) return stat(name.ptr, p);

How would you implement `isZeroTerminated` in a memory safe way? We have
exactly the same problem in toStringz and nobody ever came up
with a really safe solution. The best you could do is using special
types for zero-terminated strings but that might be cumbersome to use.

>      auto t = cast(char*) malloc(name.length + 1);
>      scope(exit) free(t);
>      memcpy(t, name.ptr, name.length);
>      t[name.length] = 0;
>      return stat(t, p);
> }
> 
> Such wrappers would allow safe code to use more C stdlib primitives. The question is whether these wrappers are worth adding to core.stdc.stdio.
> 

That's the main question. There's only a limited amount of stdc functions which can be wrapped in a safe way and std.stdio etc. are already kind of a safe wrapper. And it's also important to get these wrappers right and make sure they don't introduce memory safety bugs.

On 2015-02-08 at 03:19, Andrei Alexandrescu wrote:
> Indeed we have no safe way to wrap free.

How about this to prevent double free:

Wrapped malloc keeps a static thread-local lookup structure for successful allocations (if having to release memory from the same thread is an acceptable requirement).

Wrapped free looks up the pointer in that lookup structure and, if found, frees memory, removes the lookup entry and sets the argument of the call to zero (if it was a pointer) or sets its length and ptr to zero (if it was a dynamic array).

It's not completely safe, but for that GC would have to be used instead.

On Sunday, 8 February 2015 at 12:43:38 UTC, FG wrote:
> On 2015-02-08 at 03:19, Andrei Alexandrescu wrote:
>> Indeed we have no safe way to wrap free.
>
> How about this to prevent double free:
>
> Wrapped malloc keeps a static thread-local lookup structure for successful allocations (if having to release memory from the same thread is an acceptable requirement).
>
> Wrapped free looks up the pointer in that lookup structure and, if found, frees memory, removes the lookup entry and sets the argument of the call to zero (if it was a pointer) or sets its length and ptr to zero (if it was a dynamic array).
>
> It's not completely safe, but for that GC would have to be used instead.

I don't have any data, but I'd image most double-frees come from multiple references to the same data, not repeated calls to free on the same reference.

On Sunday, 8 February 2015 at 12:43:38 UTC, FG wrote:
> On 2015-02-08 at 03:19, Andrei Alexandrescu wrote:
>> Indeed we have no safe way to wrap free.
>
> How about this to prevent double free:
>
> Wrapped malloc keeps a static thread-local lookup structure for successful allocations (if having to release memory from the same thread is an acceptable requirement).
>
> Wrapped free looks up the pointer in that lookup structure and, if found, frees memory, removes the lookup entry and sets the argument of the call to zero (if it was a pointer) or sets its length and ptr to zero (if it was a dynamic array).
>
> It's not completely safe, but for that GC would have to be used instead.

A typical C debug library trick for when the money for a proper tool isn't available.


--
Paulo

On 2/8/15 2:54 AM, Johannes Pfau wrote:
> Am Sat, 07 Feb 2015 15:50:53 -0800
> schrieb Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org>:
>> @trusted int setvbuf(T)(FILE* stream, T[] buf, int mode)
>> if (is(T == char) || is(T == byte) || is(T == ubyte))
>> {
>>       return setvbuf(stream, cast(char*) buf.ptr, mode, buf.length);
>> }
>>
>
> This can still cause memory corruption if `buf` is GC-allocated. You'd
> have to pin the buffer which might not be easy in such a low-level
> wrapper. OTOH in a higher level wrapper (std.stdio.File) you can simply
> keep a reference to the buffer.

Good point, thanks. Moving GCs didn't occur to me.

>> @trusted int stat(in char[] name, stat_t* p)
>> {
>>       if (isZeroTerminated(name)) return stat(name.ptr, p);
>
> How would you implement `isZeroTerminated` in a memory safe way? We have
> exactly the same problem in toStringz and nobody ever came up
> with a really safe solution. The best you could do is using special
> types for zero-terminated strings but that might be cumbersome to use.

I thought of a few things, nothing is 100% foolproof. But I'm not too worried - many of these functions issue system calls, and the cost of a malloc/free pulse is unlikely to be measurable. With opportunistic use of alloca it gets even better.

>>       auto t = cast(char*) malloc(name.length + 1);
>>       scope(exit) free(t);
>>       memcpy(t, name.ptr, name.length);
>>       t[name.length] = 0;
>>       return stat(t, p);
>> }
>>
>> Such wrappers would allow safe code to use more C stdlib primitives.
>> The question is whether these wrappers are worth adding to
>> core.stdc.stdio.
>>
>
> That's the main question. There's only a limited amount of stdc
> functions which can be wrapped in a safe way and std.stdio etc. are
> already kind of a safe wrapper. And it's also important to get these
> wrappers right and make sure they don't introduce memory safety bugs.

I see it as increased opportunity to rely on simple manually checkable low-level functions, both in Phobos and outside it. It seems there's merit in that.


Andrei