https://github.com/ColinKinloch updated https://github.com/llvm/llvm-project/pull/161737
>From 98912f07d2b6f3a1d8c79d2e38407dc1d9ba2905 Mon Sep 17 00:00:00 2001 From: Colin Kinloch <[email protected]> Date: Thu, 2 Oct 2025 22:01:40 +0100 Subject: [PATCH 1/3] [clang][Sema] Add fortify warnings for `unistd.h` Define as builtin and check for overflows and over-reads in: * `read` * `pread`/`pread64` * `write` * `pwrite`/`pwrite64` * `getcwd` * `readlink` * `readlinkat` It also enables `off_t`, `off64_t` and `ssize_t` for use in builtin signatures. Signed-off-by: Colin Kinloch <[email protected]> --- clang/include/clang/Basic/Builtins.td | 65 +++++++++++++++++ .../clang/Basic/DiagnosticSemaKinds.td | 4 ++ clang/lib/AST/ASTContext.cpp | 9 ++- clang/lib/Sema/SemaChecking.cpp | 70 ++++++++++++++++++- clang/test/Sema/warn-fortify-source.c | 55 +++++++++++++++ clang/utils/TableGen/ClangBuiltinsEmitter.cpp | 3 + 6 files changed, 201 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index 468121f7d20ab..3c3a5a82a28a6 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -3490,6 +3490,8 @@ def StrnCaseCmp : GNULibBuiltin<"strings.h"> { let RequiresUndef = 1; } +// POSIX unistd.h + def GNU_Exit : GNULibBuiltin<"unistd.h"> { let Spellings = ["_exit"]; let Attributes = [NoReturn]; @@ -3502,6 +3504,69 @@ def VFork : LibBuiltin<"unistd.h"> { let Prototype = "pid_t()"; } +def Read : LibBuiltin<"unistd.h"> { + let Spellings = ["read"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, void*, size_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def PRead : LibBuiltin<"unistd.h"> { + let Spellings = ["pread"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, void*, size_t, off_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def PRead64 : LibBuiltin<"unistd.h"> { + let Spellings = ["pread64"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, void*, size_t, off64_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def Write : LibBuiltin<"unistd.h"> { + let Spellings = ["write"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, void const*, size_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def PWrite : LibBuiltin<"unistd.h"> { + let Spellings = ["pwrite"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, void const*, size_t, off_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def PWrite64 : LibBuiltin<"unistd.h"> { + let Spellings = ["pwrite64"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, void const*, size_t, off64_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def GetCWD : LibBuiltin<"unistd.h"> { + let Spellings = ["getcwd"]; + let Attributes = [NoThrow]; + let Prototype = "char*(char*, size_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def ReadLink : LibBuiltin<"unistd.h"> { + let Spellings = ["readlink"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(char const* restrict, char* restrict, size_t)"; + let AddBuiltinPrefixedAlias = 1; +} + +def ReadLinkAt : LibBuiltin<"unistd.h"> { + let Spellings = ["readlinkat"]; + let Attributes = [NoThrow]; + let Prototype = "ssize_t(int, char const* restrict, char* restrict, size_t)"; + let AddBuiltinPrefixedAlias = 1; +} + // POSIX pthread.h def PthreadCreate : GNULibBuiltin<"pthread.h"> { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b157cbb0b8069..f2206ad08414e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -953,6 +953,10 @@ def warn_fortify_source_overflow def warn_fortify_source_size_mismatch : Warning< "'%0' size argument is too large; destination buffer has size %1," " but size argument is %2">, InGroup<FortifySource>; +def warn_fortify_destination_size_mismatch + : Warning<"'%0' size argument is too large; source buffer has size %2," + " but size argument is %1">, + InGroup<FortifySource>; def warn_fortify_strlen_overflow: Warning< "'%0' will always overflow; destination buffer has size %1," diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 056bfe36b2a0a..d0432de14dbb3 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -12528,9 +12528,12 @@ static QualType DecodeTypeFromStr(const char *&Str, const ASTContext &Context, assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'b'!"); Type = Context.BoolTy; break; - case 'z': // size_t. - assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'z'!"); - Type = Context.getSizeType(); + case 'z': // size_t and ssize_t. + assert(HowLong == 0 && "Bad modifiers for 'z'!"); + if (Signed) + Type = Context.getSignedSizeType(); + else + Type = Context.getSizeType(); break; case 'w': // wchar_t. assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'w'!"); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 7b37e0b8d5430..c7d6d0c142eee 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1243,6 +1243,14 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, std::optional<llvm::APSInt> DestinationSize; unsigned DiagID = 0; bool IsChkVariant = false; + bool IsTriggered = false; + + auto CompareSizeSourceToDest = [&]() { + return SourceSize && DestinationSize + ? std::optional<int>{llvm::APSInt::compareValues( + *SourceSize, *DestinationSize)} + : std::nullopt; + }; auto GetFunctionName = [&]() { std::string FunctionNameStr = @@ -1270,6 +1278,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DiagID = diag::warn_fortify_strlen_overflow; SourceSize = ComputeStrLenArgument(1); DestinationSize = ComputeSizeArgument(0); + IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1279,6 +1288,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = ComputeStrLenArgument(1); DestinationSize = ComputeExplicitObjectSizeArgument(2); IsChkVariant = true; + IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1349,11 +1359,13 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, } else { DestinationSize = ComputeSizeArgument(0); } + IsTriggered = CompareSizeSourceToDest() > 0; break; } } return; } + case Builtin::BI__builtin___memcpy_chk: case Builtin::BI__builtin___memmove_chk: case Builtin::BI__builtin___memset_chk: @@ -1369,6 +1381,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DestinationSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); IsChkVariant = true; + IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1378,6 +1391,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, SourceSize = ComputeExplicitObjectSizeArgument(1); DestinationSize = ComputeExplicitObjectSizeArgument(3); IsChkVariant = true; + IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1395,6 +1409,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DiagID = diag::warn_fortify_source_size_mismatch; SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = ComputeSizeArgument(0); + IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1409,8 +1424,58 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, DiagID = diag::warn_fortify_source_overflow; SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = ComputeSizeArgument(0); + IsTriggered = CompareSizeSourceToDest() > 0; + break; + } + + case Builtin::BIread: + case Builtin::BI__builtin_read: + case Builtin::BIreadlink: + case Builtin::BI__builtin_readlink: + case Builtin::BIreadlinkat: + case Builtin::BI__builtin_readlinkat: + case Builtin::BIgetcwd: + case Builtin::BI__builtin_getcwd: { + DiagID = diag::warn_fortify_source_size_mismatch; + SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + DestinationSize = ComputeSizeArgument(TheCall->getNumArgs() - 2); + IsTriggered = CompareSizeSourceToDest() > 0; + break; + } + + case Builtin::BIpread: + case Builtin::BI__builtin_pread: + case Builtin::BIpread64: + case Builtin::BI__builtin_pread64: { + DiagID = diag::warn_fortify_source_size_mismatch; + SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); + DestinationSize = ComputeSizeArgument(TheCall->getNumArgs() - 3); + IsTriggered = CompareSizeSourceToDest() > 0; + break; + } + + case Builtin::BIwrite: + case Builtin::BI__builtin_write: { + DiagID = diag::warn_fortify_destination_size_mismatch; + SourceSize = ComputeSizeArgument(TheCall->getNumArgs() - 2); + DestinationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + IsTriggered = CompareSizeSourceToDest() < 0; break; } + + case Builtin::BIpwrite: + case Builtin::BI__builtin_pwrite: + case Builtin::BIpwrite64: + case Builtin::BI__builtin_pwrite64: { + DiagID = diag::warn_fortify_destination_size_mismatch; + SourceSize = ComputeSizeArgument(TheCall->getNumArgs() - 3); + DestinationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); + IsTriggered = CompareSizeSourceToDest() < 0; + break; + } + case Builtin::BIsnprintf: case Builtin::BI__builtin_snprintf: case Builtin::BIvsnprintf: @@ -1446,11 +1511,12 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, } } DestinationSize = ComputeSizeArgument(0); + IsTriggered = CompareSizeSourceToDest() > 0; + break; } } - if (!SourceSize || !DestinationSize || - llvm::APSInt::compareValues(*SourceSize, *DestinationSize) <= 0) + if (!IsTriggered) return; std::string FunctionName = GetFunctionName(); diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index 216878c0836d8..5d4a8d81b0b3e 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -96,6 +96,61 @@ void call_memset(void) { __builtin_memset(buf, 0xff, 11); // expected-warning {{'memset' will always overflow; destination buffer has size 10, but size argument is 11}} } +void call_read(void) { + char buf[10]; + __builtin_read(0, buf, 10); + __builtin_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]; + __builtin_pread(0, buf, 10, 0); + __builtin_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]; + __builtin_pread64(0, buf, 10, 0); + __builtin_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]; + __builtin_write(0, buf, 10); + __builtin_write(0, buf, 20); // expected-warning {{'write' size argument is too large; source buffer has size 10, but size argument is 20}} +} + +void call_pwrite(void) { + char buf[10]; + __builtin_pwrite(0, buf, 10, 0); + __builtin_pwrite(0, buf, 20, 0); // expected-warning {{'pwrite' size argument is too large; source buffer has size 10, but size argument is 20}} +} + +void call_pwrite64(void) { + char buf[10]; + __builtin_pwrite64(0, buf, 10, 0); + __builtin_pwrite64(0, buf, 20, 0); // expected-warning {{'pwrite64' size argument is too large; source buffer has size 10, but size argument is 20}} +} + +void call_getcwd(void) { + char buf[10]; + __builtin_getcwd(buf, 10); + __builtin_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]; + __builtin_readlink("path", buf, 10); + __builtin_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]; + __builtin_readlinkat(0, "path", buf, 10); + __builtin_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"); diff --git a/clang/utils/TableGen/ClangBuiltinsEmitter.cpp b/clang/utils/TableGen/ClangBuiltinsEmitter.cpp index 352765acf5338..30c7afd95a89a 100644 --- a/clang/utils/TableGen/ClangBuiltinsEmitter.cpp +++ b/clang/utils/TableGen/ClangBuiltinsEmitter.cpp @@ -347,12 +347,15 @@ class PrototypeParser { .Case("msint32_t", "Ni") .Case("msuint32_t", "UNi") .Case("objc_super", "M") + .Case("off_t", "Li") + .Case("off64_t", "Wi") .Case("pid_t", "p") .Case("ptrdiff_t", "Y") .Case("SEL", "H") .Case("short", "s") .Case("sigjmp_buf", "SJ") .Case("size_t", "z") + .Case("ssize_t", "Sz") .Case("ucontext_t", "K") .Case("uint32_t", "UZi") .Case("uint64_t", "UWi") >From 496eec840f8f0903da2b6cbd440631962ebad5e9 Mon Sep 17 00:00:00 2001 From: Colin Kinloch <[email protected]> Date: Mon, 3 Nov 2025 02:53:09 +0000 Subject: [PATCH 2/3] [clang][Sema] Add min/max operation size for fortify checks --- .../clang/Basic/DiagnosticSemaKinds.td | 8 +- clang/lib/Sema/SemaChecking.cpp | 162 +++++++++++------- clang/test/Sema/builtin-memcpy.c | 3 +- clang/test/Sema/warn-fortify-source.c | 39 ++++- 4 files changed, 140 insertions(+), 72 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f2206ad08414e..484f161194199 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -950,12 +950,16 @@ def warn_builtin_chk_overflow : Warning< def warn_fortify_source_overflow : Warning<warn_builtin_chk_overflow.Summary>, InGroup<FortifySource>; +def warn_fortify_destination_over_read + : Warning<"'%0' will always over-read; source buffer has size %1," + " but size argument is %2">, + InGroup<FortifySource>; def warn_fortify_source_size_mismatch : Warning< "'%0' size argument is too large; destination buffer has size %1," " but size argument is %2">, InGroup<FortifySource>; def warn_fortify_destination_size_mismatch - : Warning<"'%0' size argument is too large; source buffer has size %2," - " but size argument is %1">, + : Warning<"'%0' size argument is too large; source buffer has size %1," + " but size argument is %2">, InGroup<FortifySource>; def warn_fortify_strlen_overflow: Warning< diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index c7d6d0c142eee..3889a07d5f962 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1239,18 +1239,20 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, return llvm::APSInt::getUnsigned(Result + 1).extOrTrunc(SizeTypeWidth); }; + // Size of the memory read from std::optional<llvm::APSInt> SourceSize; + // Size of the memory written to std::optional<llvm::APSInt> DestinationSize; - unsigned DiagID = 0; + // Maximum operation size for detecting possible out of bounds access + std::optional<llvm::APSInt> MaxOperationSize; + // Minimum operation size for detecting definate out of bounds access + std::optional<llvm::APSInt> MinOperationSize; + + unsigned DiagOverflowID = diag::warn_fortify_source_overflow; + unsigned DiagMayOverflowID = diag::warn_fortify_source_size_mismatch; + unsigned DiagOverReadID = diag::warn_fortify_destination_over_read; + unsigned DiagMayOverReadID = diag::warn_fortify_destination_size_mismatch; bool IsChkVariant = false; - bool IsTriggered = false; - - auto CompareSizeSourceToDest = [&]() { - return SourceSize && DestinationSize - ? std::optional<int>{llvm::APSInt::compareValues( - *SourceSize, *DestinationSize)} - : std::nullopt; - }; auto GetFunctionName = [&]() { std::string FunctionNameStr = @@ -1275,20 +1277,18 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BIstpcpy: case Builtin::BI__builtin_strcpy: case Builtin::BIstrcpy: { - DiagID = diag::warn_fortify_strlen_overflow; - SourceSize = ComputeStrLenArgument(1); + DiagOverflowID = diag::warn_fortify_strlen_overflow; + MinOperationSize = ComputeStrLenArgument(1); DestinationSize = ComputeSizeArgument(0); - IsTriggered = CompareSizeSourceToDest() > 0; break; } case Builtin::BI__builtin___stpcpy_chk: case Builtin::BI__builtin___strcpy_chk: { - DiagID = diag::warn_fortify_strlen_overflow; - SourceSize = ComputeStrLenArgument(1); + DiagOverflowID = diag::warn_fortify_strlen_overflow; + MinOperationSize = ComputeStrLenArgument(1); DestinationSize = ComputeExplicitObjectSizeArgument(2); IsChkVariant = true; - IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1312,12 +1312,12 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, auto Diagnose = [&](unsigned ArgIndex, unsigned DestSize, unsigned SourceSize) { - DiagID = diag::warn_fortify_scanf_overflow; unsigned Index = ArgIndex + DataIndex; std::string FunctionName = GetFunctionName(); DiagRuntimeBehavior(TheCall->getArg(Index)->getBeginLoc(), TheCall, - PDiag(DiagID) << FunctionName << (Index + 1) - << DestSize << SourceSize); + PDiag(diag::warn_fortify_scanf_overflow) + << FunctionName << (Index + 1) << DestSize + << SourceSize); }; auto ShiftedComputeSizeArgument = [&](unsigned Index) { @@ -1348,18 +1348,17 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, if (!analyze_format_string::ParsePrintfString( H, FormatBytes, FormatBytes + StrLen, getLangOpts(), Context.getTargetInfo(), false)) { - DiagID = H.isKernelCompatible() - ? diag::warn_format_overflow - : diag::warn_format_overflow_non_kprintf; - SourceSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound()) - .extOrTrunc(SizeTypeWidth); + DiagOverflowID = H.isKernelCompatible() + ? diag::warn_format_overflow + : diag::warn_format_overflow_non_kprintf; + MinOperationSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound()) + .extOrTrunc(SizeTypeWidth); if (BuiltinID == Builtin::BI__builtin___sprintf_chk) { DestinationSize = ComputeExplicitObjectSizeArgument(2); IsChkVariant = true; } else { DestinationSize = ComputeSizeArgument(0); } - IsTriggered = CompareSizeSourceToDest() > 0; break; } } @@ -1376,22 +1375,21 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin___stpncpy_chk: case Builtin::BI__builtin___memccpy_chk: case Builtin::BI__builtin___mempcpy_chk: { - DiagID = diag::warn_builtin_chk_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); + DiagOverflowID = diag::warn_builtin_chk_overflow; + MinOperationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); DestinationSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); IsChkVariant = true; - IsTriggered = CompareSizeSourceToDest() > 0; break; } case Builtin::BI__builtin___snprintf_chk: case Builtin::BI__builtin___vsnprintf_chk: { - DiagID = diag::warn_builtin_chk_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(1); + DiagOverflowID = diag::warn_builtin_chk_overflow; + MinOperationSize = ComputeExplicitObjectSizeArgument(1); DestinationSize = ComputeExplicitObjectSizeArgument(3); IsChkVariant = true; - IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1406,10 +1404,17 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, // diagnostic isn't quite right. We should still diagnose passing a buffer // size larger than the destination buffer though; this is a runtime abort // in _FORTIFY_SOURCE mode, and is quite suspicious otherwise. - DiagID = diag::warn_fortify_source_size_mismatch; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + MaxOperationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + DestinationSize = ComputeSizeArgument(0); + break; + } + + case Builtin::BImemset: + case Builtin::BI__builtin_memset: { + MinOperationSize = MaxOperationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = ComputeSizeArgument(0); - IsTriggered = CompareSizeSourceToDest() > 0; break; } @@ -1417,14 +1422,12 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_memcpy: case Builtin::BImemmove: case Builtin::BI__builtin_memmove: - case Builtin::BImemset: - case Builtin::BI__builtin_memset: case Builtin::BImempcpy: case Builtin::BI__builtin_mempcpy: { - DiagID = diag::warn_fortify_source_overflow; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); + MinOperationSize = MaxOperationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = ComputeSizeArgument(0); - IsTriggered = CompareSizeSourceToDest() > 0; + SourceSize = ComputeSizeArgument(1); break; } @@ -1436,10 +1439,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_readlinkat: case Builtin::BIgetcwd: case Builtin::BI__builtin_getcwd: { - DiagID = diag::warn_fortify_source_size_mismatch; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); DestinationSize = ComputeSizeArgument(TheCall->getNumArgs() - 2); - IsTriggered = CompareSizeSourceToDest() > 0; + MaxOperationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); break; } @@ -1447,20 +1449,17 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_pread: case Builtin::BIpread64: case Builtin::BI__builtin_pread64: { - DiagID = diag::warn_fortify_source_size_mismatch; - SourceSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); DestinationSize = ComputeSizeArgument(TheCall->getNumArgs() - 3); - IsTriggered = CompareSizeSourceToDest() > 0; + MaxOperationSize = + ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); break; } case Builtin::BIwrite: case Builtin::BI__builtin_write: { - DiagID = diag::warn_fortify_destination_size_mismatch; SourceSize = ComputeSizeArgument(TheCall->getNumArgs() - 2); - DestinationSize = + MaxOperationSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 1); - IsTriggered = CompareSizeSourceToDest() < 0; break; } @@ -1468,11 +1467,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_pwrite: case Builtin::BIpwrite64: case Builtin::BI__builtin_pwrite64: { - DiagID = diag::warn_fortify_destination_size_mismatch; SourceSize = ComputeSizeArgument(TheCall->getNumArgs() - 3); - DestinationSize = + MaxOperationSize = ComputeExplicitObjectSizeArgument(TheCall->getNumArgs() - 2); - IsTriggered = CompareSizeSourceToDest() < 0; break; } @@ -1480,12 +1477,11 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, case Builtin::BI__builtin_snprintf: case Builtin::BIvsnprintf: case Builtin::BI__builtin_vsnprintf: { - DiagID = diag::warn_fortify_source_size_mismatch; - SourceSize = ComputeExplicitObjectSizeArgument(1); + MaxOperationSize = ComputeExplicitObjectSizeArgument(1); const auto *FormatExpr = TheCall->getArg(2)->IgnoreParenImpCasts(); StringRef FormatStrRef; size_t StrLen; - if (SourceSize && + if (MaxOperationSize && ProcessFormatStringLiteral(FormatExpr, FormatStrRef, StrLen, Context)) { EstimateSizeFormatHandler H(FormatStrRef); const char *FormatBytes = FormatStrRef.data(); @@ -1495,13 +1491,13 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, llvm::APSInt FormatSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound()) .extOrTrunc(SizeTypeWidth); - if (FormatSize > *SourceSize && *SourceSize != 0) { + if (FormatSize > *MaxOperationSize && *MaxOperationSize != 0) { unsigned TruncationDiagID = H.isKernelCompatible() ? diag::warn_format_truncation : diag::warn_format_truncation_non_kprintf; SmallString<16> SpecifiedSizeStr; SmallString<16> FormatSizeStr; - SourceSize->toString(SpecifiedSizeStr, /*Radix=*/10); + MaxOperationSize->toString(SpecifiedSizeStr, /*Radix=*/10); FormatSize.toString(FormatSizeStr, /*Radix=*/10); DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, PDiag(TruncationDiagID) @@ -1511,23 +1507,57 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, } } DestinationSize = ComputeSizeArgument(0); - IsTriggered = CompareSizeSourceToDest() > 0; break; } } - if (!IsTriggered) - return; - std::string FunctionName = GetFunctionName(); + SmallString<16> MaxOpStr; + SmallString<16> MinOpStr; + + if (MinOperationSize) + MinOperationSize->toString(MinOpStr, /*Radix=*/10); + if (MaxOperationSize) + MaxOperationSize->toString(MaxOpStr, /*Radix=*/10); + + if (DestinationSize) { + SmallString<16> DestinationStr; + DestinationSize->toString(DestinationStr, /*Radix=*/10); + // Check for definate overflow + if (MinOperationSize && + llvm::APSInt::compareValues(*MinOperationSize, *DestinationSize) > 0) { + DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + PDiag(DiagOverflowID) + << FunctionName << DestinationStr << MinOpStr); + } + // Check for possible overflow + else if (MaxOperationSize && llvm::APSInt::compareValues( + *MaxOperationSize, *DestinationSize) > 0) { + DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + PDiag(DiagMayOverflowID) + << FunctionName << DestinationStr << MaxOpStr); + } + } + + if (SourceSize) { + SmallString<16> SourceStr; + SourceSize->toString(SourceStr, /*Radix=*/10); + // Check for definate over-read + if (MinOperationSize && + llvm::APSInt::compareValues(*MinOperationSize, *SourceSize) > 0) { + DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + PDiag(DiagOverReadID) + << FunctionName << SourceStr << MinOpStr); - SmallString<16> DestinationStr; - SmallString<16> SourceStr; - DestinationSize->toString(DestinationStr, /*Radix=*/10); - SourceSize->toString(SourceStr, /*Radix=*/10); - DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, - PDiag(DiagID) - << FunctionName << DestinationStr << SourceStr); + } + // Check for possible over-read + else if (MaxOperationSize && + llvm::APSInt::compareValues(*MaxOperationSize, *SourceSize) > 0) { + DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall, + PDiag(DiagMayOverReadID) + << FunctionName << SourceStr << MaxOpStr); + } + } } static bool BuiltinSEHScopeCheck(Sema &SemaRef, CallExpr *TheCall, diff --git a/clang/test/Sema/builtin-memcpy.c b/clang/test/Sema/builtin-memcpy.c index 2a55e78034a02..af7f2034f3c30 100644 --- a/clang/test/Sema/builtin-memcpy.c +++ b/clang/test/Sema/builtin-memcpy.c @@ -7,7 +7,8 @@ /// Zero-sized structs should not crash. int b() { struct { } a[10]; - __builtin_memcpy(&a[2], a, 2); // c-warning {{buffer has size 0, but size argument is 2}} + __builtin_memcpy(&a[2], a, 2); // c-warning {{buffer has size 0, but size argument is 2}} \ + // c-warning {{buffer has size 0, but size argument is 2}} return 0; } diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c index 5d4a8d81b0b3e..62068361b92f9 100644 --- a/clang/test/Sema/warn-fortify-source.c +++ b/clang/test/Sema/warn-fortify-source.c @@ -30,6 +30,8 @@ void call_memcpy(void) { char dst[10]; char src[20]; memcpy(dst, src, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 10, but size argument is 20}} + memcpy(dst, src, 21); // expected-warning {{memcpy' will always overflow; destination buffer has size 10, but size argument is 21}} \ + // expected-warning {{memcpy' will always over-read; source buffer has size 20, but size argument is 21}} if (sizeof(dst) == sizeof(src)) memcpy(dst, src, 20); // no warning, unreachable @@ -43,18 +45,29 @@ void call_memcpy_type(void) { struct pair p; char buf[20]; memcpy(&p.first, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}} + memcpy(&p.first, buf, 21); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 21}} \ + // expected-warning {{memcpy' will always over-read; source buffer has size 20, but size argument is 21}} +} + +void call_memcpy_chk(void) { + char dst[10]; + char src[10]; + __builtin___memcpy_chk(dst, src, 10, 10); + __builtin___memcpy_chk(dst, src, 10, 9); // expected-warning {{memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} } void call_strncat(void) { char s1[10], s2[20]; __builtin_strncat(s2, s1, 20); __builtin_strncat(s1, s2, 20); // expected-warning {{'strncat' size argument is too large; destination buffer has size 10, but size argument is 20}} + __builtin_strncat(s1, "abcd", 20); // expected-warning {{'strncat' size argument is too large; destination buffer has size 10, but size argument is 20}} } void call_strncpy(void) { char s1[10], s2[20]; __builtin_strncpy(s2, s1, 20); __builtin_strncpy(s1, s2, 20); // expected-warning {{'strncpy' size argument is too large; destination buffer has size 10, but size argument is 20}} + __builtin_strncpy(s1, "abcd", 20); // expected-warning {{'strncpy' size argument is too large; destination buffer has size 10, but size argument is 20}} } void call_stpncpy(void) { @@ -84,9 +97,17 @@ void call_stpcpy(void) { __builtin_stpcpy(dst2, src); // expected-warning {{'stpcpy' will always overflow; destination buffer has size 4, but the source string has length 5 (including NUL byte)}} } +void call_stpcpy_chk(void) { + const char *const src = "abcd"; + char dst1[5]; + __builtin___stpcpy_chk(dst1, src, 5); + __builtin___stpcpy_chk(dst1, src, 4); // expected-warning {{'stpcpy' will always overflow; destination buffer has size 4, but the source string has length 5 (including NUL byte)}} +} + void call_memmove(void) { char s1[10], s2[20]; - __builtin_memmove(s2, s1, 20); + __builtin_memmove(s2, s1, 10); + __builtin_memmove(s2, s1, 20); // expected-warning {{'memmove' will always over-read; source buffer has size 10, but size argument is 20}} __builtin_memmove(s1, s2, 20); // expected-warning {{'memmove' will always overflow; destination buffer has size 10, but size argument is 20}} } @@ -290,11 +311,23 @@ template <int A, int B> void call_memcpy_dep() { char bufferA[A]; char bufferB[B]; - memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} + if (sizeof(bufferA) < 10 && sizeof(bufferB) < 10) { + memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} \ + // expected-warning{{'memcpy' will always over-read; source buffer has size 9, but size argument is 10}} + } else if (sizeof(bufferA) < 10) { + memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}} + } else if (sizeof(bufferB) < 10) { + memcpy(bufferA, bufferB, 10); // expected-warning{{'memcpy' will always over-read; source buffer has size 9, but size argument is 10}} + } else { + memcpy(bufferA, bufferB, 10); + } + } void call_call_memcpy() { - call_memcpy_dep<10, 9>(); + call_memcpy_dep<10, 10>(); + call_memcpy_dep<10, 9>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<10, 9>' requested here}} call_memcpy_dep<9, 10>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<9, 10>' requested here}} + call_memcpy_dep<9, 9>(); // expected-note {{in instantiation of function template specialization 'call_memcpy_dep<9, 9>' requested here}} } #endif >From fcd697de00bb6bd0db9da2645c470b09c67c702f Mon Sep 17 00:00:00 2001 From: Colin Kinloch <[email protected]> Date: Mon, 3 Nov 2025 20:46:15 +0000 Subject: [PATCH 3/3] [clang][Sema] Avoid builtin redecleration for asm-label test As `readlink` is now defined as bultin it throws an error when redeclared with a different type: "incompatible redeclaration of library function function 'readlink'" As `sync` has the type `void(void)` it this issue should not happen in the future. --- clang/test/Sema/asm-label.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/Sema/asm-label.c b/clang/test/Sema/asm-label.c index eb0259863b2b2..a8b6afc252c2d 100644 --- a/clang/test/Sema/asm-label.c +++ b/clang/test/Sema/asm-label.c @@ -25,6 +25,6 @@ int z __asm__("zooms"); // expected-error{{conflicting asm label}} // No diagnostics on the following. -void __real_readlink(void) __asm("readlink"); -void readlink(void) __asm("__protected_readlink"); -void readlink(void) { __real_readlink(); } +void __real_sync(void) __asm("sync"); +void sync(void) __asm("__protected_sync"); +void sync(void) { __real_sync(); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
