On 2/6/2015 12:31 AM, Kagamin wrote:
> On Thursday, 5 February 2015 at 23:39:39 UTC, Walter Bright wrote:
>>   static void trustedMemcopy(T[] dest, T[] src) @trusted
>>   {
>>     assert(src.length == dest.length);
>>     memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
>>   }
>
> Should be enforce: assert doesn't guard against malicious usage.

Cue my endless attempts to explain the difference between input errors and logic errors :-(

On Friday, 6 February 2015 at 08:58:05 UTC, Walter Bright wrote:
> On 2/6/2015 12:31 AM, Kagamin wrote:
>> On Thursday, 5 February 2015 at 23:39:39 UTC, Walter Bright wrote:
>>>  static void trustedMemcopy(T[] dest, T[] src) @trusted
>>>  {
>>>    assert(src.length == dest.length);
>>>    memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
>>>  }
>>
>> Should be enforce: assert doesn't guard against malicious usage.
>
> Cue my endless attempts to explain the difference between input errors and logic errors :-(

So which one is it?

On one hand, it is clearly a logic error - passing arrays of different length is clearly a program bug.

On the other hand, this is a library function, and as you said, we can't know how it's going to be used - so the check has to be unconditional.

On 2/6/2015 1:01 AM, Vladimir Panteleev wrote:
> On Friday, 6 February 2015 at 08:58:05 UTC, Walter Bright wrote:
>> Cue my endless attempts to explain the difference between input errors and
>> logic errors :-(
>
> So which one is it?

http://www.digitalmars.com/d/archives/digitalmars/D/Program_logic_bugs_vs_input_environmental_errors_244143.html

On Friday, 6 February 2015 at 10:28:38 UTC, Walter Bright wrote:
> On 2/6/2015 1:01 AM, Vladimir Panteleev wrote:
>> On Friday, 6 February 2015 at 08:58:05 UTC, Walter Bright wrote:
>>> Cue my endless attempts to explain the difference between input errors and
>>> logic errors :-(
>>
>> So which one is it?
>
> http://www.digitalmars.com/d/archives/digitalmars/D/Program_logic_bugs_vs_input_environmental_errors_244143.html

Do you want to say, that safe code can still corrupt memory if it's crafted by a malevolent programmer or if it doesn't perform additional domain-specific logic checks of otherwise memory-safe data?

On Friday, 6 February 2015 at 08:58:05 UTC, Walter Bright wrote:
> On 2/6/2015 12:31 AM, Kagamin wrote:
>> On Thursday, 5 February 2015 at 23:39:39 UTC, Walter Bright wrote:
>>>  static void trustedMemcopy(T[] dest, T[] src) @trusted
>>>  {
>>>    assert(src.length == dest.length);
>>>    memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
>>>  }
>>
>> Should be enforce: assert doesn't guard against malicious usage.
>
> Cue my endless attempts to explain the difference between input errors and logic errors :-(

I agree with you, that this should be an assert, but then it cannot be @trusted. A @trusted function should provide it's guarantee for all input that can be crafted in @safe code. This is why this


void bar()
{
    ubyte[] a;
    a.ptr = 0; // arbitrary value as long as NOT from an allocator
    a.len = 1;

    setToZero(a);
}

is not a valid counter-example and that memcpy cannot be @trusted. It has no safe interface.


Back to topic. Since @trusted must provide a safe interface, I don't think it makes much sense to attach @trusted to smaller code blocks than functions. That might not be true for the opposite: @safe blocks where the compiler turns the safety check back on.

This prevents that a @trusted/@safe function becoming @system breaks the @trusted guarantee of called functions.

void bar(void* argument) @trusted { ... }

void forward() @trusted
{
    // REVIEW: forwards arguments to bar, which is @trusted, so we can
    // trust forward as well
    void* p = malloc(...);
    bar(arguments);
    free(p);
}

Changing bar to @system is a breaking change and currently a silent one. This is a maintenance problem I see as well. Solution:

void forward() @trusted
{
    void* p = malloc(...);
    // safe is bar is safe, so turn compiler checks on again.
    @safe  { bar(p); }
    free(p);
}


Introducing an error in an @trusted function is _not_ an maintenance horror. While your @safe-ty guarantee is out of the window, it can be catched my reviewing one function: the one where the error is introduced in. Making a @trusted function @system forces us to review all @trusted functions calling the now @system function.

>
> This prevents that a @trusted/@safe function becoming @system breaks the @trusted guarantee of called functions.
>

Calling function, forward in the example.

If I understand it correctly, Walter is against adding trusted blocks (trusted {...}) into @safe functions. But what about having safe blocks in @trusted functions ?

int somefunc() @trusted
{
   int a = systemfunc();
   int b;

   @safe {
      b = safefunc(); // calling systemfunc() not allowed;
   }

   return a + b;
}

On Friday, 6 February 2015 at 08:58:05 UTC, Walter Bright wrote:
> On 2/6/2015 12:31 AM, Kagamin wrote:
>> On Thursday, 5 February 2015 at 23:39:39 UTC, Walter Bright wrote:
>>>  static void trustedMemcopy(T[] dest, T[] src) @trusted
>>>  {
>>>    assert(src.length == dest.length);
>>>    memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
>>>  }
>>
>> Should be enforce: assert doesn't guard against malicious usage.
>
> Cue my endless attempts to explain the difference between input errors and logic errors :-(

A little offtop: if this function is compiled in release mode and compiler assumes assert holds, it's free to use dest.length instead of src.length and if at runtime dest is longer than src, this will create heartbleed-like bug in safe code.

On Friday, 6 February 2015 at 10:28:38 UTC, Walter Bright wrote:
> On 2/6/2015 1:01 AM, Vladimir Panteleev wrote:
>> On Friday, 6 February 2015 at 08:58:05 UTC, Walter Bright wrote:
>>> Cue my endless attempts to explain the difference between input errors and
>>> logic errors :-(
>>
>> So which one is it?
>
> http://www.digitalmars.com/d/archives/digitalmars/D/Program_logic_bugs_vs_input_environmental_errors_244143.html

That doesn't answer my question.

A few years ago, I recall, you were arguing that for functions which are or may be exported to a DLL, thus all Phobos functions, it is impossible to predict how the functions will be used. Thus, you argued, the functions' input has to be validated, even if invalid parameters can only be passed to the function as a result of a program bug, and never user input.

So, to repeat my question: which one is it? Have you changed your mind, or are there exceptions to the rules in the post you quoted?

First, I want to say that I didn't want to cause a huge rift between D developers with this, I didn't think this was such a drastic issue, just a possible idea. But here we are. I hope we can mend this, and move forward. But on to the discussion.

On 2/5/15 6:39 PM, Walter Bright wrote:
> Consider the following code excerpted from std.array.join:
>
>    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }
>    return trustedCast!RetType(result);
>
> This is because the compiler would complain that the following line
> would not be @safe:
>
>    return cast(RetType)(result);
>
> The rationale is "I know it's safe, so I'll create an @trusted wrapper
> to eliminate the error." What comes next is "that's cumbersome. How
> about a better syntax:"
>
>    trusted {
>       return cast(RetType)(result);
>    }

No. This was NEVER THE ARGUMENT.

Now, let me explain why the latter is BETTER.

It's better because I know where it is used. It's used in one place, and I can squash it right there saying "No, you can't do this in this one place." Instead of reviewing an API in ALL POSSBILE CONTEXTS (which if trustedCast is a public API, would be a lot), I have to review one call in ONE CONTEXT.

The former is WORSE because it can be used in 100 places. Now I have to go through and fix ALL THOSE FUNCTIONS that use it, because its interface was exposed to the whole of phobos.

The problem, as we have said many times, is maintenance. Sure, in both cases they never should have shown up in the first place. But there they are, and we now have to fix them. All of them.

And let's also talk about long term maintenance. Any time a @trusted function is amended or tweaked, we have to reconsider all the contexts. If you mark a single line as @trusted, then additional lines to the same function would not need as much scrutiny, especially if you warn when @trusted tainted data is touched.

> The trouble with it is, what if the cast is converting from an integer
> to a pointer? That could lead to memory corruption. The code allows a
> potentially memory corrupting operation to be inserted into code that is
> otherwise @safe.

And so, you reject that code in both cases, except in the case where you don't define a callable API, you don't have to worry about any other code or contexts. This has to be the worst example to explain your point.

> The only way to deal with it is to then manually review everything about
> 'RetType' and 'result' to prove to oneself that it is not converting
> random bit patterns into pointers. In other words, one is manually
> reviewing @safe code for memory corruption errors.

@trusted code is not @safe code. You are reviewing that one line in its current context, not the whole function to see where it is called in various contexts.

> The solution is to regard @trusted as a means of encapsulating unsafe
> operations, not escaping them. Encapsulating them means that the
> interface from the @trusted code is such that it is usable from safe
> code without having to manually review the safe code for memory safety.
> For example (also from std.array):
>
>    static void trustedMemcopy(T[] dest, T[] src) @trusted
>    {
>      assert(src.length == dest.length);
>      memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
>    }
>
> I don't have to review callers of trustedMemory() because it
> encapsulates an unsafe operation (memcpy) with a safe interface.

Sure. But let's consider it's called in one place:

trustedMemcopy(array[pos..pos+to_insert], tmp);

What if that became:

assert(tmp.length == to_insert); // same as src.length == dest.length
@trusted memcpy(array.ptr + pos, tmp.ptr, tmp.length);

What is missing here is, make sure the type of array and tmp is the same. All one has to do is review the function to see that. You could put in a static assert if you wish:

static assert(is(typeof(array) == typeof(tmp)));

If there are multiple places that it's called from, sure we can encapsulate that in a function:

static void trustedMemcopy(T[] dest, T[] src)
{
   assert(src.length == dest.length);
   @trusted memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
}

There is very little difference here. Except if I add code to my version of trustedMemcopy that is @system, I have to properly mark that too, and that should imply greater scrutiny. I fail to see why making extra unintended @system calls inside a "trusted" function shouldn't require extra marking. Consider if this is a github review, and the context for the new lines is missing (i.e. you don't see that the new line is inside a @trusted function because github doesn't give you that line).

> The reason @trusted applies only to functions, and not to blocks of
> code, is that functions are the construct in D that provides an
> interface. Arbitrary blocks of code do not have a structured interface.
> Adding @trusted { code } support will encourage incorrect uses like the
> opening example. The existence of @trusted blocks will require review of
> every line of code in the function that encloses it, and transitively
> every function that calls it!

This is like opposite land! You don't have to review every function that calls a @safe function with @trusted escapes any more than you have to review every function that calls a @trusted function! That is the point of having the attribute on the function call.

However, if @trusted is improperly placed on a function, then I have to break every function that calls it if it now becomes @system. While the same would be for a @safe function that has incorrectly escaped @trusted calls, the temptation to make those small calls into a nice neat public API without context is not there.

Having @trusted as a function attribute *encourages* bad encapsulation of unsafe calls WITHOUT CONTEXT. This is so easy to do with templates.

> Adding @trusted as a function attribute, on the other hand, only
> requires review of the function's interface to determine if it is
> acceptable to use in safe code. Safety review of its callers is
> unnecessary.

I see zero difference between a @trusted function and a @safe function with @trusted escapes. You have to review both with the same scrutiny. Except less so for the one with escapes because you only have to scrutinize the @trusted calls.

You know, I'm surprised you have rejected H.S. Teoh's proposal because it seems like you are stuck on the concept that nobody should have to review @safe code for memory problems. Perhaps marking functions @trusted and then having escapes with @system is a better way of looking at it, because you do have to review that code for memory problems.

The bottom line of my reasoning is that code changes over time, by different people. Context is forgotten. It's much better to have the compiler verify you know what you are doing when working with @trusted than it is to just allow anyone to inject code anywhere.

-Steve