This is an archive of the discontinued LLVM Phabricator instance.

Differential D147919

[libc] Add implementation of getchar
ClosedPublic

Authored by xbjfk on Apr 10 2023, 1:35 AM.

Details

Reviewers
sivachandra
lntue
michaelrj
Commits
rG1c261e360f55: [libc] Add implementation of getchar
Summary

added getchar and getchar_unlocked which are just wrappers getc and getc_unlocked respectively.

Diff Detail

Event Timeline

xbjfk created this revision.Apr 10 2023, 1:35 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 10 2023, 1:35 AM
Herald added subscribers: libc-commits, luke, frasercrmck and 22 others. · View Herald Transcript
xbjfk requested review of this revision.Apr 10 2023, 1:35 AM
Herald added a subscriber: • pcwang-thead. · View Herald TranscriptApr 10 2023, 1:35 AM
Harbormaster completed remote builds in B224531: Diff 512095.Apr 10 2023, 6:51 AM
lntue added a reviewer: michaelrj.Apr 10 2023, 7:02 AM
michaelrj added a comment.Apr 10 2023, 10:16 AM

In our codebase we try to avoid calling entrypoints from other entrypoints. In this case that means not calling getc from getchar, instead mostly copying the code. This may seem silly, but it allows users to include any entrypoint on its own without having to worry about dependencies.

Other than that, this patch looks good, I'll approve it once that change is made.

xbjfk updated this revision to Diff 512393.Apr 11 2023, 3:54 AM

Dont call getc/getc_unlocked from getchar/getchar_unlocked

xbjfk added a comment.Apr 11 2023, 3:58 AM

after this is reviewed, can someone please commit this? (I dont have commit access yet)

libc/spec/posix.td
1083

Is this the correct way to define a function that takes no arguments?

Harbormaster completed remote builds in B224733: Diff 512393.Apr 11 2023, 4:02 AM
michaelrj accepted this revision.Apr 11 2023, 11:16 AM

LGTM with these changes. I can commit for you.

libc/spec/posix.td
1083

good catch, it should be ArgSpec<VoidType>

libc/spec/stdc.td
586

same here, it should be ArgSpec<VoidType>

This revision is now accepted and ready to land.Apr 11 2023, 11:16 AM
lntue added inline comments.Apr 11 2023, 3:36 PM
libc/config/linux/x86_64/entrypoints.txt
426

Can this also be enabled for ARM targets?

xbjfk updated this revision to Diff 512636.Apr 11 2023, 6:26 PM

Add to aarch64 entrypoints.txt, fix function spec
I didn't add to arm (32) entrypoints since the stdio.h isn't part of headers.txt yet

xbjfk marked an inline comment as done.Apr 11 2023, 6:26 PM
xbjfk marked an inline comment as done.
Harbormaster completed remote builds in B224913: Diff 512636.Apr 11 2023, 6:33 PM
lntue accepted this revision.Apr 11 2023, 8:28 PM
sivachandra accepted this revision.Apr 11 2023, 10:18 PM
sivachandra added inline comments.
libc/src/stdio/getchar.cpp
25

Nit: This can be if (result.value != 1). Same in getchar_unlocked.

xbjfk updated this revision to Diff 513526.Apr 14 2023, 3:52 AM

Remove redundant temporary variable

xbjfk marked 2 inline comments as done.Apr 14 2023, 3:54 AM
Harbormaster completed remote builds in B225563: Diff 513526.Apr 14 2023, 3:59 AM
michaelrj accepted this revision.Apr 14 2023, 3:13 PM

LGTM, I'm going to land it once I'm done with what I'm currently doing.

Closed by commit rG1c261e360f55: [libc] Add implementation of getchar (authored by michaelrj). · Explain WhyApr 14 2023, 3:40 PM
This revision was automatically updated to reflect the committed changes.
michaelrj added a commit: rG1c261e360f55: [libc] Add implementation of getchar.

Revision Contents

PathSize
libc/
config/
linux/
aarch64/
2 lines
riscv64/
2 lines
x86_64/
2 lines
docs/
2 lines
spec/
5 lines
5 lines
src/
stdio/
26 lines
18 lines
28 lines
18 lines
28 lines

Diff 513777

libc/config/linux/aarch64/entrypoints.txt

Show First 20 Lines • Show All 390 Lines • ▼ Show 20 Lines list(APPEND TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.fopencookie libc.src.stdio.fopencookie
libc.src.stdio.fread libc.src.stdio.fread
libc.src.stdio.fread_unlocked libc.src.stdio.fread_unlocked
libc.src.stdio.fseek libc.src.stdio.fseek
libc.src.stdio.funlockfile libc.src.stdio.funlockfile
libc.src.stdio.fwrite libc.src.stdio.fwrite
libc.src.stdio.fwrite_unlocked libc.src.stdio.fwrite_unlocked
libc.src.stdio.fprintf libc.src.stdio.fprintf
libc.src.stdio.getchar
libc.src.stdio.getchar_unlocked
libc.src.stdio.printf libc.src.stdio.printf
libc.src.stdio.putc libc.src.stdio.putc
libc.src.stdio.putchar libc.src.stdio.putchar
libc.src.stdio.puts libc.src.stdio.puts
libc.src.stdio.stderr libc.src.stdio.stderr
libc.src.stdio.stdin libc.src.stdio.stdin
libc.src.stdio.stdout libc.src.stdio.stdout
▲ Show 20 Lines • Show All 72 Lines • Show Last 20 Lines

libc/config/linux/riscv64/entrypoints.txt

Show First 20 Lines • Show All 403 Lines • ▼ Show 20 Lines list(APPEND TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.fseek libc.src.stdio.fseek
libc.src.stdio.ftell libc.src.stdio.ftell
libc.src.stdio.funlockfile libc.src.stdio.funlockfile
libc.src.stdio.fwrite libc.src.stdio.fwrite
libc.src.stdio.fwrite_unlocked libc.src.stdio.fwrite_unlocked
libc.src.stdio.fprintf libc.src.stdio.fprintf
libc.src.stdio.getc libc.src.stdio.getc
libc.src.stdio.getc_unlocked libc.src.stdio.getc_unlocked
libc.src.stdio.getchar
libc.src.stdio.getchar_unlocked
libc.src.stdio.printf libc.src.stdio.printf
libc.src.stdio.sscanf libc.src.stdio.sscanf
libc.src.stdio.scanf libc.src.stdio.scanf
libc.src.stdio.fscanf libc.src.stdio.fscanf
libc.src.stdio.putc libc.src.stdio.putc
libc.src.stdio.putchar libc.src.stdio.putchar
libc.src.stdio.puts libc.src.stdio.puts
libc.src.stdio.setbuf libc.src.stdio.setbuf
▲ Show 20 Lines • Show All 89 Lines • Show Last 20 Lines

libc/config/linux/x86_64/entrypoints.txt

Show First 20 Lines • Show All 417 Lines • ▼ Show 20 Lines list(APPEND TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.fread_unlocked libc.src.stdio.fread_unlocked
libc.src.stdio.fseek libc.src.stdio.fseek
libc.src.stdio.ftell libc.src.stdio.ftell
libc.src.stdio.funlockfile libc.src.stdio.funlockfile
libc.src.stdio.fwrite libc.src.stdio.fwrite
libc.src.stdio.fwrite_unlocked libc.src.stdio.fwrite_unlocked
libc.src.stdio.getc libc.src.stdio.getc
libc.src.stdio.getc_unlocked libc.src.stdio.getc_unlocked
libc.src.stdio.getchar
lntueUnsubmitted
Done
ReplyInline Actions

Can this also be enabled for ARM targets?

lntue: Can this also be enabled for ARM targets?
libc.src.stdio.getchar_unlocked
libc.src.stdio.sscanf libc.src.stdio.sscanf
libc.src.stdio.scanf libc.src.stdio.scanf
libc.src.stdio.fscanf libc.src.stdio.fscanf
libc.src.stdio.putc libc.src.stdio.putc
libc.src.stdio.putchar libc.src.stdio.putchar
libc.src.stdio.puts libc.src.stdio.puts
libc.src.stdio.setbuf libc.src.stdio.setbuf
libc.src.stdio.setvbuf libc.src.stdio.setvbuf
▲ Show 20 Lines • Show All 88 Lines • Show Last 20 Lines

libc/docs/stdio.rst

Show First 20 Lines • Show All 77 Lines • ▼ Show 20 Lines
The ``gets`` function was removed in C11 for having no bounds checking and The ``gets`` function was removed in C11 for having no bounds checking and
therefor being impossible to use safely. therefor being impossible to use safely.
============= ========= ============= =========
Function Name Available Function Name Available
============= ========= ============= =========
(f)getc |check| (f)getc |check|
fgets |check| fgets |check|
getchar getchar |check|
fread |check| fread |check|
(f)putc |check| (f)putc |check|
(f)puts |check| (f)puts |check|
putchar |check| putchar |check|
fwrite |check| fwrite |check|
ungetc |check| ungetc |check|
============= ========= ============= =========

libc/spec/posix.td

Show First 20 Lines • Show All 1,071 Lines • ▼ Show 20 Lines HeaderSpec StdIO = HeaderSpec<
RetValSpec<VoidType>, RetValSpec<VoidType>,
[ArgSpec<FILEPtr>] [ArgSpec<FILEPtr>]
>, >,
FunctionSpec< FunctionSpec<
"getc_unlocked", "getc_unlocked",
RetValSpec<IntType>, RetValSpec<IntType>,
[ArgSpec<FILEPtr>] [ArgSpec<FILEPtr>]
>, >,
FunctionSpec<
"getchar_unlocked",
RetValSpec<IntType>,
[ArgSpec<VoidType>]
xbjfkAuthorUnsubmitted
Done
ReplyInline Actions

Is this the correct way to define a function that takes no arguments?

xbjfk: Is this the correct way to define a function that takes no arguments?
michaelrjUnsubmitted
Done
ReplyInline Actions

good catch, it should be ArgSpec<VoidType>

michaelrj: good catch, it should be `ArgSpec<VoidType>`
>,
] ]
>; >;
HeaderSpec Dirent = HeaderSpec< HeaderSpec Dirent = HeaderSpec<
"dirent.h", "dirent.h",
[], // Macros [], // Macros
[InoT, StructDirent, DIR], // Types [InoT, StructDirent, DIR], // Types
[], // Enumerations [], // Enumerations
▲ Show 20 Lines • Show All 248 Lines • Show Last 20 Lines

libc/spec/stdc.td

Show First 20 Lines • Show All 575 Lines • ▼ Show 20 Lines HeaderSpec StdIO = HeaderSpec<
[ArgSpec<FILEPtr>] [ArgSpec<FILEPtr>]
>, >,
FunctionSpec< FunctionSpec<
"getc", "getc",
RetValSpec<IntType>, RetValSpec<IntType>,
[ArgSpec<FILEPtr>] [ArgSpec<FILEPtr>]
>, >,
FunctionSpec< FunctionSpec<
"getchar",
RetValSpec<IntType>,
[ArgSpec<VoidType>]
michaelrjUnsubmitted
Done
ReplyInline Actions

same here, it should be ArgSpec<VoidType>

michaelrj: same here, it should be `ArgSpec<VoidType>`
>,
FunctionSpec<
"putc", "putc",
RetValSpec<IntType>, RetValSpec<IntType>,
[ArgSpec<IntType>, [ArgSpec<IntType>,
ArgSpec<FILEPtr>] ArgSpec<FILEPtr>]
>, >,
FunctionSpec< FunctionSpec<
"putchar", "putchar",
RetValSpec<IntType>, RetValSpec<IntType>,
▲ Show 20 Lines • Show All 518 Lines • Show Last 20 Lines

libc/src/stdio/CMakeLists.txt

Show First 20 Lines • Show All 149 Lines • ▼ Show 20 Linesadd_entrypoint_object(
DEPENDS DEPENDS
libc.src.errno.errno libc.src.errno.errno
libc.include.stdio libc.include.stdio
libc.src.__support.File.file libc.src.__support.File.file
libc.src.__support.File.platform_file libc.src.__support.File.platform_file
) )
add_entrypoint_object( add_entrypoint_object(
getchar
SRCS
getchar.cpp
HDRS
getchar.h
DEPENDS
libc.src.errno.errno
libc.include.stdio
libc.src.__support.File.file
libc.src.__support.File.platform_file
)
add_entrypoint_object(
getchar_unlocked
SRCS
getc_unlocked.cpp
HDRS
getc_unlocked.h
DEPENDS
libc.src.errno.errno
libc.include.stdio
libc.src.__support.File.file
libc.src.__support.File.platform_file
)
add_entrypoint_object(
fgets fgets
SRCS SRCS
fgets.cpp fgets.cpp
HDRS HDRS
fgets.h fgets.h
DEPENDS DEPENDS
libc.src.errno.errno libc.src.errno.errno
libc.include.stdio libc.include.stdio
▲ Show 20 Lines • Show All 374 Lines • Show Last 20 Lines

libc/src/stdio/getchar.h

  • This file was added.
//===-- Implementation header of getchar ------------------------*- 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_GETCHAR_H
#define LLVM_LIBC_SRC_STDIO_GETCHAR_H
namespace __llvm_libc {
int getchar();
} // namespace __llvm_libc
#endif // LLVM_LIBC_SRC_STDIO_GETCHAR_H

libc/src/stdio/getchar.cpp

  • This file was added.
//===-- Implementation of getchar -----------------------------------------===//
//
// 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/getchar.h"
#include "src/__support/File/file.h"
#include "src/errno/libc_errno.h"
#include <stdio.h>
namespace __llvm_libc {
LLVM_LIBC_FUNCTION(int, getchar, ()) {
unsigned char c;
auto result = stdin->read(&c, 1);
if (result.has_error())
libc_errno = result.error;
if (result.value != 1)
return EOF;
return c;
sivachandraUnsubmitted
Done
ReplyInline Actions

Nit: This can be if (result.value != 1). Same in getchar_unlocked.

sivachandra: Nit: This can be `if (result.value != 1)`. Same in `getchar_unlocked`.
}
} // namespace __llvm_libc

libc/src/stdio/getchar_unlocked.h

  • This file was added.
//===-- Implementation header of getchar_unlocked ---------------*- 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_GETCHAR_UNLOCKED_H
#define LLVM_LIBC_SRC_STDIO_GETCHAR_UNLOCKED_H
namespace __llvm_libc {
int getchar_unlocked();
} // namespace __llvm_libc
#endif // LLVM_LIBC_SRC_STDIO_GETCHAR_UNLOCKED_H

libc/src/stdio/getchar_unlocked.cpp

  • This file was added.
//===-- Implementation of getchar_unlocked --------------------------------===//
//
// 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/getchar_unlocked.h"
#include "src/__support/File/file.h"
#include "src/errno/libc_errno.h"
#include <stdio.h>
namespace __llvm_libc {
LLVM_LIBC_FUNCTION(int, getchar_unlocked, ()) {
unsigned char c;
auto result = stdin->read_unlocked(&c, 1);
if (result.has_error())
libc_errno = result.error;
if (result.value != 1)
return EOF;
return c;
}
} // namespace __llvm_libc