https://github.com/nuclearcat updated https://github.com/llvm/llvm-project/pull/224979
>From f5fb762d7c09cb11d2c589809f10121098be1c6a Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko <[email protected]> Date: Mon, 21 Sep 2026 03:28:40 +0300 Subject: [PATCH] [clang][Sema] Add buffer-size checks for unistd I/O functions Diagnose destination buffer-size mismatches in read, pread(64), readlink(at), and getcwd, and source over-reads in write and pwrite(64). Register library builtins with IgnoreSignature and empty prototypes, following recv/recvfrom, and guard checks by argument count and types to accommodate libc differences. Add regression coverage; all 31 focused tests pass with assertions enabled. Split from #196499, based on #161737. Co-authored-by: Colin Kinloch <[email protected]> Signed-off-by: Denys Fedoryshchenko <[email protected]> --- clang/docs/ReleaseNotes.md | 5 + clang/include/clang/Basic/Builtins.td | 57 ++++++ clang/lib/Sema/SemaChecking.cpp | 37 ++++ .../Sema/warn-fortify-source-prototype-gate.c | 186 ++++++++++++++++++ .../Sema/warn-fortify-source-signed-count.c | 34 ++++ clang/test/Sema/warn-fortify-source-typedef.c | 18 ++ .../Sema/warn-fortify-source-undeclared.c | 16 ++ clang/test/Sema/warn-fortify-source.c | 68 +++++++ 8 files changed, 421 insertions(+) create mode 100644 clang/test/Sema/warn-fortify-source-prototype-gate.c create mode 100644 clang/test/Sema/warn-fortify-source-signed-count.c create mode 100644 clang/test/Sema/warn-fortify-source-typedef.c create mode 100644 clang/test/Sema/warn-fortify-source-undeclared.c diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 3c6acf353f93f..1b6439f44df7c 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -284,6 +284,11 @@ features cannot lower the translation-unit ABI level; ### Improvements to Clang's diagnostics +- `-Wfortify-source` now diagnoses destination buffer-size mismatches in calls to + `read`, `pread`, `pread64`, `readlink`, `readlinkat`, and `getcwd` when the buffer + size and byte count are statically known. `-Wstringop-overread` diagnoses source + buffer over-reads in `write`, `pwrite`, and `pwrite64`. + - `-Wfortify-source` now diagnoses when `strlcat`, `__builtin_strlcat`, `strlcpy`, or `__builtin_strlcpy` is called with a size argument larger than the destination buffer. diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index f1628c175490f..05dc560a2ae58 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -3860,6 +3860,63 @@ def VFork : LibBuiltin<"unistd.h"> { let Prototype = "pid_t()"; } +// The unistd I/O signatures vary across targets (ssize_t, off_t and count +// types). Require a declaration rather than synthesizing a prototype. + +def Read : LibBuiltin<"unistd.h"> { + let Spellings = ["read"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def Write : LibBuiltin<"unistd.h"> { + let Spellings = ["write"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def PRead : LibBuiltin<"unistd.h"> { + let Spellings = ["pread"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def PRead64 : LibBuiltin<"unistd.h"> { + let Spellings = ["pread64"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def PWrite : LibBuiltin<"unistd.h"> { + let Spellings = ["pwrite"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def PWrite64 : LibBuiltin<"unistd.h"> { + let Spellings = ["pwrite64"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def ReadLink : LibBuiltin<"unistd.h"> { + let Spellings = ["readlink"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def ReadLinkAt : LibBuiltin<"unistd.h"> { + let Spellings = ["readlinkat"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + +def GetCwd : LibBuiltin<"unistd.h"> { + let Spellings = ["getcwd"]; + let Attributes = [IgnoreSignature]; + let Prototype = ""; +} + // POSIX sys/stat.h def Umask : LibBuiltin<"sys/stat.h"> { diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 687af75c1496b..234ff4dce87dc 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1466,6 +1466,43 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, break; } + case Builtin::BIread: + case Builtin::BIpread: + case Builtin::BIpread64: + case Builtin::BIreadlink: + case Builtin::BIreadlinkat: + case Builtin::BIgetcwd: { + unsigned BufIdx = 1; + if (BuiltinID == Builtin::BIgetcwd) + BufIdx = 0; + else if (BuiltinID == Builtin::BIreadlinkat) + BufIdx = 2; + unsigned CountIdx = BufIdx + 1; + unsigned ExpectedArgs = CountIdx + 1; + if (BuiltinID == Builtin::BIpread || BuiltinID == Builtin::BIpread64) + ++ExpectedArgs; + if (TheCall->getNumArgs() != ExpectedArgs || + !TheCall->getArg(BufIdx)->getType()->isPointerType() || + !TheCall->getArg(CountIdx)->getType()->isIntegerType()) + return; + DiagID = diag::warn_fortify_source_size_mismatch; + SourceSize = Checker.ComputeExplicitObjectSizeArgument(CountIdx); + DestinationSize = Checker.ComputeSizeArgument(BufIdx); + break; + } + + case Builtin::BIwrite: + case Builtin::BIpwrite: + case Builtin::BIpwrite64: { + unsigned ExpectedArgs = BuiltinID == Builtin::BIwrite ? 3 : 4; + if (TheCall->getNumArgs() != ExpectedArgs || + !TheCall->getArg(1)->getType()->isPointerType() || + !TheCall->getArg(2)->getType()->isIntegerType()) + return; + Checker.checkSourceOverread(1, 2); + return; + } + case Builtin::BIrecv: case Builtin::BIrecvfrom: { unsigned ExpectedArgs = BuiltinID == Builtin::BIrecv ? 4 : 6; diff --git a/clang/test/Sema/warn-fortify-source-prototype-gate.c b/clang/test/Sema/warn-fortify-source-prototype-gate.c new file mode 100644 index 0000000000000..a889d1a364ea6 --- /dev/null +++ b/clang/test/Sema/warn-fortify-source-prototype-gate.c @@ -0,0 +1,186 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c %s -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ %s -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c %s -DWRONG_ARITY -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ %s -DWRONG_ARITY -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c %s -DWRONG_BUFFER -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ %s -DWRONG_BUFFER -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c %s -DWRONG_COUNT -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ %s -DWRONG_COUNT -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c %s -DNO_BUILTIN -fno-builtin -verify -Werror +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ %s -DNO_BUILTIN -fno-builtin -verify -Werror +// expected-no-diagnostics + +// No __builtin_ aliases are provided. The library names are recognized +// unless builtin recognition is disabled. Ignore unrelated argument shapes, +// internal linkage, and C++ language linkage. +typedef __SIZE_TYPE__ size_t; + +#if __has_builtin(__builtin_read) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(read) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_write) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(write) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_pread) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(pread) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_pread64) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(pread64) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_pwrite) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(pwrite) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_pwrite64) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(pwrite64) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_readlink) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(readlink) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_readlinkat) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(readlinkat) +#error missing library builtin +#endif +#endif +#if __has_builtin(__builtin_getcwd) +#error unexpected builtin alias +#endif +#ifndef NO_BUILTIN +#if !__has_builtin(getcwd) +#error missing library builtin +#endif +#endif + +#if defined(WRONG_ARITY) || defined(WRONG_BUFFER) || defined(WRONG_COUNT) || defined(NO_BUILTIN) +#ifdef WRONG_ARITY +#define LAST(x) +#else +#define LAST(x) , x +#endif +#ifdef WRONG_BUFFER +#define BUFFER int +#define BUF_ARG 0 +#else +#define BUFFER void * +#define BUF_ARG buf +#endif +#ifdef WRONG_COUNT +#define COUNT double +#else +#define COUNT size_t +#endif + +#ifdef __cplusplus +extern "C" { +#endif +int read(int, BUFFER LAST(COUNT)); +int write(int, BUFFER LAST(COUNT)); +int pread(int, BUFFER, COUNT LAST(long)); +int pread64(int, BUFFER, COUNT LAST(long long)); +int pwrite(int, BUFFER, COUNT LAST(long)); +int pwrite64(int, BUFFER, COUNT LAST(long long)); +int readlink(const char *, BUFFER LAST(COUNT)); +int readlinkat(int, const char *, BUFFER LAST(COUNT)); +int getcwd(BUFFER LAST(COUNT)); +#ifdef __cplusplus +} +#endif + +void call_mismatched(void) { + char buf[4]; + read(0, BUF_ARG LAST(8)); + write(0, BUF_ARG LAST(8)); + pread(0, BUF_ARG, 8 LAST(0)); + pread64(0, BUF_ARG, 8 LAST(0)); + pwrite(0, BUF_ARG, 8 LAST(0)); + pwrite64(0, BUF_ARG, 8 LAST(0)); + readlink("/", BUF_ARG LAST(8)); + readlinkat(0, "/", BUF_ARG LAST(8)); + getcwd(BUF_ARG LAST(8)); +} +#else +static int read(int arg0, void * arg1, size_t arg2) { return 0; } +static int write(int arg0, const void * arg1, size_t arg2) { return 0; } +static int pread(int arg0, void * arg1, size_t arg2, long arg3) { return 0; } +static int pread64(int arg0, void * arg1, size_t arg2, long long arg3) { return 0; } +static int pwrite(int arg0, const void * arg1, size_t arg2, long arg3) { return 0; } +static int pwrite64(int arg0, const void * arg1, size_t arg2, long long arg3) { return 0; } +static int readlink(const char * arg0, char * arg1, size_t arg2) { return 0; } +static int readlinkat(int arg0, const char * arg1, char * arg2, size_t arg3) { return 0; } +static int getcwd(char * arg0, size_t arg1) { return 0; } +void call_static(void) { + char buf[4]; + read(0, buf, 8); + write(0, buf, 8); + pread(0, buf, 8, 0); + pread64(0, buf, 8, 0); + pwrite(0, buf, 8, 0); + pwrite64(0, buf, 8, 0); + readlink("/", buf, 8); + readlinkat(0, "/", buf, 8); + getcwd(buf, 8); +} + +#ifdef __cplusplus +namespace user { +int read(int, void *, size_t); +int write(int, const void *, size_t); +int pread(int, void *, size_t, long); +int pread64(int, void *, size_t, long long); +int pwrite(int, const void *, size_t, long); +int pwrite64(int, const void *, size_t, long long); +int readlink(const char *, char *, size_t); +int readlinkat(int, const char *, char *, size_t); +int getcwd(char *, size_t); +void call(void) { + char buf[4]; + read(0, buf, 8); + write(0, buf, 8); + pread(0, buf, 8, 0); + pread64(0, buf, 8, 0); + pwrite(0, buf, 8, 0); + pwrite64(0, buf, 8, 0); + readlink("/", buf, 8); + readlinkat(0, "/", buf, 8); + getcwd(buf, 8); +} +} // namespace user +#endif +#endif diff --git a/clang/test/Sema/warn-fortify-source-signed-count.c b/clang/test/Sema/warn-fortify-source-signed-count.c new file mode 100644 index 0000000000000..6ac2dad7b5773 --- /dev/null +++ b/clang/test/Sema/warn-fortify-source-signed-count.c @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -triple i686-unknown-linux %s -verify=expected,signed32 +// RUN: %clang_cc1 -triple i686-unknown-linux %s -fexperimental-new-constant-interpreter -verify=expected,signed32 +// RUN: %clang_cc1 -triple i686-unknown-linux %s -DUNSIGNED_COUNT -verify=expected,unsigned32 +// RUN: %clang_cc1 -triple i686-unknown-linux %s -DUNSIGNED_COUNT -fexperimental-new-constant-interpreter -verify=expected,unsigned32 +// RUN: %clang_cc1 -triple x86_64-unknown-linux %s -verify=expected,signed64 +// RUN: %clang_cc1 -triple x86_64-unknown-linux %s -fexperimental-new-constant-interpreter -verify=expected,signed64 +// RUN: %clang_cc1 -triple x86_64-unknown-linux %s -DUNSIGNED_COUNT -verify=expected,unsigned64 +// RUN: %clang_cc1 -triple x86_64-unknown-linux %s -DUNSIGNED_COUNT -fexperimental-new-constant-interpreter -verify=expected,unsigned64 +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc %s -verify=expected,signed64 +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc %s -fexperimental-new-constant-interpreter -verify=expected,signed64 +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc %s -DUNSIGNED_COUNT -verify=expected,unsigned64 +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc %s -DUNSIGNED_COUNT -fexperimental-new-constant-interpreter -verify=expected,unsigned64 + +// Count and return types need not match POSIX size_t and ssize_t. In +// particular, Windows read/write use unsigned int counts on LLP64. +#ifdef UNSIGNED_COUNT +typedef unsigned int count_t; +#else +typedef int count_t; +#endif +int read(int, char *, count_t); +int write(int, const char *, count_t); + +void test_counts(count_t n) { + char buf[4]; + read(0, buf, 4); + write(0, buf, 4); + read(0, buf, 8); // expected-warning {{'read' size argument is too large; destination buffer has size 4, but size argument is 8}} + write(0, buf, 8); // expected-warning {{'write' reading 8 bytes from a region of size 4}} + read(0, buf, n); + write(0, buf, n); + read(0, buf, -1); // signed32-warning {{size argument is 4294967295}} signed64-warning {{size argument is 18446744073709551615}} unsigned32-warning {{size argument is 4294967295}} unsigned64-warning {{size argument is 4294967295}} + write(0, buf, -1); // signed32-warning {{reading 4294967295 bytes}} signed64-warning {{reading 18446744073709551615 bytes}} unsigned32-warning {{reading 4294967295 bytes}} unsigned64-warning {{reading 4294967295 bytes}} +} diff --git a/clang/test/Sema/warn-fortify-source-typedef.c b/clang/test/Sema/warn-fortify-source-typedef.c new file mode 100644 index 0000000000000..1e67eef7c1d65 --- /dev/null +++ b/clang/test/Sema/warn-fortify-source-typedef.c @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -triple i686-unknown-linux %s -verify +// RUN: %clang_cc1 -triple x86_64-unknown-linux %s -verify + +// Verify that the fortify dispatch is tolerant of libc typedef choices for +// ssize_t. glibc uses `long` while other libcs (musl, bionic) may use a +// type that matches Clang's signed counterpart of size_t. On ILP32 these +// differ canonically (`long` vs `int`) even though both are 32-bit signed +// integers; builtin recognition must accept either as ssize_t. + +typedef __SIZE_TYPE__ size_t; +typedef long ssize_t; + +ssize_t read(int, void *, size_t); + +void test_read(void) { + char b[4]; + read(0, b, 8); // expected-warning {{'read' size argument is too large; destination buffer has size 4, but size argument is 8}} +} diff --git a/clang/test/Sema/warn-fortify-source-undeclared.c b/clang/test/Sema/warn-fortify-source-undeclared.c new file mode 100644 index 0000000000000..64884c2324447 --- /dev/null +++ b/clang/test/Sema/warn-fortify-source-undeclared.c @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c99 %s -verify +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ %s -verify + +// Empty builtin prototypes must not provide implicit library declarations. +void call_undeclared(void) { + char buf[4]; + read(0, buf, 4); // expected-error {{undeclared}} + write(0, buf, 4); // expected-error {{undeclared}} + pread(0, buf, 4, 0); // expected-error {{undeclared}} + pread64(0, buf, 4, 0); // expected-error {{undeclared}} + pwrite(0, buf, 4, 0); // expected-error {{undeclared}} + pwrite64(0, buf, 4, 0); // expected-error {{undeclared}} + readlink("/", buf, 4); // expected-error {{undeclared}} + readlinkat(0, "/", buf, 4); // expected-error {{undeclared}} + getcwd(buf, 4); // expected-error {{undeclared}} +} diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index cdfe33454707d..d69da959a4dba 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -32,6 +32,19 @@ ssize_t recvfrom(int, void *, size_t, int, struct sockaddr *, socklen_t *); void bcopy(const void *src, void *dst, size_t n); void bzero(void *dst, size_t n); +typedef long ssize_t; +typedef long off_t; +typedef long long off64_t; +ssize_t read(int fd, void *buf, size_t count); +ssize_t write(int fd, const void *buf, size_t count); +ssize_t pread(int fd, void *buf, size_t count, off_t offset); +ssize_t pread64(int fd, void *buf, size_t count, off64_t offset); +ssize_t pwrite(int fd, const void *buf, size_t count, off_t offset); +ssize_t pwrite64(int fd, const void *buf, size_t count, off64_t offset); +char *getcwd(char *buf, size_t size); +ssize_t readlink(const char *path, char *buf, size_t bufsize); +ssize_t readlinkat(int fd, const char *path, char *buf, size_t bufsize); + #ifdef __cplusplus } #endif @@ -140,6 +153,61 @@ void call_bcopy_bzero(void) { __builtin_bzero(dst, 11); // expected-warning {{'bzero' will always overflow; destination buffer has size 10, but size argument is 11}} } +void call_read(void) { + char buf[10]; + read(0, buf, 10); + read(0, buf, 20); // expected-warning {{'read' size argument is too large; destination buffer has size 10, but size argument is 20}} +} + +void call_pread(void) { + char buf[10]; + pread(0, buf, 10, 0); + pread(0, buf, 20, 0); // expected-warning {{'pread' size argument is too large; destination buffer has size 10, but size argument is 20}} +} + +void call_pread64(void) { + char buf[10]; + pread64(0, buf, 10, 0); + pread64(0, buf, 20, 0); // expected-warning {{'pread64' size argument is too large; destination buffer has size 10, but size argument is 20}} +} + +void call_write(void) { + char buf[10]; + write(0, buf, 10); + write(0, buf, 20); // expected-warning {{'write' reading 20 bytes from a region of size 10}} +} + +void call_pwrite(void) { + char buf[10]; + pwrite(0, buf, 10, 0); + pwrite(0, buf, 20, 0); // expected-warning {{'pwrite' reading 20 bytes from a region of size 10}} +} + +void call_pwrite64(void) { + char buf[10]; + pwrite64(0, buf, 10, 0); + pwrite64(0, buf, 20, 0); // expected-warning {{'pwrite64' reading 20 bytes from a region of size 10}} +} + +void call_getcwd(void) { + char buf[10]; + getcwd(buf, 10); + getcwd(buf, 20); // expected-warning {{'getcwd' size argument is too large; destination buffer has size 10, but size argument is 20}} +} + +void call_readlink(void) { + char buf[10]; + readlink("path", buf, 10); + readlink("path", buf, 20); // expected-warning {{'readlink' size argument is too large; destination buffer has size 10, but size argument is 20}} +} + +void call_readlinkat(void) { + char buf[10]; + readlinkat(0, "path", buf, 10); + readlinkat(0, "path", buf, 20); // expected-warning {{'readlinkat' size argument is too large; destination buffer has size 10, but size argument is 20}} +} + + void call_snprintf(double d, int n) { char buf[10]; __builtin_snprintf(buf, 10, "merp"); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
