misplaced @trust?

H. S. Teoh via Digitalmars-d digitalmars-d at puremagic.com
Thu Feb 5 10:54:13 PST 2015


On Thu, Feb 05, 2015 at 11:50:18AM -0500, Steven Schveighoffer via Digitalmars-d wrote:
> Reading the thread in bug report
> https://issues.dlang.org/show_bug.cgi?id=14125 gets me thinking,
> perhaps @trusted is mis-designed.

That's the feeling I'm getting too!

Well, to be fair, I can see why it arose in its current form
historically, but history is not justification for mis-design.


> At the moment, a @trusted function is the same as a @system function,
> but is allowed to be called from a @safe function. And that's it.
> 
> But a @trusted function may only need to be marked @trusted because of
> a few lines of code. So we now use this mechanism where we mark the
> @trusted portions with a lambda or static nested function, and call
> that internally, and mark the entire function @safe.

The more I think about it, the more I'm becoming convinced that @trusted
is a misfeature. Basically, it's a blanket permission for a function to
perform arbitrary @system operations and yet hide under the sheep's
clothing of being callable from @safe code.  While that may work for
trivial 4-line functions, it quickly becomes a maintenance nightmare
when a large, complex function is marked @trusted.

Consider, for example, if trustedFunction() calls helper functions
helper1(), helper2(), and helper3(), that are currently marked @safe.
The last review of trustedFunction() verified its safety, *keyed on the
fact that helper1, helper2, and helper3 are @safe*. Now, helper1,
helper2 and helper3 are not directly related to trustedFunction; they
are merely general utilities that trustedFunction depends on. So people
may make changes to them later on, without realizing the implications
they may have on the trustworthiness of trustedFunction(). One of these
changes may, inadvertently or otherwise, make helper1() @system instead
of @safe. Note that with template attribute inference, this can even be
an *unconscious* change. However, since helper1() is a general utility,
nobody realizes that trustedFunction's manual proof of safety has been
compromised. As a result, the PR gets merged, and now trustedFunction()
is no longer trustworthy but is still marked @trusted.

Worse yet, it may not be helper1() directly that breaks the
trustworthiness of trustedFunction(), but helper1 calls helper4 which
calls helper5 that in turn calls helper6. Due to some obscure change in
helper6, which used to be @safe, it now becomes @system, and because of
that helper5, helper4, and helper1 all become @system. But since
trustedFunction() is allowed to call @system functions without
restriction, the breakage goes completely unnoticed. Even a thorough
review of trustedFunction may not detect this problem, unless the
reviewer recursively reviews ALL dependencies of trustedFunction.

Now, if @trusted functions are *still* under the restrictions of @safe
code, except small parts explicitly marked to require human
verification, then if helper1 is outside the marked section, as soon as
helper6 changes in safety, trustedFunction no longer compiles. This at
least provides us with some safety net in case things go wrong.

However, the problem still remains. No matter how confined those
explicitly marked sections of code may be, they are still subject to the
above indirect breach of trust problem. I see no real solution to this.


[...]
> At least with the @trusted inner function/lambda, you limit yourself
> to the code that has been marked as such, and you don't need to worry
> about the actual @safe code that is added to the function.

Actually, the @trusted inner function is an abuse of @trusted. They are
not trustworthy AT ALL, unless, as Walter said on the bug comments, they
present an API that CANNOT be made to break safety, no matter what
arguments you give it. The current implementation of wrapping &ptr in a
@trusted inner function that simply turns the & operator into a @safe
operation, is a completely wrong solution. Consider:

	auto trustedFunc(ref int x) @safe {
		ref int trustedDeref(int* x) @trusted {
			return *x;
		}
		auto p = &x;
		trustedDeref(p) = 999;
	}

On the surface, this looks good, since the dangerous operation inside
trustedFunc has been overtly marked as such, so reviewers will carefully
review it to make sure it's correct... except, it *cannot* be correct,
because it's assuming that its CALLER doesn't pass invalid arguments to
it. Somebody could easily change the code to:

	auto trustedFunc(ref int x) @safe {
		ref int trustedDeref(int* x) @trusted {
			return *x;
		}
		int* p;		// <--- N.B.: new change, now p is null
		trustedDeref(p) = 999; // arghhh....
	}

The compiler continues to accept it blindly, even though it's now
blatantly wrong.

While the *idea* of marking out specific sections of code inside a
@trusted function for scrutiny is valid, the above approach is NOT the
right way to go about implementing it.

Rather, what *should* have been done, is that trustedFunc should be
marked @trusted, but the compiler STILL imposes @safe restrictions on
the function body, except for explicitly-marked blocks inside. To use
hypothetical syntax, it should look something like this:

	auto trustedFunc(ref int x) @trusted { // <-- @trusted to indicate need of review
		int *p = &x;
		@system {	// indicate that code inside this block needs manual verification
			*p = 999;
		}
		//*p = 888;	// Illegal: @system operations not allowed outside @system block
	}

IOW, the entire function is marked @trusted to indicate that it needs
review, but the function body is STILL under @safe restrictions. So
@trusted becomes the same as @safe, except that it permits @system
blocks inside, and @system operations must be confined to these blocks.


> Or do you?... the problem with this mentality is interactions with
> data inside the @safe code after a @trusted function is called can
> still be subject to memory safety issues. An example (using static
> functions for clarity):
> 
> void foo() @safe
> {
>    static auto tmalloc(size_t x) @trusted {return (cast(int *)malloc(x *
> sizeof(int)))[0..x];}
>    static void tfree(void[] arr) @trusted {free(arr.ptr);}
> 
>    auto mem = tmalloc(100);
>    tfree(mem);
>    mem[0] = 5;
> }
> 
> Note that the final line in the function, setting mem[0] to 5, is the
> only unsafe part. it's safe to malloc, it's safe to free as long as
> you don't ever refer to that memory again.
> 
> But the problem with the above is that @trusted is not needed to apply
> to the mem[0] = 5 call. And imagine that the mem[0] = 5 function may
> be simply added, by another person who didn't understand the context.
> Marking the whole function as safe is kind of meaningless here.

Exactly, this is a totally wrong approach to implementing maintainable
@trusted functions. The function should not be marked @safe because it
is actually @trusted, not @safe. The inner functions tmalloc and tfree
are mis-attributed as @trusted, when they are actually @system.


> Changing gears, one of the issues raised in the aforementioned bug is
> that a function like this really should be marked trusted in its
> entirety. But what does this actually mean?
> 
> When we mark a function @safe, we can assume that the compiler has
> checked it for us. When we mark a function @trusted, we can assume the
> compiler has NOT checked it for us. But how does this help? A @safe
> function can call a @trusted function. So there is little difference
> between a @safe and a @trusted function from an API point of view. I
> would contend that @trusted really should NEVER HAVE BEEN a function
> attribute. You may as well call them all @safe, there is no
> difference.

Yes, @trusted in its current incarnation is fundamentally flawed.

However, I don't agree that the entire function can be marked @safe.
Otherwise, @safe code will now contain arbitrary @trusted blocks inside,
and so anybody can freely escape @safe restrictions just by putting
objectionable operations inside @trusted blocks. The function still
needs to be marked @trusted -- to draw attention for the need of
scrutiny -- *but* the function body is still confined under @safe
requirements, except that now the "escape hatch" of @trusted code blocks
are permitted as well.


> What I think we need to approach this correctly is that instead of
> marking *functions* with @trusted, we need to mark *data* and *calls*
> with @trusted.  Once data is used inside a @trusted island, it becomes
> tainted with the @trusted mark. The compiler can no longer guarantee
> safety for that data.
> 
> So how can we implement this without too much pain? I envision that we
> can mark lines or blocks inside a @safe function as @trusted (a
> @trusted block inside a @system function would be a no-op, but
> allowed).

I think it's better to keep @safe as-is, no @trusted blocks are allowed
inside @safe code. Instead, change @trusted to mean "@safe by default,
but now allow @trusted blocks for performing operations that must be
manually verified". Any function that contains these "escape blocks"
can no longer be marked @safe, because they now require manual
verification.


> I also envision that any variable used inside the function is
> internally marked as @safe until it's used in a @trusted block, and
> then is marked as @trusted (again internally, no requirement to mark
> in syntax). If at any time during compilation a @trusted variable is
> used in @safe code, it's a compile error.
> 
> There may be situations in which you DO need this to work, and in
> those cases, I would say a cast(@safe) could get rid of that mark. For
> example, if you want to return the result of a @trusted call in a
> @safe function.
> 
> Such a change would be somewhat disruptive, but much more in line with
> what @trusted calls really mean.
> 
> I've left some of the details fuzzy on purpose, because I'm not a
> compiler writer :)
[...]

I like the idea of tainting data as @system (for lack of a better term).
This increases the compiler's ability to catch mistakes, and does not
require completely turning off @safe checks inside @trusted functions. I
think it was a grave mistake for @trusted to completely turn off all
@safe checks. @trusted functions should still be under @safe
restrictions, and any unsafe operations must be explicitly marked as
such before being allowed.


T

-- 
I am not young enough to know everything. -- Oscar Wilde


More information about the Digitalmars-d mailing list