Add `fchmodat` fallback on Linux when `flags` is nonzero.

[The-King-of-Toasters]

After updating the syscall list, I researched why fchmodat2 was introduced. The fchmodat syscall doesn't take a flags argument, meaning this wrapper has been subtly broken since its introduction. This pr changes fchmodat to:

• Use the libc implementation if linking with libc.
• Use fchmodat if flags is zero.
• Use fchmodat2 if available.
• Use a workaround via procfs if all else fails.

The procfs workaround is what glibc and musl do. This pr copies the glibc logic as it requires the least amount of syscalls.

I've also replaced the fchmodat tests to check for this case and ensured they passed on Linux and other systems (FreeBSD 13, NetBSD 10).

[The-King-of-Toasters]

Dang, I have no idea why this new test is flaky. This shouldn't even throw an error given that we're not chmoding a regular file:

    try os.fchmodat(tmp.dir.fd, "regfile", 0o600, os.AT.SYMLINK_NOFOLLOW);

[The-King-of-Toasters]

I think I understand what's happening. CI is failing when linking with glibc, and the version being used is 2.31. Looking at the fchmodat impl, it's easy to see what's wrong:

int
fchmodat (int fd, const char *file, mode_t mode, int flag)
{
  if (flag & ~AT_SYMLINK_NOFOLLOW)
    return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
#ifndef __NR_lchmod		/* Linux so far has no lchmod syscall.  */
  if (flag & AT_SYMLINK_NOFOLLOW)
    return INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOTSUP);
#endif

  return INLINE_SYSCALL (fchmodat, 3, fd, file, mode);
}

lchmod was never a syscall in its own right, so this branch is always taken. I guess the solution is to skip over using libc entirely, or check if the glibc version is known to be bad (2.32 was the first to include the workaround)

[The-King-of-Toasters]

I've rebased this PR atop of master due to the recent Atomic changes. I'd really like someone from the core team to give a simple yea/nay on the changes since it is a correctness fix.

[The-King-of-Toasters]

FYI: I authored a similar patch that was merged into Cosmopolitan a while ago. The difference was that atomic booleans aren't used, instead relying on ENOSYS being returned.

[The-King-of-Toasters]

I don't quite follow, do you mean to split the fallback code into an inline function, or to mark os.fchmodat inline? Since use_c is the conditional for the while loop, wouldn't this have the same effect?

[andrewrk]

I was going to link you to the relevant section of the language reference but I realized that the section was missing, so I created it: https://github.com/ziglang/zig/commit/aba8d4f62c5643333ca50a884c0fa1898a31dfed

Hopefully it should be clear after reading these new docs.

[The-King-of-Toasters]

Hopefully it should be clear after reading these new docs.

Not really? If fchmodat is marked inline, then use_c will be comptime true if flags == 0 and the rest of the code isn't generated. But wouldn't this needlessly bloat the binary size like you mentioned?

[andrewrk]

I am not suggesting to inline all the logic, only the use_c calculation.

[The-King-of-Toasters]

I think I understand now, is this acceptable?

BTW #18131 gives a similar improvement to a function used here, might be worth adding to the review pile ;).

[andrewrk]

No, there is too much logic in the inline function. Do you want me to show you?

[The-King-of-Toasters]

Yes please. Edit: Unless you mean something like:

inline fn skipFchmodatFallback(flags: u32) bool {
    return flags == 0;
}

Honestly I'm more confused. Is there a way for me to verify that either of these approaches?

[andrewrk]

It looks like you've unchecked the box "allow maintainers to push commits to this branch" so I moved the PR here: #18547

Have a look and let me know if it is satisfactory