Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[llvm][Support] Put back filename into FileToRemoveList #124065

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Jan 23, 2025

Prevents avoidable memory leaks.

Looks like exchange added in aa1333a didn't
take "continue" into account.

==llc==2150782==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x5f1b0f9ac14a in strdup llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:593:3
    #1 0x5f1b1768428d in FileToRemoveList llvm-project/llvm/lib/Support/Unix/Signals.inc:105:55

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Jan 23, 2025

@llvm/pr-subscribers-llvm-support

Author: Vitaly Buka (vitalybuka)

Changes

Prevents avoidable memory leaks.

Looks like exchange added in aa1333a didn't
take "continue" into account.


Full diff: https://github.com/llvm/llvm-project/pull/124065.diff

1 Files Affected:

  • (modified) llvm/lib/Support/Unix/Signals.inc (+19-15)
diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc
index 330b5d26fa50be..9a12663228a368 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -149,6 +149,24 @@ public:
     }
   }
 
+  static void removeFile(char *path) {
+    // Get the status so we can determine if it's a file or directory. If we
+    // can't stat the file, ignore it.
+    struct stat buf;
+    if (stat(path, &buf) != 0)
+      return;
+
+    // If this is not a regular file, ignore it. We want to prevent removal
+    // of special files like /dev/null, even if the compiler is being run
+    // with the super-user permissions.
+    if (!S_ISREG(buf.st_mode))
+      return;
+
+    // Otherwise, remove the file. We ignore any errors here as there is
+    // nothing else we can do.
+    unlink(path);
+  }
+
   // Signal-safe.
   static void removeAllFiles(std::atomic<FileToRemoveList *> &Head) {
     // If cleanup were to occur while we're removing files we'd have a bad time.
@@ -162,21 +180,7 @@ public:
       // If erasing was occuring while we're trying to remove files we'd look
       // at free'd data. Take away the path and put it back when done.
       if (char *path = currentFile->Filename.exchange(nullptr)) {
-        // Get the status so we can determine if it's a file or directory. If we
-        // can't stat the file, ignore it.
-        struct stat buf;
-        if (stat(path, &buf) != 0)
-          continue;
-
-        // If this is not a regular file, ignore it. We want to prevent removal
-        // of special files like /dev/null, even if the compiler is being run
-        // with the super-user permissions.
-        if (!S_ISREG(buf.st_mode))
-          continue;
-
-        // Otherwise, remove the file. We ignore any errors here as there is
-        // nothing else we can do.
-        unlink(path);
+        removeFile(path);
 
         // We're done removing the file, erasing can safely proceed.
         currentFile->Filename.exchange(path);

@vitalybuka vitalybuka merged commit 1311b36 into main Jan 23, 2025
10 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/llvmsupport-put-back-filename-into-filetoremovelist branch January 23, 2025 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants