Story Time: I Accidentally Kept a Pointer to the Stack in Zig

I ran into a classic stack-use-after-return bug in Zig the other day while working on cmdtest. I thought I could just create a stdout and stderr reader and pass a reference to my InteractiveProcess struct.

Here is the code I wrote:

Zig
pub fn spawn(options: SpawnOptions) SpawnError!InteractiveProcess {
    var child = Child.init(options.argv, options.allocator);
    child.cwd = options.cwd;
    child.env_map = options.env_map;
    child.stdin_behavior = .Pipe;
    child.stdout_behavior = .Pipe;
    child.stderr_behavior = .Pipe;

    try child.spawn();

    const stdout_buf = try options.allocator.alloc(u8, options.max_output_bytes);
    const stderr_buf = try options.allocator.alloc(u8, options.max_output_bytes);

    const stdout_file = child.stdout orelse return error.MissingStdout;
    const stderr_file = child.stderr orelse return error.MissingStderr;

    var stdout_reader = stdout_file.reader(stdout_buf);
    var stderr_reader = stderr_file.reader(stderr_buf);

    return InteractiveProcess{
        .child = child,
        .pid = child.id,
        .allocator = options.allocator,
        .stdout_reader = &stdout_reader.interface,
        .stderr_reader = &stderr_reader.interface,
    };
}

The problem is that stdout_reader and stderr_reader are created on the stack.

Zig
var stdout_reader = stdout_file.reader(stdout_buf);
var stderr_reader = stderr_file.reader(stderr_buf);

Then I am taking a pointer to them and storing it in the InteractiveProcess struct that I am returning.

Zig
return InteractiveProcess{
    // ...
    .stdout_reader = &stdout_reader.interface,
    .stderr_reader = &stderr_reader.interface,
};

When the spawn function returns, the stack frame is gone, and so are my readers. This means InteractiveProcess is holding dangling pointers.

The right way to fix this is to pass the readers by value, not by reference.

Zig
return InteractiveProcess{
    .child = child,
    .pid = child.id,
    .allocator = options.allocator,
    .stdout_reader = stdout_reader,
    .stderr_reader = stderr_reader,
};

This way, InteractiveProcess owns the readers and the buffers they use. I can then free the buffers in the deinit function for InteractiveProcess.

But then I realized that the readers are valid as long as child.stdout and child.stderr are not null. So for my use case, the better solution is to just pass the buffers to InteractiveProcess and create the readers on demand when I need them.

Tags zig , debugging