llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Linisha (linisha15)
<details>
<summary>Changes</summary>
## Summary
`fileno()` currently assumes every `FILE*` is backed by a `LinuxFile` and unconditionally casts to read its file descriptor. For streams created by `fopencookie()` or `fmemopen()` (backed by `CookieFile`/`MemoryFile`, neither of which has a file descriptor member), this reads unrelated memory and returns a bogus "descriptor" instead of following POSIX, which requires `fileno()` to fail with `EBADF` when "the stream is not associated with a file."
## Fix
`File` has no RTTI/type tag by design (some `LinuxFile` instances, e.g. `stdin`/`stdout`/`stderr`, are `constexpr` globals). Instead of adding one, this adds `is_linux_file()`, which positively identifies a `LinuxFile` by checking that all four of its registered I/O callbacks (`read`/`write`/`seek`/`close`) are the known `linux_file_*` functions — every `LinuxFile` always registers exactly these, and no other `File` subclass does. `get_fileno()` now checks this first and returns `-1` with `errno = EBADF` for anything else. `CookieFile`/`MemoryFile` are untouched — no fake file descriptors added, per the issue's request.
## Testing
Added `FilenoFails` to `fmemopen_test.cpp` and `fopencookie_test.cpp`, asserting `fileno()` returns `-1`/`EBADF` for those stream types. Verified the new logic compiles cleanly against the real headers/toolchain and behaves correctly (a genuine `LinuxFile` still returns its real fd; a non-`LinuxFile` stand-in correctly gets `-1`/`EBADF`) via a standalone harness linked against the actual compiled objects — this environment couldn't complete a full `LLVM_LIBC_FULL_BUILD=ON` configure (hit memory limits during CMake's generate step). CI's `libc-fullbuild-tests` workflow should run the added tests directly on the PR.
## Related, out of scope here
`reopenfile_unlocked` (used by `freopen`) has the same unchecked cast to `LinuxFile`, likely the same bug reachable via `freopen(path, mode, fmemopen_stream)`. Left out of this PR to keep it scoped to the filed issue — happy to fold it in here or send it as a follow-up, whichever you'd prefer.
Fixes #<!-- -->223550.
---
Full diff: https://github.com/llvm/llvm-project/pull/227760.diff
6 Files Affected:
- (modified) libc/src/__support/File/file.h (+9)
- (modified) libc/src/__support/File/linux/file.cpp (+11)
- (modified) libc/src/__support/File/linux/file.h (+6)
- (modified) libc/test/src/stdio/CMakeLists.txt (+2)
- (modified) libc/test/src/stdio/fmemopen_test.cpp (+13)
- (modified) libc/test/src/stdio/fopencookie_test.cpp (+18)
``````````diff
diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h
index 84dde0560f467f..71a5813ee3d5c8 100644
--- a/libc/src/__support/File/file.h
+++ b/libc/src/__support/File/file.h
@@ -61,6 +61,15 @@ class File {
using SeekFunc = ErrorOr<off_t>(File *, off_t, int);
using CloseFunc = int(File *);
+ // Exposes the platform I/O callbacks so a platform backend can positively
+ // identify which concrete File subclass an object is (e.g. distinguishing
+ // a LinuxFile from a CookieFile/MemoryFile) without needing RTTI. See
+ // is_linux_file() in the Linux backend for the concrete use.
+ ReadFunc *get_read_func() const { return platform_read; }
+ WriteFunc *get_write_func() const { return platform_write; }
+ SeekFunc *get_seek_func() const { return platform_seek; }
+ CloseFunc *get_close_func() const { return platform_close; }
+
// This is a convenience RAII class to lock and unlock file objects.
class FileLock {
File *file;
diff --git a/libc/src/__support/File/linux/file.cpp b/libc/src/__support/File/linux/file.cpp
index cc6a1fcbd31688..b0a3828a5d55a0 100644
--- a/libc/src/__support/File/linux/file.cpp
+++ b/libc/src/__support/File/linux/file.cpp
@@ -280,7 +280,18 @@ int LinuxFile::reopen_unlocked(const char *path, const char *mode) {
return 0;
}
+bool is_linux_file(const File *f) {
+ return f->get_read_func() == &linux_file_read &&
+ f->get_write_func() == &linux_file_write &&
+ f->get_seek_func() == &linux_file_seek &&
+ f->get_close_func() == &linux_file_close;
+}
+
int get_fileno(File *f) {
+ if (!is_linux_file(f)) {
+ libc_errno = EBADF;
+ return -1;
+ }
auto *lf = reinterpret_cast<LinuxFile *>(f);
return lf->get_fd();
}
diff --git a/libc/src/__support/File/linux/file.h b/libc/src/__support/File/linux/file.h
index 539957b8b22c8c..d7aaa3a83dc202 100644
--- a/libc/src/__support/File/linux/file.h
+++ b/libc/src/__support/File/linux/file.h
@@ -18,6 +18,12 @@ FileIOResult linux_file_read(File *, void *, size_t);
ErrorOr<off_t> linux_file_seek(File *, off_t, int);
int linux_file_close(File *);
+// Returns true iff |f| is actually a LinuxFile, as opposed to some other
+// File subclass (e.g. CookieFile, MemoryFile) that doesn't have a real
+// file descriptor. Used because File has no RTTI/type tag: every LinuxFile
+// always registers these exact four callbacks, and no other subclass does.
+bool is_linux_file(const File *f);
+
class LinuxFile : public File {
int fd;
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 28b07cd984e9ea..2f00bdc8ad9dbc 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -161,6 +161,7 @@ add_libc_test(
libc.src.stdio.ferror
libc.src.stdio.fflush
libc.src.stdio.fgetc
+ libc.src.stdio.fileno
libc.src.stdio.fmemopen
libc.src.stdio.fread
libc.src.stdio.fseek
@@ -189,6 +190,7 @@ add_libc_test(
libc.src.stdio.feof
libc.src.stdio.ferror
libc.src.stdio.fflush
+ libc.src.stdio.fileno
libc.src.stdio.fopencookie
libc.src.stdio.fread
libc.src.stdio.fseek
diff --git a/libc/test/src/stdio/fmemopen_test.cpp b/libc/test/src/stdio/fmemopen_test.cpp
index 1736580914a453..344a1f2d6c7bc4 100644
--- a/libc/test/src/stdio/fmemopen_test.cpp
+++ b/libc/test/src/stdio/fmemopen_test.cpp
@@ -21,6 +21,7 @@
#include "src/stdio/ferror.h"
#include "src/stdio/fflush.h"
#include "src/stdio/fgetc.h"
+#include "src/stdio/fileno.h"
#include "src/stdio/fmemopen.h"
#include "src/stdio/fread.h"
#include "src/stdio/fseek.h"
@@ -246,6 +247,18 @@ TEST_F(LlvmLibcFMemOpenTest, InternalBuffer) {
}
}
+TEST_F(LlvmLibcFMemOpenTest, FilenoFails) {
+ char storage[8];
+ ::FILE *f = LIBC_NAMESPACE::fmemopen(storage, sizeof(storage), "r+");
+ ASSERT_TRUE(f != nullptr);
+ scope_exit close([&] { EXPECT_EQ(0, LIBC_NAMESPACE::fclose(f)); });
+
+ // A memory stream has no underlying file descriptor: POSIX requires
+ // fileno() to fail with EBADF rather than return a bogus value derived
+ // from unrelated internal state.
+ ASSERT_THAT(LIBC_NAMESPACE::fileno(f), Fails(EBADF));
+}
+
TEST_F(LlvmLibcFMemOpenTest, ZeroCapacity) {
char guard = 'X';
void *buffers[] = {&guard, nullptr};
diff --git a/libc/test/src/stdio/fopencookie_test.cpp b/libc/test/src/stdio/fopencookie_test.cpp
index 4c6dadd7ff70a4..676fa77f95015c 100644
--- a/libc/test/src/stdio/fopencookie_test.cpp
+++ b/libc/test/src/stdio/fopencookie_test.cpp
@@ -11,6 +11,7 @@
#include "src/stdio/feof.h"
#include "src/stdio/ferror.h"
#include "src/stdio/fflush.h"
+#include "src/stdio/fileno.h"
#include "src/stdio/fopencookie.h"
#include "src/stdio/fread.h"
#include "src/stdio/fseek.h"
@@ -247,3 +248,20 @@ TEST_F(LlvmLibcFOpenCookieTest, WriteUpdateCookieTest) {
ASSERT_EQ(LIBC_NAMESPACE::fclose(f), 0);
free(ss);
}
+
+TEST_F(LlvmLibcFOpenCookieTest, FilenoFails) {
+ auto *ss = reinterpret_cast<StringStream *>(malloc(sizeof(StringStream)));
+ ss->buf = nullptr;
+ ss->bufsize = ss->offset = ss->endpos = 0;
+
+ ::FILE *f = LIBC_NAMESPACE::fopencookie(ss, "r", STRING_STREAM_FUNCS);
+ ASSERT_TRUE(f != nullptr);
+
+ // A cookie stream has no underlying file descriptor: POSIX requires
+ // fileno() to fail with EBADF.
+ ASSERT_EQ(LIBC_NAMESPACE::fileno(f), -1);
+ ASSERT_ERRNO_EQ(EBADF);
+
+ ASSERT_EQ(0, LIBC_NAMESPACE::fclose(f));
+ free(ss);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/227760