[libc-commits] [libc] cc92212 - [libc] Remove global constructors on File type

[Joseph Huber via libc-commits]

Author: Joseph Huber
Date: 2023-07-20T08:11:18-05:00
New Revision: cc92212d75dff07644e478c55a6f145c020f6f8e

URL: https://github.com/llvm/llvm-project/commit/cc92212d75dff07644e478c55a6f145c020f6f8e
DIFF: https://github.com/llvm/llvm-project/commit/cc92212d75dff07644e478c55a6f145c020f6f8e.diff

LOG: [libc] Remove global constructors on File type

The `File` interface currently has a destructor to delete the buffer if
it is owned by the file. This is problematic for the globally allocated
`stdout`, `stdin`, and `stderr` files. This causes the file interface to
have global constructors to initialize the destructors to use these.
However, these never use the destructors because they don't own the
buffer. This patch removes the destructor and calls in manually in the
close implementation. The platform close should never need to access the
buffer and it needs to be done before clearing the whole thing, so this
should work.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D155762

Added: 
    

Modified: 
    libc/src/__support/File/file.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h
index 97dc5ff66f1ca9..2c6aff3b3be4d8 100644
--- a/libc/src/__support/File/file.h
+++ b/libc/src/__support/File/file.h
@@ -151,15 +151,6 @@ class File {
                    static_cast<ModeFlags>(OpenMode::PLUS));
   }
 
-  // The GPU build should not emit a destructor because we do not support global
-  // destructors in all cases and it is unneccessary without buffering.
-#if !defined(LIBC_TARGET_ARCH_IS_GPU)
-  ~File() {
-    if (own_buf)
-      delete buf;
-  }
-#endif
-
 public:
   // We want this constructor to be constexpr so that global file objects
   // like stdout do not require invocation of the constructor which can
@@ -234,6 +225,13 @@ class File {
         }
       }
     }
+
+    // If we own the buffer, delete it before calling the platform 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;
+
     // Platform close is expected to cleanup the file data structure which
     // includes the file mutex. Hence, we call platform_close after releasing
     // the file lock. Another thread doing file operations while a thread is