[libc] Extend the baremetal I/O vendor ABI

[petrhosek]

This refines and extends the external ABI for I/O, later changes will update the baremetal implementations of I/O functions to use these.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Petr Hosek (petrhosek)

Changes

This refines and extends the external ABI for I/O, later changes will update the baremetal implementations of I/O functions to use these.

---

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

2 Files Affected:

• (modified) libc/src/__support/OSUtil/baremetal/io.cpp (+37-6)
• (modified) libc/src/__support/OSUtil/baremetal/io.h (+1)

diff --git a/libc/src/__support/OSUtil/baremetal/io.cpp b/libc/src/__support/OSUtil/baremetal/io.cpp
index c97bd5ae65b13..bd5946f267328 100644
--- a/libc/src/__support/OSUtil/baremetal/io.cpp
+++ b/libc/src/__support/OSUtil/baremetal/io.cpp
@@ -12,20 +12,51 @@
 
 namespace LIBC_NAMESPACE {
 
-// This is intended to be provided by the vendor.
+// These are intended to be provided by the vendor.
+//
+// The signature of these types and functions intentionally match `fopencookie`
+// which allows the following:
+//
+// ```
+// struct __llvm_libc_stdio_cookie __llvm_libc_stdin_cookie;
+// cookie_io_functions_t stdin_func = { .read = __llvm_libc_stdio_read };
+// FILE *stdin = fopencookie(&__llvm_libc_stdin_cookie, "r", stdin_func);
+// ...
+// struct __llvm_libc_stdio_cookie __llvm_libc_stdout_cookie;
+// cookie_io_functions_t stdout_func = { .write = __llvm_libc_stdio_write };
+// FILE *stdout = fopencookie(&__llvm_libc_stdout_cookie, "w", stdout_func);
+// ...
+// struct __llvm_libc_stdio_cookie __llvm_libc_stderr_cookie;
+// cookie_io_functions_t stdout_func = { .write = __llvm_libc_stdio_write };
+// FILE *stderr = fopencookie(&__llvm_libc_stderr_cookie, "w", stderr_func);
+// ```
+//
+// At the same time, implementation of functions like `printf` and `scanf` can
+// use `__llvm_libc_stdio_read` and `__llvm_libc_stdio_write` directly to avoid
+// the extra indirection.
 
-extern struct __llvm_libc_stdin __llvm_libc_stdin;
-extern "C" ssize_t __llvm_libc_stdin_read(void *cookie, char *buf, size_t size);
+struct __llvm_libc_stdio_cookie;
 
-extern "C" void __llvm_libc_log_write(const char *msg, size_t len);
+extern struct __llvm_libc_stdio_cookie __llvm_libc_stdin_cookie;
+extern struct __llvm_libc_stdio_cookie __llvm_libc_stdout_cookie;
+extern struct __llvm_libc_stdio_cookie __llvm_libc_stderr_cookie;
+
+extern "C" ssize_t __llvm_libc_stdio_read(void *cookie, char *buf, size_t size);
+extern "C" ssize_t __llvm_libc_stdio_write(void *cookie, const char *buf, size_t size);
 
 ssize_t read_from_stdin(char *buf, size_t size) {
-  return __llvm_libc_stdin_read(reinterpret_cast<void *>(&__llvm_libc_stdin),
+  return __llvm_libc_stdio_read(static_cast<void *>(&__llvm_libc_stdin_cookie),
                                 buf, size);
 }
 
+void write_to_stdout(cpp::string_view msg) {
+  __llvm_libc_stdio_write(static_cast<void *>(&__llvm_libc_stdout_cookie),
+                                 msg.data(), msg.size());
+}
+
 void write_to_stderr(cpp::string_view msg) {
-  __llvm_libc_log_write(msg.data(), msg.size());
+  __llvm_libc_stdio_write(static_cast<void *>(&__llvm_libc_stderr_cookie),
+                                 msg.data(), msg.size());
 }
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/__support/OSUtil/baremetal/io.h b/libc/src/__support/OSUtil/baremetal/io.h
index b9ae0bde502be..95e8277860a82 100644
--- a/libc/src/__support/OSUtil/baremetal/io.h
+++ b/libc/src/__support/OSUtil/baremetal/io.h
@@ -17,6 +17,7 @@ namespace LIBC_NAMESPACE {
 
 ssize_t read_from_stdin(char *buf, size_t size);
 void write_to_stderr(cpp::string_view msg);
+void write_to_stdout(cpp::string_view msg);
 
 } // namespace LIBC_NAMESPACE
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[frobtech]

This is a good incremental step and we can continue the cleanup and formalization with later changes.

[frobtech]

The cast is superfluous but does no harm.

[frobtech]

I think the comments here need to be a bit more clear that these three symbols must be provided, even if they don't point at anything, and give examples of how to meet the ABI.

All these declarations belong in some header file. Eventually such a header file should be installed somewhere so embedders can use it for the ABI contract they're meeting. But that can be a later cleanup, a comment here is enough for now.

[frobtech]

I think we should change these to propagate return values, and leave it to the embedder to decide if they want to return apparent success in error (or bit-bucket) cases.
But that can be a follow-on cleanup after this change lands.

[petrhosek]

I agree, but I'd like to do that in a follow-up change since I'll also need to update the callers.

[michaelrj-google]

LGTM, I agree with the comments. For return types I think we should generally match what fread and fwrite do: return the number of characters read/written.

[petrhosek]

LGTM, I agree with the comments. For return types I think we should generally match what fread and fwrite do: return the number of characters read/written.

I plan to change the signatures and update all callers in a follow-up change.

[frobtech]

LGTM, I agree with the comments. For return types I think we should generally match what fread and fwrite do: return the number of characters read/written.

The precise protocol here is already set by the fopencookie callbacks' APIs, which have the same return value semantics as their POSIX counterparts.