Skip to content

[libc] Allow using sscanf() and vsscanf() on baremetal targets. #130527

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

Closed
wants to merge 2 commits into from
Closed

[libc] Allow using sscanf() and vsscanf() on baremetal targets. #130527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9b681a
[libc] Allow using sscanf() and vsscanf() on baremetal targets.
jdeguire Mar 9, 2025
ad6617d
fixup! [libc] Allow using sscanf() and vsscanf() on baremetal targets.
jdeguire Mar 9, 2025
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
3 changes: 3 additions & 0 deletions libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ function(_get_common_compile_options output_var flags)
list(APPEND compile_options "-fpie")

if(LLVM_LIBC_FULL_BUILD)
if(LIBC_TARGET_OS_IS_BAREMETAL)
list(APPEND compile_options "-DLIBC_TARGET_OS_IS_BAREMETAL")
endif()
Comment on lines +122 to +124
Copy link
Contributor

Choose a reason for hiding this comment

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

When possible we prefer to have compile options based on capabilities instead of targets. In this specific case we'd probably want something similar to LIBC_COPT_STDIO_USE_SYSTEM_FILE, which is an existing flag that changes printf/scanf behavior to use another libc's FILE* functions. It's also defined closer to where it's used, in the stdio CMake file, which I think would also be good here.

# Only add -ffreestanding flag in non-GPU full build mode.
if(NOT LIBC_TARGET_OS_IS_GPU)
list(APPEND compile_options "-ffreestanding")
Expand Down
2 changes: 1 addition & 1 deletion libc/src/stdio/baremetal/getchar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(int, getchar, ()) {
char buf[1];
auto result = read_from_stdin(buf, sizeof(buf));
if (result < 0)
if (result <= 0)
return EOF;
return buf[0];
}
Expand Down
18 changes: 11 additions & 7 deletions libc/src/stdio/scanf_core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ if(LIBC_TARGET_OS_IS_GPU)
libc.src.stdio.ungetc
libc.src.stdio.ferror
)
elseif(LIBC_TARGET_OS_IS_BAREMETAL)
list(APPEND file_deps
libc.src.stdio.getchar
)
elseif(LLVM_LIBC_FULL_BUILD)
list(APPEND file_deps
libc.src.__support.File.file
Expand Down Expand Up @@ -54,13 +58,6 @@ add_header_library(
libc.src.__support.CPP.string_view
)

if(NOT(TARGET libc.src.__support.File.file) AND LLVM_LIBC_FULL_BUILD AND
(NOT LIBC_TARGET_OS_IS_GPU))
# Not all platforms have a file implementation. If file is unvailable, and a
# full build is requested, then we must skip all file based scanf sections.
return()
endif()

add_object_library(
scanf_main
SRCS
Expand Down Expand Up @@ -117,6 +114,13 @@ add_object_library(
${use_system_file}
)

if(NOT(TARGET libc.src.__support.File.file) AND LLVM_LIBC_FULL_BUILD AND
(NOT LIBC_TARGET_OS_IS_GPU))
# Not all platforms have a file implementation. If file is unvailable, and a
# full build is requested, then we must skip all file based scanf sections.
return()
endif()

#TODO: condense the file-related code as possible.
add_header_library(
vfscanf_internal
Expand Down
43 changes: 42 additions & 1 deletion libc/src/stdio/scanf_core/reader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@

#include "hdr/types/FILE.h"

#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
#if !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE) && \
!defined(LIBC_TARGET_OS_IS_BAREMETAL)
#include "src/__support/File/file.h"
#endif

#if defined(LIBC_TARGET_ARCH_IS_GPU)
#include "src/stdio/getc.h"
#include "src/stdio/ungetc.h"
#elif defined(LIBC_TARGET_OS_IS_BAREMETAL)
#include "hdr/stdio_macros.h" // for EOF.
#include "src/stdio/getchar.h"
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

We try to avoid having libc functions depend on each other where possible. The GPU build is a special case (that I'd like to clean up at some point) but for baremetal I think just calling read_from_stdin should work fine instead of going through the public entrypoint. That way you also don't have to declare stdin in the middle of an unrelated file.

#endif

#include "src/__support/macros/attributes.h" // For LIBC_INLINE
Expand Down Expand Up @@ -47,6 +51,28 @@ LIBC_INLINE void ungetc(int c, void *f) {
LIBC_NAMESPACE::ungetc(c, reinterpret_cast<::FILE *>(f));
}

#elif defined(LIBC_TARGET_OS_IS_BAREMETAL)
// The baremetal build does not currently support file operations, but it does
// declare pointers to the stanard FILE streams. The user just needs to declare
// "FILE *stdin;" somwhere. That is not much to ask since a user already has to
// define cookie structures for stdio.
extern "C" FILE *stdin;

LIBC_INLINE int getc(void *f) {
if (f == stdin) {
int c = getchar();
if (EOF == c)
c = '\0';

return c;
}

return '\0';
}
Comment on lines +61 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this code would go in baremetal/scanf.cpp (which I realize doesn't exist yet but it probably should). This gives me an idea for how to solve this problem in a more general way, I'll see if I can get a basic proof of concept patch up.


// Baremetal does not currently provide an ungetc(), so the Reader will need to
// handle that for now.

#elif !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)

LIBC_INLINE int getc(void *f) {
Expand Down Expand Up @@ -89,6 +115,10 @@ class Reader {
ReadBuffer *rb;
void *input_stream = nullptr;
size_t cur_chars_read = 0;
#if defined(LIBC_TARGET_OS_IS_BAREMETAL)
// Baremetal does not provide an ungetc(), so track that ourselves for now.
int unget_char = EOF;
#endif
Comment on lines +118 to +121
Copy link
Contributor

Choose a reason for hiding this comment

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

the char from ungetc can be ignored since we never modify it, so you can skip most of the extra handling there.


public:
// TODO: Set buff_len with a proper constant
Expand All @@ -107,6 +137,13 @@ class Reader {
++(rb->buff_cur);
return output;
}
#if defined(LIBC_TARGET_OS_IS_BAREMETAL)
if (EOF != unget_char) {
char output = (char)unget_char;
unget_char = EOF;
return output;
}
#endif
// This should reset the buffer if applicable.
return static_cast<char>(reader_internal::getc(input_stream));
}
Expand All @@ -123,7 +160,11 @@ class Reader {
--(rb->buff_cur);
return;
}
#if defined(LIBC_TARGET_OS_IS_BAREMETAL)
unget_char = c;
#else
reader_internal::ungetc(static_cast<int>(c), input_stream);
#endif
}

LIBC_INLINE size_t chars_read() { return cur_chars_read; }
Expand Down
Loading