https://github.com/nuclearcat updated https://github.com/llvm/llvm-project/pull/224979
>From 322a3c059ac87e50492ceeba8376adf496bd6ef1 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 statically known buffer-size mismatches in read, pread(64), readlink(at), and getcwd, and source over-reads in write and pwrite(64). Reuse FortifiedBufferChecker's destination-size and source-overread checks. Match these functions by name and external C prototype, accepting libc differences in ssize_t and off_t while rejecting incompatible declarations and internal-linkage helpers. Add coverage for matching prototypes, typedef differences, and same-name declarations that should be ignored. Split the Sema changes 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/lib/Sema/SemaChecking.cpp | 137 +++++++++++++++++- .../Sema/warn-fortify-source-prototype-gate.c | 81 +++++++++++ .../Sema/warn-fortify-source-signed-count.c | 17 +++ clang/test/Sema/warn-fortify-source-typedef.c | 18 +++ clang/test/Sema/warn-fortify-source.c | 68 +++++++++ 6 files changed, 324 insertions(+), 2 deletions(-) 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 diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index e5da258b9950a3..3cfb92c6da830d 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/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index ea8f3babcb0e01..cabbd9c134c54c 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1299,8 +1299,66 @@ class FortifiedBufferChecker { const DiagnoseAsBuiltinAttr *DABAttr; unsigned SizeTypeWidth; }; + +// read/write/readlink/readlinkat/getcwd and pread/pwrite are deliberately not +// Clang builtins: the former are common identifiers whose builtin declarations +// would clash with unrelated user code, and pread/pwrite use off_t, whose +// width is target- and _FILE_OFFSET_BITS-dependent. They are matched by name +// against the POSIX prototype below instead. +struct LibcFuncDesc { + enum ParamType { + Int, + VoidPtr, + ConstVoidPtr, + CharPtr, + ConstCharPtr, + Size, + SignedOffset + }; + ParamType Params[4]; + unsigned NumParams; + bool ReturnsPointer; + unsigned BufIdx; + unsigned CountIdx; + // True if CountIdx bytes are read from BufIdx (source over-read, like write); + // false if written into BufIdx (destination mismatch, like read). + bool BufIsSource; +}; } // anonymous namespace +static const LibcFuncDesc *lookupLibCFunctionDesc(StringRef Name) { + using D = LibcFuncDesc; + // Store only type-independent metadata: QualTypes belong to an ASTContext. + static constexpr D Getcwd = {{D::CharPtr, D::Size}, 2, true, 0, 1, false}; + static constexpr D Read = { + {D::Int, D::VoidPtr, D::Size}, 3, false, 1, 2, false}; + static constexpr D Write = { + {D::Int, D::ConstVoidPtr, D::Size}, 3, false, 1, 2, true}; + static constexpr D Readlink = { + {D::ConstCharPtr, D::CharPtr, D::Size}, 3, false, 1, 2, false}; + static constexpr D Readlinkat = { + {D::Int, D::ConstCharPtr, D::CharPtr, D::Size}, 4, false, 2, 3, false}; + static constexpr D Pread = { + {D::Int, D::VoidPtr, D::Size, D::SignedOffset}, 4, false, 1, 2, false}; + static constexpr D Pwrite = { + {D::Int, D::ConstVoidPtr, D::Size, D::SignedOffset}, + 4, + false, + 1, + 2, + true}; + + return llvm::StringSwitch<const D *>(Name) + .Case("getcwd", &Getcwd) + .Case("read", &Read) + .Case("write", &Write) + .Case("readlink", &Readlink) + .Case("readlinkat", &Readlinkat) + .Cases({"pread", "pread64"}, &Pread) + .Cases({"pwrite", "pwrite64"}, &Pwrite) + .Default(nullptr); +} + void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall) { if (TheCall->isInstantiationDependent() || isConstantEvaluatedContext()) @@ -1309,7 +1367,68 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, FortifiedBufferChecker Checker(*this, FD, TheCall); unsigned BuiltinID = Checker.getBuiltinID(); - if (!BuiltinID) + + // Match the non-builtin libc I/O functions (see LibcFuncDesc) by name. Only + // external-linkage C declarations qualify: a static same-name helper is + // file-local and not the libc function. + const LibcFuncDesc *LibCMatch = nullptr; + if (!BuiltinID && FD->isExternC() && FD->hasExternalFormalLinkage() && + FD->getIdentifier() && !FD->isVariadic()) { + if (const auto *Desc = + lookupLibCFunctionDesc(FD->getIdentifier()->getName())) { + ASTContext &Ctx = getASTContext(); + // ssize_t is a signed integer of size_t width whose spelling varies + // across libcs, so a non-pointer return is matched structurally (getcwd, + // the only pointer-returning entry, matches exactly). The off_t offset + // matches any signed integer (its width varies); the int fd, pointer + // buffer, and size_t count must match exactly -- pinning identity and + // keeping a signed count out of the (asserted-unsigned) size-argument + // evaluation. + auto IsSSizeT = [&](QualType T) { + return T->isSignedIntegerType() && + Ctx.getTypeSize(T) == Ctx.getTypeSize(Ctx.getSizeType()); + }; + QualType RetTy = FD->getReturnType(); + bool Matches = + FD->getNumParams() == Desc->NumParams && + TheCall->getNumArgs() == Desc->NumParams && + (Desc->ReturnsPointer ? Ctx.hasSameUnqualifiedType( + Ctx.getPointerType(Ctx.CharTy), RetTy) + : IsSSizeT(RetTy)); + for (unsigned I = 0; Matches && I < Desc->NumParams; ++I) { + QualType ActualTy = FD->getParamDecl(I)->getType(); + QualType ExpectedTy; + switch (Desc->Params[I]) { + case LibcFuncDesc::Int: + ExpectedTy = Ctx.IntTy; + break; + case LibcFuncDesc::VoidPtr: + ExpectedTy = Ctx.VoidPtrTy; + break; + case LibcFuncDesc::ConstVoidPtr: + ExpectedTy = Ctx.getPointerType(Ctx.VoidTy.withConst()); + break; + case LibcFuncDesc::CharPtr: + ExpectedTy = Ctx.getPointerType(Ctx.CharTy); + break; + case LibcFuncDesc::ConstCharPtr: + ExpectedTy = Ctx.getPointerType(Ctx.CharTy.withConst()); + break; + case LibcFuncDesc::Size: + ExpectedTy = Ctx.getSizeType(); + break; + case LibcFuncDesc::SignedOffset: + Matches = ActualTy->isSignedIntegerType(); + continue; + } + Matches = Ctx.hasSameUnqualifiedType(ExpectedTy, ActualTy); + } + if (Matches) + LibCMatch = Desc; + } + } + + if (!BuiltinID && !LibCMatch) return; unsigned SizeTypeWidth = Checker.getSizeTypeWidth(); @@ -1318,9 +1437,22 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, std::optional<llvm::APSInt> DestinationSize; unsigned DiagID = 0; + if (LibCMatch) { + if (LibCMatch->BufIsSource) { + // write/pwrite: a source over-read, same as the memcpy family below. + Checker.checkSourceOverread(LibCMatch->BufIdx, LibCMatch->CountIdx); + return; + } + DiagID = diag::warn_fortify_source_size_mismatch; + SourceSize = Checker.ComputeExplicitObjectSizeArgument(LibCMatch->CountIdx); + DestinationSize = Checker.ComputeSizeArgument(LibCMatch->BufIdx); + } + + // For a LibCMatch the sizes are already set above and BuiltinID is zero, so + // the switch falls through its default to the shared size check below. switch (BuiltinID) { default: - return; + break; case Builtin::BI__builtin_strcat: case Builtin::BIstrcat: case Builtin::BI__builtin_stpcpy: @@ -1557,6 +1689,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, const Expr *Dest = TheCall->getArg(0)->IgnoreCasts(); IdentifierInfo *FnInfo = FD->getIdentifier(); CheckSizeofMemaccessArgument(LenArg, Dest, FnInfo); + break; } } 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 00000000000000..75e9d9897ef143 --- /dev/null +++ b/clang/test/Sema/warn-fortify-source-prototype-gate.c @@ -0,0 +1,81 @@ +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -Werror + +// The fortify dispatch for read/write/pread/pwrite/readlink/readlinkat/getcwd +// is gated on the full POSIX prototype: a function sharing one of these names +// but with a different signature must not be diagnosed as the libc function. + +typedef unsigned long size_t; +typedef long ssize_t; + +// expected-no-diagnostics + +// Variadic: prefix happens to match POSIX read, but the varargs make this +// an unrelated declaration. +ssize_t read(int, void *, size_t, ...); + +void test_read_variadic(void) { + char b[4]; + read(0, b, 8); +} + +// Unsigned return (size_t): only the return differs from POSIX write (which +// returns signed ssize_t), isolating the return-type gate. +size_t write(int fd, const void *buf, size_t n); + +void test_write_unsigned_return(void) { + char buf[4]; + write(0, buf, 8); +} + +// Wrong return type (void) for readlink. +void readlink(const char *p, char *b, size_t n); + +void test_readlink_wrong_return(void) { + char b[4]; + readlink("/", b, 8); +} + +// Wrong second parameter type (int instead of char*) for getcwd. +char *getcwd(int x, size_t n); + +void test_getcwd_wrong_param(void) { + getcwd(42, 8); +} + +// pread64 with 'char *' buffer instead of POSIX 'void *'. Newlib-style +// syscall stubs commonly use this shape; the pointer-type mismatch trips +// the prototype gate. +int pread64(int, char *, size_t, long long); + +void test_pread64_newlib_buffer(void) { + char b[4]; + pread64(0, b, 8, 0); +} + +// pwrite64 with 'const char *' instead of POSIX 'const void *': same shape. +int pwrite64(int, const char *, size_t, long long); + +void test_pwrite64_newlib_buffer(void) { + char b[4]; + pwrite64(0, b, 8, 0); +} + +// Unsigned offset: off_t/off64_t are signed, so only the offset slot differs +// from POSIX pread. It is matched as a signed integer, so this is rejected. +ssize_t pread(int, void *, size_t, unsigned long); + +void test_pread_unsigned_offset(void) { + char b[4]; + pread(0, b, 8, 0); +} + +// static (internal-linkage) lookalike with the exact POSIX signature: it is +// file-local, not the libc function, so it must not be diagnosed. +static ssize_t pwrite(int fd, const void *buf, size_t n, long off) { + return 0; +} + +void test_pwrite_static_lookalike(void) { + char b[4]; + pwrite(0, b, 8, 0); +} 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 00000000000000..e67675621261ed --- /dev/null +++ b/clang/test/Sema/warn-fortify-source-signed-count.c @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -Werror + +// A read() whose count parameter is a signed 'int' rather than POSIX size_t +// must not be treated as the libc read(): the size_t count slot is matched +// exactly, so this lookalike is not diagnosed. + +typedef unsigned long size_t; +typedef long ssize_t; + +// expected-no-diagnostics + +ssize_t read(int fd, void *buf, int count); + +void test_read_signed_count(void) { + char b[4]; + read(0, b, 8); +} 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 00000000000000..af7c7565d7658c --- /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; the gate 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.c b/clang/test/Sema/warn-fortify-source.c index 8339efddc5b3e7..735570725657ce 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -24,6 +24,19 @@ void *memcpy(void *dst, const void *src, size_t c); 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 @@ -132,6 +145,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
