-
-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@system variables #179
@system variables #179
Conversation
DIPs/DIP1NNN-DK.md
Outdated
| } | ||
|
|
||
| Cstring cStringLiteral(string s)() @trusted { | ||
| return Cstring(s.ptr); // string literals are guaranteed zero-terminated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be @safe
Cstring cStringLiteral(string s)() @safe {
return Cstring(&s[0]);
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't, since it's modifying a @system variable. Also your version gives a range violation error on empty strings. You do remind me to mention default constructors though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can cStringLiteral really be @trusted, though? You're assuming that all compile-time strings (not just literals) are zero-terminated. As far as I can tell, that's not guaranteed.
import std.stdio: writeln;
import core.stdc.string: strlen;
void main()
{
enum string e = ['f', 'o', 'o'];
writeln(strlen(e.ptr)); /* Prints "6" for me. */
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I think this fixes it:
Cstring cStringLiteral(string s)() @trusted {
static immutable string tmp = s;
return Cstring(tmp.ptr); // static strings are guaranteed zero-terminated
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand where we differ.
A struct's generated constructor should be considered @trusted as far as this DIP is concerned (right now it would be @safe).
You are taking a known state, setting it to another known state that was intended by the author.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So invariants as in function contracts, are one form of validation. But there is also invariant blocks which can also perform this function. Both of which get compiled out in -release mode.
The reason you would give it @system is because once initialized you don't want it to become an unknown state. Which would be inherently unsafe. This is what concerns me, becoming unsafe, rather than being initialized as unsafe.
It would be the responsibility of the developer to write a constructor should they need more guarantees. I would highly doubt that you will find struct's that have member fields annotated with @system in the wild today. Meaning if any has been annotated, that it is on purpose.
Regarding Cstring, your example has already solved this and I do not believe you can solve this in the general case. The compiler knows that the pointer does point to something. What it doesn't understand is the requirement for it being null terminated. If you wish to force this (without writing an @system constructor), you would need to disable the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invariant blocks and contracts are completely unrelated, with 'invariants' I mean a compile time guarantee that certain type + bit-pattern combinations never reach @safe code, e.g. a bool that is not 0 or 1. While the compiler knows that bool(3) is invalid, @system fields allow you to make your own types with forbidden bit-patterns, but the compiler can't know what the restrictions are. Therefor it has to be conservative and assume the result of a default constructor may not be valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what concerns me, becoming unsafe, rather than being initialized as unsafe.
Can you give an example of a type that can't be initialized to an invalid value with a default constructor but can be modified to one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your C string example is a good one.
It has a separate constructor in the form of a free-function that performs the validation.
If you need more guarantees than this provides, you end up having to @disable the constructor otherwise unsafe code can freely call it and set it into an invalid state. Without realizing what they are doing.
No matter which way you go, if you want these guarantees as the author of the code, you will have to either disable it or write your own constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The C string is not a good example since it can be initialized with an invalid value, which is why the free function needs to be @trusted.
It is a case in favor of a @system default constructor by default instead of a @trusted default constructor.
| Since memory safety-related attributes now have an effect on variables and fields, it becomes useful to inspect them. | ||
| Therefor the restriction on the getFunctionAttributes trait gets lifted. | ||
|
|
||
| The name "function attributes" is a bit unfortunate in this case, but this DIP does not aim to fix that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getDeclarationAttributes
Add a second name, deprecate old one. This DIP can and probably should fix this (traits don't need a DIP to be added).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the list of traits has gotten bigger without much resistance, I am not a big fan of that direction. It's messy right now, and Atila mentioned how he wanted a more uniform way for introspection, so I try to avoid touching the pile of traits right now. Thanks for your suggestion though, if I get some more opinions on this I'll try to accommodate.
DIPs/DIP1NNN-DK.md
Outdated
|
|
||
| > My best idea so far (might not be originally mine) is to let the @system attribute apply to variables. | ||
|
|
||
| Any prior discussion about `@system` variables has not been found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some previous mentions of the idea:
- https://forum.dlang.org/post/cxupcgybvqwvkuvcokoz@forum.dlang.org (Olivier FAURE)
- https://forum.dlang.org/post/pqleml$2kpg$1@digitalmars.com (Timon Gehr)
- fix Issue 18554 - tupleof ignoring private shouldn't be accepted in @… dmd#8035 (comment) (myself)
- https://forum.dlang.org/post/pqn0dc$2cq7$1@digitalmars.com (myself)
That last one contains an alternative solution to the problem that isn't mentioned in this DIP: an Unsafe type wrapper that obscures the stored value, e.g. with void[T.sizeof], so that all accesses become @system. It's probably not good for precise garbage collection, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the links, I'll add them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The void[T.sizeof] trick does not work right? You may not retrieve pointers with it, but you can still write to them in @safe code.
@safe:
void[int.sizeof] unsafeVar; // hides a pointer
union U {
int asInt = 3;
ubyte[int.sizeof] asBytes;
}
void main() {
unsafeVar = U.init.asBytes;
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, from dlang/dmd#8035 (comment) it seems like Walter wants to make going around private not @safe after all. I might need to spend more time considering that alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
void[T.sizeof]trick does not work right? You may not retrieve pointers with it, but you can still write to them in@safecode.
Huh. I completely missed that. Looks like the void[T.sizeof] idea is a no-go.
Writing through a void[] that way also seems to be allowed, and that's obviously unsafe. Filed as https://issues.dlang.org/show_bug.cgi?id=20345. But void[]'s problem is that it might alias pointers. void[n] can't do that in @safe code (I think). So it's not exactly the same issue.
DIPs/DIP1NNN-DK.md
Outdated
| While constructing arbitrary pointers inside `@safe` code is not allowed, there is currently no defence around pointers initialized to an unsafe value outside any function: | ||
| ```D | ||
| @safe: | ||
| int* x = cast(int*) 0x7FFE_E800_0000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't compile anymore with DMD 2.088. dlang/dmd#10056
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a surprise! It still compiles when marking the declarations individually with @safe, and I assumed putting @safe: above it would be identical.
The fix seems to only be for unsafe casts, and the following still compiles:
auto getPtr() @system {return cast(int*) 0x7FFE_E800_0000;}
@safe:
int* x = getPtr();
void main() @safe {
*x = 3; // memory corruption
}So I'll update the example.
598a4bc
to
7b10974
Compare
| It does not work when annotating the declaration as `@safe`, only when the variables is under a `@safe:` section. | ||
| It is also easily circumvented: | ||
|
|
||
| ```D | ||
| auto getPtr() @system {return cast(int*) 0x7FFE_E800_0000;} | ||
|
|
||
| @safe: | ||
| int* x = getPtr(); | ||
|
|
||
| void main() @safe { | ||
| int y = *x; // = 3; // memory corruption | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, I've filed an issue for these oversights: 20347
7b10974
to
5f2d812
Compare
DIPs/DIP1NNN-DK.md
Outdated
| T pop() @trusted {if (empty) assert(0); return stack[--_top];} | ||
| void push(T x) @trusted {if (full) assert(0); stack[_top++] = x;} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is somewhat besides the point of the DIP, but these methods can't be @trusted for other reasons. pop and push will call an unsafe copy constructor or opAssign:
struct S
{
import core.stdc.stdio: printf;
void opAssign(ref S src) @system { printf("unsafe opAssign\n"); }
this(ref S src) @system { printf("unsafe copy ctor\n"); }
}
void main() @safe
{
auto stack = getStack!S();
stack.push(S()); /* calls unsafe opAssign */
auto s = stack.pop(); /* calls unsafe copy ctor */
}Also, T might contain a pointer and it might have a destructor that writes through that pointer. That can't be allowed to run on uninitialized memory.
struct S
{
int* p;
~this() @safe { *p = 42; }
}
void f() @safe { auto stack = getStack!S(); }
/* writes 42 to undefined memory location */Automatically adding an @system destructor might help here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While indeed besides the point of the DIP, I do find it important that the examples are correct, so it's good you point these things out. I simply added the restriction isPOD to keep the example simple and limited the scope of @trusted for push.
Automatically adding an
@systemdestructor might help here.
I don't know what you mean with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what you mean with that.
Never mind. I had missed a couple things there.
The idea was to infer Stack's implicit destructor as @system so that Stack can't be used in @safe code. Then unsafe destruction would be prevented.
I thought that you could then write an explicit @trusted destructor that avoids destruction of uninitialized data. But even with an explicit destructor, it seems that the destructors of fields are still called automatically. So unsafe destruction can't be prevented that way.
And anyway, it probably makes more sense to say that getStack violates safety if the returned object can't be destroyed safely.
With the isPOD restriction, all this doesn't apply anymore, of course.
DIPs/DIP1NNN-DK.md
Outdated
| ```D | ||
| @system private ubyte[4096] heap; | ||
| @system private int heapIndex; | ||
|
|
||
| /// allocate an array of T | ||
| /// never frees the memory | ||
| T[] customAlloc(T)(int length) { | ||
| // round up heapIndex to multiple of T.alignof | ||
| heapIndex = (heapIndex + T.alignof-1) & ~(T.alignof-1); | ||
| auto result = cast(T[]) heap[heapIndex..heapIndex + T.sizeof*length]; | ||
| heapIndex += heapIndex + T.sizeof*length; | ||
| return result; | ||
| } | ||
|
|
||
| import std; | ||
|
|
||
| void main() @safe { | ||
| string[] strArr = customAlloc!string(3); | ||
| heapIndex = 0; // mess up allocator integrity! not allowed after this DIP. | ||
| int[] intArr = customAlloc!int(3); | ||
| intArr[2] = 0x8035FDF0; // overwrites string array | ||
| writeln(strArr[0]); // memory corruption: constructed pointer | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text above says that "each example wrongly uses @trusted", but there's no @trusted in this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to test this one, thanks.
d3c8c14
to
a59fef4
Compare
a59fef4
to
5c55a8f
Compare
a34fc6b
to
c002eaf
Compare
DIPs/DIP1NNN-DK.md
Outdated
|
|
||
| ## Abstract | ||
| When the `@system` attribute is attached to variables or fields, it means they cannot be written to in `@safe` functions. | ||
| If the variable has an unsafe type, it also cannot be read in `@safe` code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is an "unsafe type"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read down a bit, but its there. Basically anything that is a pointer.
https://github.com/dlang/DIPs/pull/179/files#diff-62f04e927443e2ad2cc392bb9f890dc1R223
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case I think there should be at least a link to what the DIP means. It's akin to introducing an acronym without defining it then defining it hundreds of lines later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
| ### Unsafe types | ||
| In the proposed changes I use the term "unsafe types". | ||
| An unsafe type is a type which has underlying bit-patterns that risk causing memory corruption in `@safe` code. | ||
| The prime example of an unsafe type is a pointer, but any type that contains a pointer is also unsafe: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would these reference types be unsafe if they were allocated on the GC heap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I evidently didn't communicate things clearly, so I'm not sure how to answer this. Here are some key insights:
- The concepts in this DIP have nothing to do with memory allocation or lifetimes. In a world where freeing memory did not exist, the definitions still apply.
- A pointer is an unsafe type because if I were allowed to arbitrarily change the bits, I could construct a garbage pointer that can cause memory corruption in
@safecode. - By contrast, if I could arbitrarily change the bits of an
intorfloat, it could get strange values and mess up the program, but because of bounds checks it is not sufficient for memory corruption in@safecode. There is no equivalent to a dereference operator on ints or floats, all their operators don't touch memory. - With void initialization, union and array casting I can arbitrarily change bits of a variable regardless of type. That's why they are not allowed on pointers in
@safecode, but they are on ints and floats. - Using GC pointers is
@safeprecisely because pointers are an unsafe type: the language ensures pointer integrity. If pointers were a safe type, I could change it to something that is not a GC pointer and dereference it to corrupt memory.
I think the name "unsafe types" is confusing. The long name would be "types where the integrity of the underlying bits is necessary for memory safety".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that all of what you just wrote belongs in the DIP itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the DIP needs to be self-explanatory, and include all of this rationale so that it's understood in the proper context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added those points to the DIP shortly after Atila's comment, if you still think something is missing please tell.
DIPs/DIP1NNN-DK.md
Outdated
| ## Background | ||
|
|
||
| A distinction can be made between syntactic types and semantic types. | ||
| Syntactic types are the things the compiler understands: you can't assign a string to an int, you can't pass an integer array to a function that takes a float parameter etc. | ||
| While this helps catching bugs that assembly programmers encounter, it has its limitations with regards to data abstraction and unsafe features. | ||
| Often it is useful to make types that have additional constraints on the primitive types they consist of, or types that from the outside are safe and type-sound, but internally use unsafe operations that circumvent the type system. | ||
| This can only work if the language in which such types are defined has the needed encapsulation capabilities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the background section or what it's trying to accomplish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mentioned article gives some high level rationale for what this DIP is trying to accomplish (i.e. making the type system catch more errors than simple type mismatches) and the section is a small summary of it.
DIPs/DIP1NNN-DK.md
Outdated
|
|
||
| **(0) _Writing_ to variables or fields marked `@system` is not allowed in `@safe` code** | ||
|
|
||
| This does not apply to local variables however, where `@system` remains ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the difference in local variables. Users are unlikely to mark these as @system unless there's good reason for it, and it's generally not a good idea to introduce exceptions unless necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the checks for local variables is more implementation work with no additional benefit, since there is no way local variables are accessed from the outside unless the function allows references to them to escape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @atilaneves that it is not a good idea to have exceptions like this. If no such checking is wanted for local variables, I'd suggest to simply disallow an @system / @trusted / @safe attribute on a local variable. That would also prevent a possible later introduction of @system-checking on local variables from being a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disallowing safety attributes would be inconsistent with the fact that you can still mark a local variable pure nothrow @nogc @property, and disallowing useless attributes in general is out of scope for this DIP. I don't think breaking code is a concern since there is currently no reason to add function attributes to local variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a possible use case for local system variables?
void foo()() // note the template for inference
{
@system int x; // marked system for semantic reasons
void bar() @safe { ++x; }
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give a more concrete example than foo-bar? I do think there are uses for local system variables, but if I were to add them to this DIP, I think I need a compelling argument that they are worth the extra implementation cost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a common pattern in template programming in D where you have a template function with inferred attributes, and regardless of how they are inferred, you can mark specific parts @trusted or @safe to guarantee those bits are properly checked. Making it so a @safe function can access the outer stack frame that is @system is questionable, especially if you cannot mark data as untouchable.
The stack frame should be treated like a struct that's on the stack. I don't see why a variable on the stack should be any less useful than a struct member. Otherwise, you will have something like this:
void foo()() {
struct Frame {
@system int x;
}
Frame frame;
...
}Which is kind of annoying.
Also, it should be mentioned that the foo-bar example really isn't the problem. The real problem is this:
void foo()()
{
@system int x; // thinking: no safe function can touch this
void bar() @safe {
... // doesn't use x, because it's "not allowed"
}
}And then later someone edits bar to use x and the original developers instructions were silently ignored by the compiler.
I think I need a compelling argument that they are worth the extra implementation cost
I don't know what the implementation cost is. I'm very ignorant when it comes to that. So maybe it's difficult to do. From a user perspective, I would want the feature to be available. For consistency's sake as well. I don't know how to judge the "cost" of implementing it.
It would be akin to having @safe attributes ignored for inner functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what the implementation cost is. I'm very ignorant when it comes to that. So maybe it's difficult to do.
I've started toying with an implementation, and it turns out local variables work automatically, and adding an exception for them is actually more code.
I'll change the DIP.
DIPs/DIP1NNN-DK.md
Outdated
| - passing it as an argument to a function parameter marked `ref` | ||
| - returning it by `ref` | ||
|
|
||
| A struct or union with `@system` fields can not be constructed using the default constructor in `@safe` code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What default constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct Handle {
@system int x; // must be positive or something
}
void main() @safe {
auto h = Handle(-1); // default constructor for Handle used to create invalid object
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is can be solved by using an invariant which will give you full guarantees unlike your constructor approach.
The invariant is a contract saying that the asserts must hold true. The invariant is checked when a class or struct constructor completes, and at the start of the class or struct destructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is can be solved by using an invariant
An invariant won't run on auto h = Handle(-1);, because there's no actual constructor call.
And like assert, invariant is not a safety feature. It may or may not be executed at run time. You cannot rely on it for safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, as mentioned in the rationale section, some things are hard to check at runtime. E.g. if I have a pointer to memory allocated with malloc and a corresponding length in a custom slice type, checking that the length is not going outside of the allocated block requires allocator internals and takes time. Making the length @system gives compile-time guarantees instead of requiring runtime mechanisms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is can be solved by using an invariant
An
invariantwon't run onauto h = Handle(-1);, because there's no actual constructor call.
Thats a bug
https://issues.dlang.org/show_bug.cgi?id=519
dlang/dmd#10022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is can be solved by using an invariant which will give you full guarantees unlike your constructor approach.
So are you proposing that using runtime invariant blocks is a better approach than @system variables?
If so, can you give an example of how an invariant gives guarantees unlike @trusted constructors like you say, or rewrite any of the 5 examples in the examples section with invariants instead of @system variables/fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is can be solved by using an invariant which will give you full guarantees unlike your constructor approach.
So are you proposing that using runtime invariant blocks is a better approach than
@systemvariables?
No, there are use cases for it. Otherwise I wouldn't be replying to this DIP ;)
If so, can you give an example of how an invariant gives guarantees unlike
@trustedconstructors like you say, or rewrite any of the 5 examples in the examples section with invariants instead of@systemvariables/fields?
I cannot. Your examples are very specific to the DIP in showing how it can help. That makes them good examples.
My concern is that you are using this solution for problems which would be better suited to using a constructor, private or invariants. The example you wrote above with Handle is a good example of this. Truth be told it should probably be private and accessible only through a method. Since it shouldn't be changed once created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the problem now. For simplicity I use an example of a very vague 'handle' with a simple invariant ("must be positive or something") which makes the link with memory corruption far fetched.
What I'm thinking of concretely is something returned by a C library function. For example, in OpenGL when you generate resources such as shaders or buffer objects, you get an integer identifying it. However, you can't double free / use after free a shader or buffer object, since the API is defensive and raises an error when an invalid handle is given, so it won't actually corrupt memory. The Windows API is also very defensive with invalid arguments, but there might be C libraries that assume valid handles or else corrupt memory. (If anyone knows one please tell).
I think I'll add a concrete example of unsafe types that don't contain a pointer, something like a virtual machine that avoids bounds checks for performance:
enum Opcode : ubyte {
decrement, increment, print,
}
struct VmInstruction {
@system Opcode opcode; // notice how this need not be private, just a valid enum member
}
void decrementImpl();
void incrementImpl();
void printImpl();
immutable void function()[3] jumpTable = [
&decrementImpl, &incrementImpl, &printImpl;
]
void execute(VmInstruction instruction) @trusted {
jumpTable.ptr[instruction.opcode](); // bounds check elided for performance
}Hopefully this makes it clear how even a ubyte (or enum with base type ubyte) can have memory-corrupting bit patterns when it's part of a struct.
| } | ||
| ``` | ||
|
|
||
| **(3) Variables and fields without annotation are `@safe` unless their initial value is not `@safe`** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the rationale for prohibiting reading such values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent memory corruption. If you could read systemPtr, you can dereference it and access an arbitrary memory address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the pointer is not the same thing as dereferencing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is that @system is not part of the type like const or shared, so you can easily strip off @system by simply reading the value, after which you can dereference the garbage pointer:
@system int* garbage = cast(int*) 0xDEADBEEF;
void main() @safe {
*garbage = 0; // error! this will be caught
int* pointer = garbage; // this is what rule (2) disallows
*pointer = 0; // this is allowed, so the above line should be prevented| int y1; // @system | ||
| ``` | ||
|
|
||
| **(4) `__traits(getFunctionAttributes)` may be called on variables and fields** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would cause __traits(getFunctionAttributes) to be a misnomer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See: https://github.com/dlang/DIPs/pull/179/files/6104d1fcc6fe2eac328cd63d42e15444b817efd5#r341413460
I'm open for suggestions.
|
|
||
| The name "function attributes" is a bit unfortunate in this case, but this DIP does not aim to fix that. | ||
|
|
||
| **(5) `bool` becomes an unsafe type** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better instead to make it @system to somehow construct a bool that is neither 0 or 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's exactly what I'm proposing. Ways to construct such a bool are with a union, void initialization, array/pointer casting, and by making bool an unsafe type those actions become forbidden in @safe code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand now. Would you mind editing it so that it's clear why bool has to be an unsafe type? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the first two lines already do that? This line:
By making
boolan unsafe type, it can be both fast and@safesince all tricks to make invalid booleans are now disabled in@safecode.
Seems synonymous to me with:
Wouldn't it be better instead to make it @System to somehow construct a bool that is neither 0 or 1?
Assuming that:
invalid == not 0 or 1
disabled in @safe == only @system
|
I like this DIP. Two points: Second point is that there is an excellent concept that @quickfur touches upon in his post here: https://forum.dlang.org/post/mailman.773.1579207677.31109.digitalmars-d@puremagic.com in which he shows that in fact, the prototypical "semantic type" for safe is the dynamic array. If you had to reimplement a dynamic array with |
DIPs/DIP1NNN-DK.md
Outdated
|
|
||
| A struct or union with `@system` fields can not be constructed using [static initializtion](https://dlang.org/spec/struct.html#static_struct_init) or the automatically generated constructor in `@safe` code. | ||
| A `@trusted` constructor can be defined to allow construction in `@safe` code. | ||
| Note that the `.init` value of a type is always usable in `@safe` code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule interacts in a very, very nasty with D's constructor flow analysis when it comes to generic data structures that have @system fields (for example, SumType). You can't mark the entire constructor as @trusted, because doing so would incorrectly allow calls to @system copy constructors/postblits to occur in @safe code. But you also can't use a lambda to mark only the initialization of the @system field as @trusted, because the compiler's flow analysis doesn't understand lambdas and will complain that you haven't initialized all of the fields. The end result will be that generic data structures like SumType will in many cases become completely impossible to construct in @safe code.
I'm not sure what the best solution to this is. Adding @trusted blocks that are transparent to constructor flow analysis would work, as would simply removing costructor flow analysis altogether. In any case, given that tagged unions are explicitly mentioned as a use-case for this feature, I think it's worth mentioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's nasty indeed.
The end result will be that generic data structures like
SumTypewill in many cases become completely impossible to construct in@safecode.
I think you can use introspection with __traits(compiles, ...) to check whether the copy constructor / postblit of the generic type are @safe and use static if or mixin to create a @trusted / @system constructor accordingly. Or did I miss something?
enum isSafeType(T) = __traits(compiles, () @safe { ... });
struct S(T) {
static if (isSafeType!T) {
@trusted this(T b) { ... }
} else {
@system this(T b) { ... }
}
}In any case, it's clumsy and having lambdas or @trusted blocks compatible with constructor flow analysis would be desirable.
In any case, given that tagged unions are explicitly mentioned as a use-case for this feature, I think it's worth mentioning.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My apologies; I forgot to mention the other reason I can't mark SumType's constructor as @trusted, which is that @trusted disables inference of the return and scope parameter attributes when compiling with -preview=dip1000.
Introspection is not a viable solution here. I would rather live with the current issues with @safe than write mountains of difficult, brittle code to do a job the compiler should be able to do for me.
|
@schveiguy Thanks for your feedback.
I wouldn't consider holes in
The "custom slice type" example touches on the idea. I don't have much time now, but I'll try to go back to the rationale section later and see if I can elaborate on the points you mentioned. The slice type is indeed a good example. If I recall correctly, associative arrays also had some built-in restricted access to its key/value pointers in |
I didn't mean to say that they weren't real problems. What I meant is that the overarching problem is that
In general, I find a quality property of a language is that you can use the language to implement the primitives. In D, we still cannot implement dictionary structs with the same semantics as the builtin AA. This limits anyone who wants to use AA API to one implementation. I find similar problems with arrays. D provides a LOT of mechanisms to duplicate the syntax of arrays, but there are some things that are still out of reach. If we can make it so custom types can be drop-ins for arrays, we can explore other implementations, which may have benefits in certain situations (your example is one). I don't know the name of this property (self implementation?), but the closer D can get to it, I think we are better off. |
DIPs/DIP1NNN-DK.md
Outdated
| ```D | ||
| immutable ubyte ub = 3; | ||
| bool getValidBool() pure @system {return true;} | ||
| bool getInvalidBool() pure @system {return *(cast(bool*) ub);} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably meant bool getInvalidBool() pure @system {return *(cast(bool*) &ub);}
(Note the &)
No description provided.