[libc] Implement perror

[michaelrj-google]

The perror function writes an error message directly to stderr. This
patch adds an implementation, tests, and header generation details.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

The perror function writes an error message directly to stderr. This
patch adds an implementation, tests, and header generation details.

---

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

8 Files Affected:

• (modified) libc/config/linux/x86_64/entrypoints.txt (+1)
• (modified) libc/include/stdio.yaml (+6)
• (modified) libc/src/stdio/CMakeLists.txt (+1)
• (modified) libc/src/stdio/generic/CMakeLists.txt (+15)
• (added) libc/src/stdio/generic/perror.cpp (+87)
• (added) libc/src/stdio/perror.h (+20)
• (modified) libc/test/src/stdio/CMakeLists.txt (+12)
• (added) libc/test/src/stdio/perror_test.cpp (+32)

diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 959bdbf08dbea..ff84069384e86 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -1113,6 +1113,7 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.stdio.getc_unlocked
     libc.src.stdio.getchar
     libc.src.stdio.getchar_unlocked
+    libc.src.stdio.perror
     libc.src.stdio.putc
     libc.src.stdio.putchar
     libc.src.stdio.puts
diff --git a/libc/include/stdio.yaml b/libc/include/stdio.yaml
index 2619984cca264..63acbb0460c11 100644
--- a/libc/include/stdio.yaml
+++ b/libc/include/stdio.yaml
@@ -247,6 +247,12 @@ functions:
       - POSIX
     return_type: int
     arguments: []
+  - name: perror
+    standards:
+      - stdc
+    return_type: void
+    arguments:
+      - type: const char *
   - name: printf
     standards:
       - stdc
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 63f6ed8a11f1d..b0a6ef1e291b5 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -221,6 +221,7 @@ add_stdio_entrypoint_object(fopen)
 add_stdio_entrypoint_object(fclose)
 add_stdio_entrypoint_object(fread_unlocked)
 add_stdio_entrypoint_object(fread)
+add_stdio_entrypoint_object(perror)
 add_stdio_entrypoint_object(puts)
 add_stdio_entrypoint_object(fputs)
 add_stdio_entrypoint_object(fwrite_unlocked)
diff --git a/libc/src/stdio/generic/CMakeLists.txt b/libc/src/stdio/generic/CMakeLists.txt
index e1f4ed5c19497..6361822b61999 100644
--- a/libc/src/stdio/generic/CMakeLists.txt
+++ b/libc/src/stdio/generic/CMakeLists.txt
@@ -206,6 +206,21 @@ add_generic_entrypoint_object(
     libc.src.__support.File.platform_file
 )
 
+add_generic_entrypoint_object(
+  perror
+  SRCS
+    perror.cpp
+  HDRS
+    ../perror.h
+  DEPENDS
+    libc.src.errno.errno
+    libc.src.__support.StringUtil.error_to_string
+    libc.src.__support.CPP.string_view
+    libc.src.__support.File.file
+    libc.src.__support.File.platform_file
+    libc.src.__support.File.platform_stderr
+)
+
 add_generic_entrypoint_object(
   fputs
   SRCS
diff --git a/libc/src/stdio/generic/perror.cpp b/libc/src/stdio/generic/perror.cpp
new file mode 100644
index 0000000000000..ce2424810f2c5
--- /dev/null
+++ b/libc/src/stdio/generic/perror.cpp
@@ -0,0 +1,87 @@
+//===-- Implementation of perror ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdio/perror.h"
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/File/file.h"
+#include "src/__support/StringUtil/error_to_string.h"
+#include "src/__support/macros/config.h"
+#include "src/errno/libc_errno.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+namespace {
+
+// TODO: this is copied from `puts`, it should be moved to a shared utility.
+//  Simple helper to unlock the file once destroyed.
+struct ScopedLock {
+  ScopedLock(LIBC_NAMESPACE::File *stream) : stream(stream) { stream->lock(); }
+  ~ScopedLock() { stream->unlock(); }
+
+private:
+  LIBC_NAMESPACE::File *stream;
+};
+
+int write_out(cpp::string_view str_view, File *f) {
+  if (str_view.size() > 0) {
+    auto result = f->write_unlocked(str_view.data(), str_view.size());
+    if (result.has_error())
+      return result.error;
+  }
+  return 0;
+}
+
+} // namespace
+
+// TODO: this seems like there should be some sort of queue system to
+// deduplicate this code.
+LLVM_LIBC_FUNCTION(void, perror, (const char *str)) {
+  const char empty_str[1] = {'\0'};
+  if (str == nullptr)
+    str = empty_str;
+  cpp::string_view str_view(str);
+
+  auto err_str = get_error_string(libc_errno);
+
+  // We need to lock the stream to ensure the newline is always appended.
+  ScopedLock lock(LIBC_NAMESPACE::stderr);
+  int write_err;
+
+  // FORMAT:
+  // if str != nullptr and doesn't start with a null byte:
+  //   "[str]: [strerror(errno)]\n"
+  // else
+  //   "[strerror(errno)]\n"
+  if (str_view.size() > 0) {
+    write_err = write_out(str_view, LIBC_NAMESPACE::stderr);
+    if (write_err != 0) {
+      libc_errno = write_err;
+      return;
+    }
+
+    write_err = write_out(": ", LIBC_NAMESPACE::stderr);
+    if (write_err != 0) {
+      libc_errno = write_err;
+      return;
+    }
+  }
+
+  write_err = write_out(err_str, LIBC_NAMESPACE::stderr);
+  if (write_err != 0) {
+    libc_errno = write_err;
+    return;
+  }
+
+  write_err = write_out("\n", LIBC_NAMESPACE::stderr);
+  if (write_err != 0) {
+    libc_errno = write_err;
+    return;
+  }
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/perror.h b/libc/src/stdio/perror.h
new file mode 100644
index 0000000000000..bf8d0af1df5d7
--- /dev/null
+++ b/libc/src/stdio/perror.h
@@ -0,0 +1,20 @@
+//===-- Implementation header of perror -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDIO_PERROR_H
+#define LLVM_LIBC_SRC_STDIO_PERROR_H
+
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+void perror(const char *s);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDIO_PERROR_H
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 01904a30504ed..ce2171f19597b 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -357,6 +357,18 @@ add_libc_test(
     libc.src.stdio.puts
 )
 
+add_libc_test(
+  perror_test
+  HERMETIC_TEST_ONLY # writes to libc's stderr
+  SUITE
+    libc_stdio_unittests
+  SRCS
+    perror_test.cpp
+  DEPENDS
+    libc.src.stdio.perror
+    libc.src.errno.errno
+)
+
 add_libc_test(
   fputs_test
   HERMETIC_TEST_ONLY # writes to libc's stdout and stderr
diff --git a/libc/test/src/stdio/perror_test.cpp b/libc/test/src/stdio/perror_test.cpp
new file mode 100644
index 0000000000000..4ac0ccc2f2f14
--- /dev/null
+++ b/libc/test/src/stdio/perror_test.cpp
@@ -0,0 +1,32 @@
+//===-- Unittests for perror ---------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdio/perror.h"
+
+#include "src/errno/libc_errno.h"
+#include "test/UnitTest/Test.h"
+
+// The standard says perror prints directly to stderr and returns nothing. This
+// makes it rather difficult to test automatically.
+
+// TODO: figure out redirecting stderr so this test can check correctness.
+TEST(LlvmLibcPerrorTest, PrintOut) {
+  LIBC_NAMESPACE::libc_errno = 0;
+  constexpr char simple[] = "A simple string";
+  LIBC_NAMESPACE::perror(simple);
+
+  // stick to stdc errno values, specifically 0, EDOM, ERANGE, and EILSEQ.
+  LIBC_NAMESPACE::libc_errno = EDOM;
+  LIBC_NAMESPACE::perror("Print this and an error");
+
+  LIBC_NAMESPACE::libc_errno = EILSEQ;
+  LIBC_NAMESPACE::perror("\0 shouldn't print this.");
+
+  LIBC_NAMESPACE::libc_errno = ERANGE;
+  LIBC_NAMESPACE::perror(nullptr);
+}

[jhuber6]

Wouldn't it be better to get a file lock on stderr using the API functions? Mostly asking this because this won't be common with the GPU now.

[michaelrj-google]

Is this better?

[jhuber6]

Well, the GPU doesn't use the file interface so it won't work. Would be easiest to just call flockfile but we don't want that kind of latency. Would probably just be easiest to write this special for the GPU (it's literally just puts for stderr.)

[michaelrj-google]

It doesn't look like there's an implementation of flockfile on gpu

[michaelrj-google]

Comments addressed

[michaelrj-google]

updated to use the new errno path. Planning to merge once presubmits finish

[lntue]

not to be addressed in this PR, but probably combining the strings into a single write to stderr is better, especially when stderr is unbuffered.

[michaelrj-google]

That'd be difficult since the strings are of unknown length. We could probably collapse it down to 2 writes since we can check the length of the strerror strings but the user could input anything.