fix Issue 18016 - using uninitialized value is considered @safe but has undefined behavior

[WalterBright]

No description provided.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18016	normal	using uninitialized value is considered @safe but has undefined behavior

[PetarKirov]

Implementation defined means that the implementation must define the behavior, meaning that all D compilers must have this defined in their docs, which is obviously not the case. Hence why I think this should remain undefined.

[PetarKirov]

As for the bug report in question, I think void initialized variables should not be usable in @safe code, unless we add data flow to the frontend which can prove the safety, which is not planned for near future.
(Because while it can be argued that this is memory safe, it is neither type-safe, nor something that we should allow to be easily overlooked in code review, due to the huge potential for logic errors.)

[WalterBright]

The fact that the compilers don't define the behavior doesn't make it undefined according to the language spec. That's a bug against the implementation, not the spec.

As for safety, that means memory safety. Ints with random values are not memory unsafe.

[PetarKirov]

Implementation defined behavior is something that you can rely on, given that you make sure you understand the differences between the implementations (if you want to make your code portable). Are you saying that you can rely to find particular values in a void initialized static array like char[128] arr = void; even when compiling with optimizations? I suspect that some optimizing backends like GCC and LLVM may consider such code (after data flow analysis) as having undefined behavior and subsequently generate arbitrary behaving programs.

[WalterBright]

Implementation defined value means the implementation defines what the value is, which may include "arbitrary bit pattern". Undefined behavior is altogether different, as it would include "execute arbitrary code". If the implementation does the latter, it is bug in that implementation.

[JinShil]

This seems to imply that the implementation is free to define the contents of a void-initialized variable. The implementation may choose to define it as 0. I suppose a security-hardened implementation may choose to do that. The implementation could also define it to be "whatever value was last written to that memory location either by the currently running process or some other process", but that would still be defined. I'm leaning towards @WalterBright's interpretation.

I suspect that some optimizing backends like GCC and LLVM may consider such code (after data flow analysis) as having undefined behavior and subsequently generate arbitrary behaving programs.

That's what's kindof weird about this. If it is implementation-defined, then isn't the implementation free to define it as undefined behavior?

[TurkeyMan]

I'm nervous that you think this is @safe... why would you say code that interacts with verifiably uninitialised data is cool and normal? It could do anything as a side-effect, and it's basically a gift-wrapped invitation to anyone crafting exploits.

[tgehr]

@WalterBright If the value is implementation defined, the implementation needs to define what the value is for each type will be if you use void-initialization. The point of void-initialization is to not initialize the memory. How can an implementation correctly specify what the value should be if it reuses random memory from other parts of the program? This change is bad, it requires implementations to default-initialize void-initialized variables.

[WalterBright]

C compilers leave it implementation defined, and indeed some compilers leave it as whatever happens to be in that memory location, and others set it to zero. I think that's quite reasonable, and it isn't necessary to define it further.

[joseph-wakeling-frequenz]

C compilers leave it implementation defined

Sorry to drop in at random (the PR got linked from a forum discussion thread), but just as a point of fact: the C99 spec §6.7.8 "Initialization" para 10 states: If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate.

§3.17.2 defines an "indeterminate value" as "either an unspecified value or a trap representation". And §3.17.1 explicitly defines "implementation defined" as "unspecified value where each implementation documents how the choice is made".

[aG0aep6G]

With this change, compilers would still be allowed to leave the variable uninitialized, right? That means using uninitialized values would be allowed in D (as long as they're not interpreted as pointers). That would be a problem for pure functions. See issue 15542 (pure function with no argument returning different values (with void-initialized static array)).

[WalterBright]

Pure functions returning void initialized data is still implementation defined behavior, even in @safe code.

[PetarKirov]

I don't think it's reasonable to allow turning @safe pure functions into essentially bad PRNGs and make potential purity based optimizations into undefined behavior magnifiers. I'm sure there may be even reasoanle applications for using uninitialized memory, but all of them strictly belong to @system code. Pure functions in D are allowed a lot more freedom compared to functions in pure functional languages, but this is all based on the premise that their implementation details don't leak. Otherwise pure may become as meaningless as are const parameters in C++.

[WalterBright]

I don't think it's reasonable to allow turning @safe pure functions into essentially bad PRNGs and make potential purity based optimizations into undefined behavior magnifiers.

It's implementation defined, not undefined. I don't think this is fundamentally different from returning allocated pointers from pure functions. The pointer values will be different each time, even though the function's arguments are identical.

[RazvanN7]

What's the status on this?

[tgehr]

I think this is the wrong way to fix this problem, because it leaves gaping @safe-ty holes. Also see:
https://issues.dlang.org/show_bug.cgi?id=19968

Note that issue 19968 is a symptom of a general problem. Allowing uninitialized memory to be read in @safe functions is a bad idea, and shouldn't be allowed by default.

[WalterBright]

Note that issue 19968 is a symptom of a general problem.

As I noted in the issue, it is a problem specific to bools.

[tgehr]

This is NOT a problem specific to bools!

• The language specification states that accessing void-initialized data is UB.
• This can be used to fill private fields of structs that have @trusted functions working on them with garbage.

[WalterBright]

The language specification states that accessing void-initialized data is UB.

Indeed, and hence this PR to fix that.

This is NOT a problem specific to bools!

I would appreciate an example of UB from other non-pointer types, as I can't think of one.

There's a reason I push on this. Consider:

char[100] tmpbuffer = void;
... code to use part of the buffer ...

It's a performance issue. I don't want to relegate this to @trusted code only.

[tgehr]

The language specification states that accessing void-initialized data is UB.

Indeed, and hence this PR to fix that.

This is NOT a problem specific to bools!

I would appreciate an example of UB from other non-pointer types, as I can't think of one.
...

module data;
int[N] array;
static assert(0<N);

struct VerifiedIndex{
    private int index_=0;
    @disable this(int); // shouldn't be necessary, but it is
    @property int index()@safe{ return index_; }
    @property void index(int i)@trusted{ // no, this can't be @safe
        enforce(0<=x&&x<N, "index out of bounds");
        index_=i;
    }
    int read()@trusted{ return array.ptr[index]; }
    void write(int x)@trusted{ array.ptr[index]=x; }
} 

module app;
import data;

void main()@safe{
    VerifiedIndex i = void;
    i.write(1337);
}

There's a reason I push on this.

I understand, but no memory corruption in @safe code trumps performance in @safe code every time. "I want this to be possible in @safe code" is never more important than "@safe guarantees no memory corruption".

Consider:

char[100] tmpbuffer = void;
... code to use part of the buffer ...

It's a performance issue. I don't want to relegate this to @trusted code only.

• The spec can define a certain subset of types for which this is @safe in impure functions. E.g., for structs, the constraint can be that void initialization is @safe for all fields, and all fields are public.
• However, why can't you encapsulate this pattern into a struct with a @trusted interface? You need to do this anyway if you want to use the tmpbuffer for non-POD data.

[schveiguy]

Just directed here from a recent forum discussion. Reading Timon's example, it makes it difficult to justify this change.

There are a couple options I can think of to address this. First, and a good possibility, is to allow only POD types to be initialized as void. Anything that has methods, or contains any types that have methods, would be prevented from being void initialized in @safe code, due to the fact that those methods can be made to circumvent safety (via @trusted blocks or methods).

Second option is to somehow allow the type to opt-in to void initializability. Can't think of a good syntax, but this only slightly gives more discretion. Currently, if you void initialize a member, it does nothing, as the whole type is initialized anyway.

However, it may simply be prudent to keep the current rules and fully implement them in the compiler (i.e. void initialization is not allowed in safe code), because Timon is right -- any possibility of corruption with @safe code should be prevented. We can relax the rules later. The only thing that really sucks about this is, it's not possible to create a buffer inside a trusted escape -- the whole function must be trusted. Is there a way around this?

[tgehr]

...
The only thing that really sucks about this is, it's not possible to create a buffer inside a trusted escape -- the whole function must be trusted. Is there a way around this?

As I stated earlier, you could do this:

struct TmpBuffer(T,size_t n){
    private T[n] buffer=void;
    // ... add @trusted interface here that ensures data is initialized before use
}

Also, given that accessing uninitialized memory is made defined (but non-deterministic) behavior, void-initialization can be kept @safe for all types for which @safe code can put any bit pattern into the memory occupied by the type without void-initialization. This does not include bool and it does not include structs that maintain some internal invariant.

[schveiguy]

you could do this

I don't know if that works, though. When I declare an instance of TmpBuffer, it initializes it to 0s.

[tgehr]

you could do this

I don't know if that works, though. When I declare an instance of TmpBuffer, it initializes it to 0s.

https://issues.dlang.org/show_bug.cgi?id=11331

[schveiguy]

Right, but that just means you can't use your proposed workaround/solution. Again, is there a way around it?

[ntrel]

@schveiguy:

it's not possible to create a buffer inside a trusted escape -- the whole function must be trusted

Then wrap the rest of the function in a safe function to enable checking again:

@trusted foo() {
T[n] buffer = void;
() @safe {
// Manipulate buffer
...
}();
}

[schveiguy]

@ntrel a possibility, but not practical for templates where you want everything inferred.

[ntrel]

@schveiguy sounds like a job for your default(@safe) attribute then ;-)

[schveiguy]

I'm not sure that solves the problem. For example, if you want foo to be inferred @system for any @system usage inside the lambda, but want to trust just the void initialization, marking the lambda default(@safe) does not alter the @trusted marking on foo.

[ntrel]

@schveiguy Yes, you're right that doesn't solve it. I think it could be done something like this:

// user code
void parent(...) {
    void work(alias safeVoidBuffer) {...}
    mixin UseSafeVoidBuffer!(T, n, work);
}

// library
module safevoidbuf;

struct SafeVoidBuffer(T, size_t n) {...}

template UseSafeVoidBuffer(T, size_t n, alias worker) {
    () @trusted {
        SafeVoidBuffer!(T, n) buf = void;
        buf.initSafely();
        worker!buf();
    }();
    static if (isSystem!(worker!(...))) // assuming we can implement isSystem
        if (0) doSomethingSystem; // force system inference for parent function
}

So it might be doable. But supporting void initialization for struct fields seems like the best fix, avoiding the need for a nested function and mixin.

[tgehr]

But supporting void initialization for struct fields seems like the best fix, avoiding the need for a nested function and mixin.

Yes.

[RazvanN7]

I don't understand how we can honestly make an argument that void initialized variables are @safe. We put ourselves on a pedestal because we initialize variables by default (whereas C and C++ have a lot of safety issues for not doing it), but when it comes to void initialization (which is literally a mean to have uninitialized variables) we consider it @safe. This is a glaring contradiction and in my opinion the fix should be to disallow void initialization in @safe code.

[RazvanN7]

I would appreciate an example of UB from other non-pointer types, as I can't think of one.

@edi33416 and I have master student that tries to find holes in D's safety guarantees. He has been able to obtain random code execution from overlapping the stack with the heap by allocating a large void initialized static array on the stack. Something similar to: https://issues.dlang.org/show_bug.cgi?id=17566 .

@WalterBright please reconsider this. At the very least, it should be illegal to have void initialized stack allocated static arrays.

[pbackus]

I don't understand how we can honestly make an argument that void initialized variables are @safe.

A void-initialized variable can be @safe if:

1. Reading from it is guaranteed by the compiler and the language spec to result in an unspecified value rather than undefined behavior.
2. Every possible value of that variable's type is a safe value.

[ljmf00]

I don't think it's reasonable to allow turning @safe pure functions into essentially bad PRNGs and make potential purity based optimizations into undefined behavior magnifiers. I'm sure there may be even reasoanle applications for using uninitialized memory, but all of them strictly belong to @system code. Pure functions in D are allowed a lot more freedom compared to functions in pure functional languages, but this is all based on the premise that their implementation details don't leak. Otherwise pure may become as meaningless as are const parameters in C++.

I agree. Pure in D is becoming meaningless with this type of behaviours.

void main() @safe
{
    auto voidInitialize(size_t Tsize)() @safe pure
    {
        ubyte[Tsize] _ = void;
        void[Tsize] ret = _[];
        return ret;
    }
    
    import std.stdio : writeln;
    voidInitialize!100.writeln;
    voidInitialize!100.writeln;
    voidInitialize!100.writeln;
}

This should simply not be an accepted code.

Weak purity rules can be checked if any reference is passed to the function, but void initializers in a pure function allow this code to be considered pure, which cannot be checked at all from an outside scope, as you do by reading the function signature and check if any reference is being passed and possibly changed.

I don't understand how we can honestly make an argument that void initialized variables are @safe. We put ourselves on a pedestal because we initialize variables by default (whereas C and C++ have a lot of safety issues for not doing it), but when it comes to void initialization (which is literally a mean to have uninitialized variables) we consider it @safe. This is a glaring contradiction and in my opinion the fix should be to disallow void initialization in @safe code.

You miss understanding the @safe behaviour. Void initializers if not stated as undefined behaviour and doesn't initialize a type with indirections produces valid safe values. Inconsistent program state is different from @safe.

---

My opinion on this is that pure functions shouldn't allow returning non-setted void initialized memory although, it is perfectly @safe, according to the @safe rules of D as long as the type is not a reference type or has indirections.

[ljmf00]

Does reference type wording cover types with indirections too?

[pbackus]

A better way to word this paragraph would be:

Void-initialization of variables whose types have unsafe values is not allowed in @safe code.

[ljmf00]

I would appreciate an example of UB from other non-pointer types, as I can't think of one.

@edi33416 and I have master student that tries to find holes in D's safety guarantees. He has been able to obtain random code execution from overlapping the stack with the heap by allocating a large void initialized static array on the stack. Something similar to: issues.dlang.org/show_bug.cgi?id=17566 .

@WalterBright please reconsider this. At the very least, it should be illegal to have void initialized stack allocated static arrays.

I would still consider implementation defined or defined behaviour as this can be checked at runtime. Take the example of GCC flag -fstack-check and -fstack-clash-protection from LLVM. This prevents that stack clashing from occurring. And this code segfaults anyway with the default initializer. This code uses unsafe calls previously that may be considered undefined behaviour too because those calls don't provide a safe interface.

[aG0aep6G]

This code uses unsafe calls previously that may be considered undefined behaviour too because those calls don't provide a safe interface.

Not having a safe interface doesn't mean every call can be considered undefined behavior. Undefined behavior only comes up when the function is called incorrectly.

I'm not sure what code you're referring to. If it's issue 17566, then the only relevant call is the one to mmap. As far as I can tell, it's called correctly. If you think there's a problem with how mmap is called, please elaborate.

[ljmf00]

Not having a safe interface doesn't mean every call can be considered undefined behavior.

True, but there's a possibility. ("that may be considered undefined behaviour")

Undefined behavior only comes up when the function is called incorrectly.

No. If the function internally does something against D rules it may cause undefined behaviour like dereferencing a pointer that is pointing to an invalid object. See 12.1.1.5. point in the specification.

I'm not sure what code you're referring to. If it's issue 17566, then the only relevant call is the one to mmap. As far as I can tell, it's called correctly. If you think there's a problem with how mmap is called, please elaborate.

In the mmap call you are passing a reference that is invalid. You are manipulating a reference.

void* dst = stackBottom - sz;

mmap may internally dereference it and cast it to another type. Casting is unsafe (See 19.24.1.3. point in the specification) and dereferencing it is undefined behaviour as that reference is manipulated to be invalid. You can say that this is valid within the stack bounds but it is not guaranteed to be a semantically behaviour defined D program according to the specification.

[aG0aep6G]

Not having a safe interface doesn't mean every call can be considered undefined behavior.

True, but there's a possibility. ("that may be considered undefined behaviour")

No, there isn't a different kind of "possibility" of UB for unsafe interfaces. When used incorrectly, both safe and unsafe interfaces can exhibit UB. When used correctly, they both still can, but only due to bugs.

The difference between safe and unsafe interfaces is how a correct call is defined. For safe interfaces, it's a bunch of language rules. For unsafe interfaces, it's an agreement between the author of the function and the user.

Undefined behavior only comes up when the function is called incorrectly.

No. If the function internally does something against D rules it may cause undefined behaviour like dereferencing a pointer that is pointing to an invalid object. See 12.1.1.5. point in the specification.

If an instance of UB is not due to an incorrect call, then it's a bug. Bugs can cause UB in both safe and unsafe interfaces, so they're not relevant here.

In the mmap call you are passing a reference that is invalid. You are manipulating a reference.

void* dst = stackBottom - sz;

That's an unsafe operation. Doesn't mean it has undefined behavior. Passing a garbage pointer to an @system function is fine if that function doesn't dereference it (and doesn't expose it to @safe code, yada, yada, yada). It does not have more of "a possibility" of UB than calling an @safe function (which also has "a possibility" of dereferencing an invalid pointer due to a bug).

mmap may internally dereference it and cast it to another type.

It doesn't. You can read here what mmap does: https://man7.org/linux/man-pages/man2/mmap.2.html

I maintain: mmap is being used correctly. The call does not exhibit "a possibility" of undefined behavior that is worth mentioning in this context. It's the same "possibility" as when calling an @safe function.

[ljmf00]

Not having a safe interface doesn't mean every call can be considered undefined behavior.

True, but there's a possibility. ("that may be considered undefined behaviour")

No, there isn't a different kind of "possibility" of UB for unsafe interfaces. When used incorrectly, both safe and unsafe interfaces can exhibit UB. When used correctly, they both still can, but only due to bugs.

The difference between safe and unsafe interfaces is how a correct call is defined. For safe interfaces, it's a bunch of language rules. For unsafe interfaces, it's an agreement between the author of the function and the user.

What do you mean used incorrectly? A safe interface cannot exhibit undefined behaviour. Do you mean called with unsafe values?

I just assumed that possibility as mmap doesn't provide a safe interface, and, by thinking the other way around, unsafe interfaces may or may not exhibit undefined behaviour.

Undefined behavior only comes up when the function is called incorrectly.

No. If the function internally does something against D rules it may cause undefined behaviour like dereferencing a pointer that is pointing to an invalid object. See 12.1.1.5. point in the specification.

If an instance of UB is not due to an incorrect call, then it's a bug. Bugs can cause UB in both safe and unsafe interfaces, so they're not relevant here.

In the mmap call you are passing a reference that is invalid. You are manipulating a reference.

void* dst = stackBottom - sz;

That's an unsafe operation. Doesn't mean it has undefined behavior. Passing a garbage pointer to an @system function is fine if that function doesn't dereference it (and doesn't expose it to @safe code, yada, yada, yada). It does not have more of "a possibility" of UB than calling an @safe function (which also has "a possibility" of dereferencing an invalid pointer due to a bug).

mmap may internally dereference it and cast it to another type.

It doesn't. You can read here what mmap does: man7.org/linux/man-pages/man2/mmap.2.html

I maintain: mmap is being used correctly. The call does not exhibit "a possibility" of undefined behavior that is worth mentioning in this context. It's the same "possibility" as when calling an @safe function.

You are right. I thought that mmap would dereference that pointer. I read the documentation of mmap and I can agree with you.

[aG0aep6G]

The difference between safe and unsafe interfaces is how a correct call is defined. For safe interfaces, it's a bunch of language rules. For unsafe interfaces, it's an agreement between the author of the function and the user.

What do you mean used incorrectly? A safe interface cannot exhibit undefined behaviour. Do you mean called with unsafe values?

From @system and @trusted code, a safe interface can be called incorrectly. And then it can exhibit UB. As you say, passing unsafe values is how it happens.

Note that the spec doesn't say that a safe interface must be called only with safe values. Passing unsafe values is allowed. It's just the programmer's responsibility then to ensure no UB, as with all unsafe operations.

Example:

import std.stdio;
void main() @system
{
    char* p = cast(char*) 0xDEADBEEF; /* This is unsafe value. */
    char[] a = p[0 .. 3]; /* This is also an unsafe value. */
    writeln(p); /* Correct call of safe interface with unsafe value. Prints "DEADBEEF". */
    writeln(a); /* Incorrect call exhibits UB. Usually segfaults. */
}

(Assuming that I'm not missing some obscure unsafety in writeln.)

[ljmf00]

From @system and @trusted code, a safe interface can be called incorrectly. And then it can exhibit UB. As you say, passing unsafe values is how it happens.

Yes, that's it.

Note that the spec doesn't say that a safe interface must be called only with safe values. Passing unsafe values is allowed. It's just the programmer's responsibility then to ensure no UB, as with all unsafe operations.

When calling a safe interface with unsafe values there is a possibility of UB. Unless you know the exact instructions of it, you can't be ensured of no UB, just trusting the documentation, especially when that interface is external.

---

Anyway, my major point on this is being against making void initializers unsafe for types without indirections. I think https://issues.dlang.org/show_bug.cgi?id=17566 can be fixed with runtime stack checking on the compiler, unless there are more edge cases that stack checking can't cover. AFAIK, this is also environment-specific.

I'm still a bit sceptical about how the code provided in the issue is 100% following D rules, but I can't prove it, given your counterarguments about the mmap documentation.

[aG0aep6G]

When calling a safe interface with unsafe values there is a possibility of UB. Unless you know the exact instructions of it, you can't be ensured of no UB, just trusting the documentation, especially when that interface is external.

I don't think I agree 100% with that, but let's not derail this PR discussion further.

Anyway, my major point on this is being against making void initializers unsafe for types without indirections. I think https://issues.dlang.org/show_bug.cgi?id=17566 can be fixed with runtime stack checking on the compiler, unless there are more edge cases that stack checking can't cover. AFAIK, this is also environment-specific.

I agree that stack probing is a solution for issue 17566.

I'm still a bit sceptical about how the code provided in the issue is 100% following D rules, but I can't prove it, given your counterarguments about the mmap documentation.

Even if there is something wrong with the mmap call, I'm confident that the core of issue 17566 is true: void initialization can jump over the guard page and that's a problem for @safe. Also note that there is no mmap call at all in the 32-bit test case. Just simple malloc calls.

[ljmf00]

Even if there is something wrong with the mmap call, I'm confident that the core of issue 17566 is true: void initialization can jump over the guard page and that's a problem for @safe. Also note that there is no mmap call at all in the 32-bit test case. Just simple malloc calls.

This surely needs a solution. Maybe stack checking can be enforced when void initializers are being used in @safe code.