Skip to content

Conversation

@MatzeB
Copy link
Contributor

@MatzeB MatzeB commented Jul 8, 2025

Calls to the posix write function can return -1 and set errno to EINTR or perform partial writes when interrupted by signals. In those cases applications are supposed to just try again. See for example the documentation in glibc: https://sourceware.org/glibc/manual/latest/html_node/I_002fO-Primitives.html#index-write

This fixes the usage in ErrorHandling.cpp to retry as needed.

@MatzeB
Copy link
Contributor Author

MatzeB commented Jul 8, 2025

I don't know how to easily create a test for out-of-memory situations or force a unix signal to happen at the same time as the write so I couldn't add any test here. I hope the change is obvious to be acceptable anyway.

@MatzeB MatzeB marked this pull request as ready for review July 8, 2025 20:37
@llvmbot
Copy link
Member

llvmbot commented Jul 8, 2025

@llvm/pr-subscribers-llvm-support

Author: Matthias Braun (MatzeB)

Changes

Calls to the posix write function can return -1 and set errno to EINTR or perform partial writes when interrupted by signals or similar. See for example the documentation in glibc: https://sourceware.org/glibc/manual/latest/html_node/I_002fO-Primitives.html#index-write

This fixes the usage in ErrorHandling.cpp to retry as needed.


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

1 Files Affected:

  • (modified) llvm/lib/Support/ErrorHandling.cpp (+16-5)
diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp
index cc16f2037ea58..097041f1ae282 100644
--- a/llvm/lib/Support/ErrorHandling.cpp
+++ b/llvm/lib/Support/ErrorHandling.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Config/llvm-config.h" // for LLVM_ENABLE_THREADS
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/Errno.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
@@ -62,6 +63,17 @@ static std::mutex ErrorHandlerMutex;
 static std::mutex BadAllocErrorHandlerMutex;
 #endif
 
+static bool write_retry(int fd, const char *buf, size_t count) {
+  while (count > 0) {
+    ssize_t written = sys::RetryAfterSignal(-1, ::write, fd, buf, count);
+    if (written <= 0)
+      return false;
+    buf += written;
+    count -= written;
+  }
+  return true;
+}
+
 void llvm::install_fatal_error_handler(fatal_error_handler_t handler,
                                        void *user_data) {
 #if LLVM_ENABLE_THREADS == 1
@@ -111,8 +123,7 @@ void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) {
     raw_svector_ostream OS(Buffer);
     OS << "LLVM ERROR: " << Reason << "\n";
     StringRef MessageStr = OS.str();
-    ssize_t written = ::write(2, MessageStr.data(), MessageStr.size());
-    (void)written; // If something went wrong, we deliberately just give up.
+    write_retry(2, MessageStr.data(), MessageStr.size());
   }
 
   // If we reached here, we are failing ungracefully. Run the interrupt handlers
@@ -190,9 +201,9 @@ void llvm::report_bad_alloc_error(const char *Reason, bool GenCrashDiag) {
   // an OOM to stderr and abort.
   const char *OOMMessage = "LLVM ERROR: out of memory\n";
   const char *Newline = "\n";
-  (void)!::write(2, OOMMessage, strlen(OOMMessage));
-  (void)!::write(2, Reason, strlen(Reason));
-  (void)!::write(2, Newline, strlen(Newline));
+  write_retry(2, OOMMessage, strlen(OOMMessage));
+  write_retry(2, Reason, strlen(Reason));
+  write_retry(2, Newline, strlen(Newline));
   abort();
 #endif
 }

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see a case where the handler prints a partial message?

@MatzeB
Copy link
Contributor Author

MatzeB commented Jul 9, 2025

I did not see any actual cases where things failed. (I was debugging build problems where no error message was given, but by now it looks like it's not LLVMs fault but the surrounding build system loosing the messages).

Anyway it seemed like the right thing to add this fix to LLVM anyway. If nothing else it will stop the next person wondering whether maybe an out-of-memory error message was lost to EINTR...

@MatzeB MatzeB merged commit bdc0119 into llvm:main Jul 9, 2025
13 checks passed
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