phobos icon indicating copy to clipboard operation
phobos copied to clipboard

`stdin`, `stdout`, and `stderr` are not `@safe`

Open vpanteleev-sym opened this issue 6 months ago • 12 comments

import std.stdio;

@safe void main()
{
	stderr.writeln("Hello");
}

Fails with:

test.d(4): Error: `@safe` function `D main` cannot call `@system` function `std.stdio.makeGlobal!"core.stdc.stdio.stderr".makeGlobal`
    stderr.writeln("Hello");
    ^
/path/to/dmd.linux.nixified/dmd2/linux/bin64/../../src/phobos/std/stdio.d(#):        and using `__gshared` instead of `shared` makes it fail to infer `@safe`
    __gshared File.Impl impl;
                        ^
/path/to/dmd.linux.nixified/dmd2/linux/bin64/../../src/phobos/std/stdio.d(#):        `std.stdio.makeGlobal!"core.stdc.stdio.stderr".makeGlobal` is declared here
@property ref File makeGlobal(StdFileHandle _iob)()
                   ^

vpanteleev-sym avatar Jun 05 '25 19:06 vpanteleev-sym

Can we do something like this?

diff --git a/std/stdio.d b/std/stdio.d
index 82a13929f..f32b5af6a 100644
--- a/std/stdio.d
+++ b/std/stdio.d
@@ -5238,8 +5238,7 @@ enum StdFileHandle: string
     stderr = "core.stdc.stdio.stderr",
 }
 
-// Undocumented but public because the std* handles are aliasing it.
-@property ref File makeGlobal(StdFileHandle _iob)()
+private ref File makeGlobal(StdFileHandle _iob)()
 {
     __gshared File.Impl impl;
     __gshared File result;
@@ -5271,6 +5270,26 @@ enum StdFileHandle: string
     return result;
 }
 
+// Undocumented but public because the std* handles are aliasing it.
+template makeSafeGlobal(StdFileHandle _iob)
+{
+    // @safe getter, @system setter
+    File makeSafeGlobal() @trusted
+    {
+        return makeGlobal!_iob();
+    }
+
+    ref File makeSafeGlobal(File value) @system
+    {
+        return makeGlobal!_iob() = value;
+    }
+
+    ref File makeSafeGlobal(ref File value) @system
+    {
+        return makeGlobal!_iob() = value;
+    }
+}
+
 /** The standard input stream.
 
     Returns:
@@ -5286,7 +5305,7 @@ enum StdFileHandle: string
         and will cause all other threads calling `read` to wait until
         the lock is released.
 */
-alias stdin = makeGlobal!(StdFileHandle.stdin);
+alias stdin = makeSafeGlobal!(StdFileHandle.stdin);
 
 ///
 @safe unittest
@@ -5324,7 +5343,7 @@ alias stdin = makeGlobal!(StdFileHandle.stdin);
         and will cause all other threads calling `write` to wait until
         the lock is released.
 */
-alias stdout = makeGlobal!(StdFileHandle.stdout);
+alias stdout = makeSafeGlobal!(StdFileHandle.stdout);
 
 ///
 @safe unittest
@@ -5387,7 +5406,7 @@ alias stdout = makeGlobal!(StdFileHandle.stdout);
         and will cause all other threads calling `write` to wait until
         the lock is released.
 */
-alias stderr = makeGlobal!(StdFileHandle.stderr);
+alias stderr = makeSafeGlobal!(StdFileHandle.stderr);
 
 ///
 @safe unittest

Reading without a lock is probably not memory safe, but would require the "cooperation" of @system code to actually result in bad memory...

vpanteleev-sym avatar Jun 05 '25 19:06 vpanteleev-sym

I've thought about this a bit, for standard IO handles, these won't ever go away.

However, other handles that people may pass in might.

Those aliases would have to go away, in favor of dedicated functions, then makeSafeGlobal can be private.

rikkimax avatar Jun 06 '25 12:06 rikkimax

I've thought about this a bit, for standard IO handles, these won't ever go away.

Generally speaking, not being able to write to stderr from @safe code is not acceptable. Something else must give, whether that's adding a new @safe public symbol such as a @safe getStderr() or writelnErr, or a breaking change (though this change could be in the form of a new major Phobos version).

One simple solution could be to make stderr etc. thread-local. Yes, a breaking change, but I doubt there are many D applications which sorely rely on overriding the std.stdio File objects (specifically at that level, and not descriptors nor C FILE objects nor Windows HANDLEs), and doing so for all threads. Projects which we predict to be affected (maybe e.g. unit-threaded) could prepare by using a non-shared static constructor.

vpanteleev-sym avatar Jun 06 '25 12:06 vpanteleev-sym

By the way, I checked what std.logger does, as it provides a @safe interface for writing to standard error, and... there are 31 occurrences of @trusted in just std.logger.core.

vpanteleev-sym avatar Jun 06 '25 12:06 vpanteleev-sym

That isn't my concern, it's all about the lifetime of the handle itself.

We can't detect this scenario where it dies, and we still have a wrapper for it.

EDIT: due to reuse of handles by the kernel this is a real risk of bad things happening.

rikkimax avatar Jun 06 '25 12:06 rikkimax

I don't understand how that's related to the issue at hand. First, that's not a memory safety issue. Second, we're talking about the standard three file descriptors, which we can generally assume are always valid.

vpanteleev-sym avatar Jun 06 '25 12:06 vpanteleev-sym

Yes, those three are hard-coded id's, they are perfectly safe to wrap.

The problem is that the function can take in others and allow you to wrap it in @safe code.

rikkimax avatar Jun 06 '25 12:06 rikkimax

the function

Sorry, what function? makeGlobal is an undocumented internal.

vpanteleev-sym avatar Jun 06 '25 12:06 vpanteleev-sym

makeSafeGlobal

rikkimax avatar Jun 06 '25 12:06 rikkimax

From the patch above? Also undocumented internal (with makeGlobal now properly private).

vpanteleev-sym avatar Jun 06 '25 12:06 vpanteleev-sym

@rikkimax I think this safety issue can be solved by adding a template constraint or static assert to makeSafeGlobal which ensures that StdFileHandle _iob is one of StdFileHandle.stdin, StdFileHandle.stdout, or StdFileHandle.stderr.

pbackus avatar Jun 06 '25 12:06 pbackus

@rikkimax I think this safety issue can be solved by adding a template constraint or static assert to makeSafeGlobal which ensures that StdFileHandle _iob is one of StdFileHandle.stdin, StdFileHandle.stdout, or StdFileHandle.stderr.

Works for me.

rikkimax avatar Jun 06 '25 12:06 rikkimax