[libc-commits] [PATCH] D155762: [libc] Remove global constructors on File type

[Joseph Huber via Phabricator via libc-commits]

jhuber6 created this revision.
jhuber6 added reviewers: gchatelet, JonChesterfield, sivachandra, lntue, michaelrj.
Herald added projects: libc-project, All.
Herald added a subscriber: libc-commits.
jhuber6 requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155762

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


Index: libc/src/__support/File/file.h
===================================================================
--- libc/src/__support/File/file.h
+++ libc/src/__support/File/file.h
@@ -151,15 +151,6 @@
                    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 @@
         }
       }
     }
+
+    // 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155762.542234.patch
Type: text/x-patch
Size: 1232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230719/5212fb1e/attachment.bin>