[libc-commits] [libc] [llvm] [libc] Release stream buffers with delete[] and stop reallocating them (PR #237127)

[Mike Lothian via libc-commits]

https://github.com/FireBurn updated https://github.com/llvm/llvm-project/pull/237127

>From 693e03a648c8c051f12c73e3d65b4c8339dc4bc1 Mon Sep 17 00:00:00 2001
From: Mike Lothian <mike at fireburn.co.uk>
Date: Fri, 30 Oct 2026 07:36:38 +0000
Subject: [PATCH] [libc] Release stream buffers with delete[] and stop
 reallocating them

The buffers a stream owns are all allocated with new[], but File::close
and File::set_buffer released them with delete, and set_buffer resized
one with realloc. Allocate the replacement with new[] and release the
old buffer with delete[] in both places.

Assisted-by: Claude
---
 libc/src/__support/File/CMakeLists.txt        |  1 -
 libc/src/__support/File/file.cpp              | 28 ++++++++-----------
 libc/test/src/stdio/setvbuf_test.cpp          | 26 +++++++++++++++++
 .../llvm-project-overlay/libc/BUILD.bazel     |  1 -
 4 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/libc/src/__support/File/CMakeLists.txt b/libc/src/__support/File/CMakeLists.txt
index ca2670281a92d7..f3b76961e2cf45 100644
--- a/libc/src/__support/File/CMakeLists.txt
+++ b/libc/src/__support/File/CMakeLists.txt
@@ -20,7 +20,6 @@ add_object_library(
   DEPENDS
     .file_io_result
     libc.hdr.errno_macros
-    libc.hdr.func.realloc
     libc.hdr.stdint_proxy
     libc.hdr.stdio_macros
     libc.hdr.types.char32_t
diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp
index f8ff4a6e52f2ca..21badb27a2ea57 100644
--- a/libc/src/__support/File/file.cpp
+++ b/libc/src/__support/File/file.cpp
@@ -16,7 +16,6 @@
 #include "file_io_result.h"
 #include "file_mode.h"
 #include "hdr/errno_macros.h"
-#include "hdr/func/realloc.h"
 #include "hdr/stdint_proxy.h"
 #include "hdr/stdio_macros.h"
 #include "hdr/types/char32_t.h"
@@ -522,7 +521,7 @@ int File::close() {
   // implementation. The platform close should not need to access the buffer
   // and we need to clean it up before the entire structure is removed.
   if (own_buf)
-    delete buf;
+    delete[] buf;
 
   // Platform close is expected to cleanup the file data structure which
   // includes the file mutex. Hence, we call platform_close after releasing
@@ -549,24 +548,19 @@ int File::set_buffer(void *buffer, size_t size, int buffer_mode) {
   if (buffer == nullptr && size != 0 && buffer_mode != _IONBF) {
     // We exclude the case of buffer_mode == _IONBF in this branch
     // because we don't need to allocate buffer in such a case.
-    if (own_buf) {
-      // This is one of the places where a C allocation function is used
-      // as C++ does not have an equivalent of realloc.
-      buf = reinterpret_cast<uint8_t *>(realloc(buf, size));
-      if (buf == nullptr)
-        return ENOMEM;
-    } else {
-      AllocChecker ac;
-      buf = new (ac) uint8_t[size];
-      if (!ac)
-        return ENOMEM;
-      own_buf = true;
-    }
+    AllocChecker ac;
+    uint8_t *new_buf = new (ac) uint8_t[size];
+    if (!ac)
+      return ENOMEM;
+    // The buffer is allocated with new[], so it is released with delete[].
+    if (own_buf)
+      delete[] buf;
+    buf = new_buf;
+    own_buf = true;
     bufsize = size;
-    // TODO: Handle allocation failures.
   } else {
     if (own_buf)
-      delete buf;
+      delete[] buf;
     if (buffer_mode != _IONBF) {
       buf = static_cast<uint8_t *>(buffer);
       bufsize = size;
diff --git a/libc/test/src/stdio/setvbuf_test.cpp b/libc/test/src/stdio/setvbuf_test.cpp
index a0936ba79ef73d..e999db7822fab3 100644
--- a/libc/test/src/stdio/setvbuf_test.cpp
+++ b/libc/test/src/stdio/setvbuf_test.cpp
@@ -106,3 +106,29 @@ TEST(LlvmLibcSetbufTest, InvalidBufferMode) {
 
   ASSERT_EQ(0, LIBC_NAMESPACE::fclose(f));
 }
+
+TEST_F(LlvmLibcSetvbufTest, ReplaceTheBufferTheStreamAllocated) {
+  // The stream allocates its own buffer. It is resized, which allocates a new
+  // one, and then replaced by one of ours, which releases the allocated one.
+  constexpr char FILENAME[] = "testdata/setvbuf_replace.test";
+  constexpr char CONTENT[] = "abcdef";
+
+  ::FILE *f = LIBC_NAMESPACE::fopen(FILENAME, "w");
+  ASSERT_FALSE(f == nullptr);
+  ASSERT_EQ(LIBC_NAMESPACE::setvbuf(f, nullptr, _IOFBF, 4096), 0);
+  char buffer[BUFSIZ];
+  ASSERT_EQ(LIBC_NAMESPACE::setvbuf(f, buffer, _IOFBF, BUFSIZ), 0);
+  ASSERT_EQ(sizeof(CONTENT),
+            LIBC_NAMESPACE::fwrite(CONTENT, 1, sizeof(CONTENT), f));
+  ASSERT_EQ(0, LIBC_NAMESPACE::fclose(f));
+
+  // A stream that keeps the buffer it allocated releases it when it is closed.
+  f = LIBC_NAMESPACE::fopen(FILENAME, "r");
+  ASSERT_FALSE(f == nullptr);
+  ASSERT_EQ(LIBC_NAMESPACE::setvbuf(f, nullptr, _IOFBF, 4096), 0);
+  char data[sizeof(CONTENT)];
+  ASSERT_EQ(sizeof(CONTENT),
+            LIBC_NAMESPACE::fread(data, 1, sizeof(CONTENT), f));
+  ASSERT_STREQ(CONTENT, data);
+  ASSERT_EQ(0, LIBC_NAMESPACE::fclose(f));
+}
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 5f8fa7e58778af..171a804cf4eeca 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -2395,7 +2395,6 @@ libc_support_library(
         ":func_aligned_alloc",
         ":func_free",
         ":func_malloc",
-        ":func_realloc",
         ":hdr_errno_macros",
         ":hdr_stdint_proxy",
         ":hdr_stdio_macros",