Skip to content

[libc] Extend the baremetal I/O vendor ABI #98683

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 14, 2024
Merged

[libc] Extend the baremetal I/O vendor ABI #98683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57cf707
[libc] Extend the baremetal I/O vendor ABI
petrhosek Jul 12, 2024
efc4e87
Fix formatting
petrhosek Jul 12, 2024
04ced5c
Update comment
petrhosek Jul 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions libc/src/__support/OSUtil/baremetal/io.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,58 @@

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 { ... };
// ...
// 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 stderr_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.
//
// All three symbols `__llvm_libc_stdin_cookie`, `__llvm_libc_stdout_cookie`,
// and `__llvm_libc_stderr_cookie` must be provided, even if they don't point
// at anything.

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

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),
Copy link
Contributor

Choose a reason for hiding this comment

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

The cast is superfluous but does no harm.

buf, size);
}

void write_to_stdout(cpp::string_view msg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

__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
1 change: 1 addition & 0 deletions libc/src/__support/OSUtil/baremetal/io.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading