ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
32.09k stars 2.34k forks source link

std.posix.open runs into unreachable when used to open a tmp file #20378

Open CPestka opened 2 weeks ago

CPestka commented 2 weeks ago

Zig Version

0.13.0

Steps to Reproduce and Observed Behavior

Passing .TMPFILE = true in the flags for posix.open() causes an unreachable to be tripped.

Code Snippet to reproduce:

const std = @import("std");
const posix = std.posix;

pub fn openAndCloseFile(flags: std.posix.O) !void {
    const tmp_file_dir = ".";
    const fd_tmp = try std.posix.open(
        tmp_file_dir,
        flags,
        0o660,
    );
    std.posix.close(fd_tmp);
}

pub fn main() !void {
    const file_name = "./regular_file.txt";
    const fd_regular = try std.posix.open(
        file_name,
        .{
            .ACCMODE = .RDWR,
            .CREAT = true,
            .TRUNC = true,
        },
        0o660,
    );
    try std.posix.unlink(file_name);
    defer std.posix.close(fd_regular);

    try openAndCloseFile(.{ .TMPFILE = true, .ACCMODE = .RDWR});
}

Results in the output

zig build run
thread 56006 panic: reached unreachable code
/home/cpestka/software/zig/zig-linux-x86_64-0.13.0/lib/std/posix.zig:1588:23: 0x10385c6 in openZ (tmpf)
            .INVAL => unreachable,
                      ^
/home/cpestka/software/zig/zig-linux-x86_64-0.13.0/lib/std/posix.zig:1565:17: 0x10350c2 in open (tmpf)
    return openZ(&file_path_c, flags, perm);
                ^
/home/cpestka/code/issues/tmpf/src/main.zig:23:38: 0x1034f76 in openAndCloseTmpFile (tmpf)
    const fd_tmp = try std.posix.open(
                                     ^
/home/cpestka/code/issues/tmpf/src/main.zig:18:28: 0x103530e in main (tmpf)
    try openAndCloseTmpFile(.{ .TMPFILE = true, .ACCMODE = .RDWR});
                           ^
/home/cpestka/software/zig/zig-linux-x86_64-0.13.0/lib/std/start.zig:524:37: 0x1034e85 in posixCallMainAndExit (tmpf)
            const result = root.main() catch |err| {
                                    ^
/home/cpestka/software/zig/zig-linux-x86_64-0.13.0/lib/std/start.zig:266:5: 0x10349a1 in _start (tmpf)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x0 in ??? (???)
run
└─ run tmpf failure
error: the following command terminated unexpectedly:
/home/cpestka/code/issues/tmpf/zig-out/bin/tmpf
Build Summary: 5/7 steps succeeded; 1 failed (disable with --summary none)
run transitive failure
└─ run tmpf failure
error: the following build command failed with exit code 1:

Expected Behavior

I would expect, that in case there is actually incorrect usage of the open call, an error to be returned rather than a crash to occur. However I do not think this is incorrect usage. O_TMPFILE only requires the path to be passed to be a directory and either RDWR or WRONLY to be passed. Additionally the equivalent c++ works, as i would have expected.

#include <cstdio>
#include <iostream>
#include <fcntl.h>

int main() {
    int fd = open(".", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
    if (fd > 0) {
        std::cout << ":) fd: " << fd << std::endl;
        return EXIT_SUCCESS;
    } else {
        perror(":/");
        return EXIT_FAILURE;
    }
}

Used fs is ext4, kernel version is 6.5.0-41-generic.

CPestka commented 1 week ago

Think I found the issue.

From the kernel, in open.c line 1255:

/* Now handle the creative implementation of O_TMPFILE. */
        if (flags & __O_TMPFILE) {
        /*
         * In order to ensure programs get explicit errors when trying
         * to use O_TMPFILE on old kernels we enforce that O_DIRECTORY
         * is raised alongside __O_TMPFILE.
         */
        if (!(flags & O_DIRECTORY))
            return -EINVAL;
        if (!(acc_mode & MAY_WRITE))
            return -EINVAL;
            }

Adding .DIRECTORY = true to the flags indeed "solves" the problem, as that makes the call succeed.

I'm not sure what the "correct" behavior of zig should be in this case. The documentation of open(3) does not say that O_DIRECTORY should be supplied when O_TMPFILE is supplied and the c++ version apparently silently adds the flag, as it works without it. The options I guess would be to either also silently add that flag if the TMPFILE flag is set or to return an error if it is set, but missing the DIRECTORY flag.

CPestka commented 1 week ago

Btw. I think that there might be two more cases that also would result in .INVAL being returned and thus crash.

  1. .TMPFILE=true and .ACCMODE= .RDONLY
  2. .DIRECT and missing fs support