Author: BgZun Date: 2026-02-21T05:04:23Z New Revision: 17ad5559c215201b9bdb7090b10c4b213861ce93
URL: https://github.com/llvm/llvm-project/commit/17ad5559c215201b9bdb7090b10c4b213861ce93 DIFF: https://github.com/llvm/llvm-project/commit/17ad5559c215201b9bdb7090b10c4b213861ce93.diff LOG: [Clang] Added clang diagnostic when snprintf/vsnprintf uses sizeof(dest) for the len parameter Closes: [#162366](https://github.com/llvm/llvm-project/issues/162366) --------- Co-authored-by: Bogdan Zunic <[email protected]> Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaChecking.cpp clang/test/SemaCXX/warn-memset-bad-sizeof.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b1d7c77e65958..0bbe75b215cad 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -258,6 +258,9 @@ Improvements to Clang's diagnostics - The ``-Wloop-analysis`` warning has been extended to catch more cases of variable modification inside lambda expressions (#GH132038). +- Clang now emits ``-Wsizeof-pointer-memaccess`` when snprintf/vsnprintf use the sizeof + the destination buffer(dynamically allocated) in the len parameter(#GH162366) + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 386cc49024311..7fe4a386c7e04 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3091,6 +3091,8 @@ class Sema final : public SemaBase { void CheckMemaccessArguments(const CallExpr *Call, unsigned BId, IdentifierInfo *FnName); + bool CheckSizeofMemaccessArgument(const Expr *SizeOfArg, const Expr *Dest, + IdentifierInfo *FnName); // Warn if the user has made the 'size' argument to strlcpy or strlcat // be the size of the source, instead of the destination. void CheckStrlcpycatArguments(const CallExpr *Call, IdentifierInfo *FnName); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 464ec377bf1a8..0ea41ff1f613e 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1453,6 +1453,10 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, } } DestinationSize = ComputeSizeArgument(0); + const Expr *LenArg = TheCall->getArg(1)->IgnoreCasts(); + const Expr *Dest = TheCall->getArg(0)->IgnoreCasts(); + IdentifierInfo *FnInfo = FD->getIdentifier(); + CheckSizeofMemaccessArgument(LenArg, Dest, FnInfo); } } @@ -10568,8 +10572,6 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call, // We have special checking when the length is a sizeof expression. QualType SizeOfArgTy = getSizeOfArgType(LenExpr); - const Expr *SizeOfArg = getSizeOfExprArg(LenExpr); - llvm::FoldingSetNodeID SizeOfArgID; // Although widely used, 'bzero' is not a standard function. Be more strict // with the argument types before allowing diagnostics and only allow the @@ -10596,60 +10598,8 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call, // actually comparing the expressions for equality. Because computing the // expression IDs can be expensive, we only do this if the diagnostic is // enabled. - if (SizeOfArg && - !Diags.isIgnored(diag::warn_sizeof_pointer_expr_memaccess, - SizeOfArg->getExprLoc())) { - // We only compute IDs for expressions if the warning is enabled, and - // cache the sizeof arg's ID. - if (SizeOfArgID == llvm::FoldingSetNodeID()) - SizeOfArg->Profile(SizeOfArgID, Context, true); - llvm::FoldingSetNodeID DestID; - Dest->Profile(DestID, Context, true); - if (DestID == SizeOfArgID) { - // TODO: For strncpy() and friends, this could suggest sizeof(dst) - // over sizeof(src) as well. - unsigned ActionIdx = 0; // Default is to suggest dereferencing. - StringRef ReadableName = FnName->getName(); - - if (const UnaryOperator *UnaryOp = dyn_cast<UnaryOperator>(Dest)) - if (UnaryOp->getOpcode() == UO_AddrOf) - ActionIdx = 1; // If its an address-of operator, just remove it. - if (!PointeeTy->isIncompleteType() && - (Context.getTypeSize(PointeeTy) == Context.getCharWidth())) - ActionIdx = 2; // If the pointee's size is sizeof(char), - // suggest an explicit length. - - // If the function is defined as a builtin macro, do not show macro - // expansion. - SourceLocation SL = SizeOfArg->getExprLoc(); - SourceRange DSR = Dest->getSourceRange(); - SourceRange SSR = SizeOfArg->getSourceRange(); - SourceManager &SM = getSourceManager(); - - if (SM.isMacroArgExpansion(SL)) { - ReadableName = Lexer::getImmediateMacroName(SL, SM, LangOpts); - SL = SM.getSpellingLoc(SL); - DSR = SourceRange(SM.getSpellingLoc(DSR.getBegin()), - SM.getSpellingLoc(DSR.getEnd())); - SSR = SourceRange(SM.getSpellingLoc(SSR.getBegin()), - SM.getSpellingLoc(SSR.getEnd())); - } - - DiagRuntimeBehavior(SL, SizeOfArg, - PDiag(diag::warn_sizeof_pointer_expr_memaccess) - << ReadableName - << PointeeTy - << DestTy - << DSR - << SSR); - DiagRuntimeBehavior(SL, SizeOfArg, - PDiag(diag::warn_sizeof_pointer_expr_memaccess_note) - << ActionIdx - << SSR); - - break; - } - } + if (CheckSizeofMemaccessArgument(LenExpr, Dest, FnName)) + break; // Also check for cases where the sizeof argument is the exact same // type as the memory argument, and where it points to a user-defined @@ -10752,6 +10702,71 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call, } } +bool Sema::CheckSizeofMemaccessArgument(const Expr *LenExpr, const Expr *Dest, + IdentifierInfo *FnName) { + llvm::FoldingSetNodeID SizeOfArgID; + const Expr *SizeOfArg = getSizeOfExprArg(LenExpr); + if (!SizeOfArg) + return false; + // Computing this warning is expensive, so we only do so if the warning is + // enabled. + if (Diags.isIgnored(diag::warn_sizeof_pointer_expr_memaccess, + SizeOfArg->getExprLoc())) + return false; + QualType DestTy = Dest->getType(); + const PointerType *DestPtrTy = DestTy->getAs<PointerType>(); + if (!DestPtrTy) + return false; + + QualType PointeeTy = DestPtrTy->getPointeeType(); + + if (SizeOfArgID == llvm::FoldingSetNodeID()) + SizeOfArg->Profile(SizeOfArgID, Context, true); + + llvm::FoldingSetNodeID DestID; + Dest->Profile(DestID, Context, true); + if (DestID == SizeOfArgID) { + // TODO: For strncpy() and friends, this could suggest sizeof(dst) + // over sizeof(src) as well. + unsigned ActionIdx = 0; // Default is to suggest dereferencing. + StringRef ReadableName = FnName->getName(); + + if (const UnaryOperator *UnaryOp = dyn_cast<UnaryOperator>(Dest); + UnaryOp && UnaryOp->getOpcode() == UO_AddrOf) + ActionIdx = 1; // If its an address-of operator, just remove it. + if (!PointeeTy->isIncompleteType() && + (Context.getTypeSize(PointeeTy) == Context.getCharWidth())) + ActionIdx = 2; // If the pointee's size is sizeof(char), + // suggest an explicit length. + + // If the function is defined as a builtin macro, do not show macro + // expansion. + SourceLocation SL = SizeOfArg->getExprLoc(); + SourceRange DSR = Dest->getSourceRange(); + SourceRange SSR = SizeOfArg->getSourceRange(); + SourceManager &SM = getSourceManager(); + + if (SM.isMacroArgExpansion(SL)) { + ReadableName = Lexer::getImmediateMacroName(SL, SM, LangOpts); + SL = SM.getSpellingLoc(SL); + DSR = SourceRange(SM.getSpellingLoc(DSR.getBegin()), + SM.getSpellingLoc(DSR.getEnd())); + SSR = SourceRange(SM.getSpellingLoc(SSR.getBegin()), + SM.getSpellingLoc(SSR.getEnd())); + } + + DiagRuntimeBehavior(SL, SizeOfArg, + PDiag(diag::warn_sizeof_pointer_expr_memaccess) + << ReadableName << PointeeTy << DestTy << DSR + << SSR); + DiagRuntimeBehavior(SL, SizeOfArg, + PDiag(diag::warn_sizeof_pointer_expr_memaccess_note) + << ActionIdx << SSR); + return true; + } + return false; +} + // A little helper routine: ignore addition and subtraction of integer literals. // This intentionally does not ignore all integer constant expressions because // we don't want to remove sizeof(). diff --git a/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp b/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp index 0a78caa924ea9..6f1cd4dd639ec 100644 --- a/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp +++ b/clang/test/SemaCXX/warn-memset-bad-sizeof.cpp @@ -188,3 +188,119 @@ void strcpy_and_friends() { strndup(FOO, sizeof(FOO)); // \ // expected-warning {{'strndup' call operates on objects of type 'const char' while the size is based on a diff erent type 'const char *'}} expected-note{{did you mean to provide an explicit length?}} } + +extern "C" int snprintf(char* buffer, __SIZE_TYPE__ buf_size, const char* format, ...); +extern "C" int vsnprintf(char* buffer, __SIZE_TYPE__ buf_size, const char* format, __builtin_va_list arg); +extern "C" void* malloc(unsigned size); + +// This is a dependent context by none of the actual operations are dependent. +template <class T> void fooNone() { + char* a = (char*) malloc(20); + const char* b = "Hello World"; + snprintf(a, sizeof(a), "%s", b); // #fooNone_diagnostic + // expected-warning@#fooNone_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooNone_diagnostic {{did you mean to provide an explicit length?}} +} + +template <class T> void fooType() { + T a = (T) malloc(20); // #fooType_error + const char* b = "Hello World"; + snprintf((char *)a, sizeof(a), "%s", b);// #fooType_diagnostic +} + +template <class T> void fooTypePtr() { + T *a = (T *) malloc(20); + const char* b = "Hello World"; + snprintf((char *)a, sizeof(a), "%s", b);// #fooTypePtr_diagnostic +} + +void check_prints(){ + char* a = (char*) malloc(20); + const char* b = "Hello World"; + snprintf(a, sizeof(a), "%s", b); // #CheckBasePrint + // expected-warning@#CheckBasePrint {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#CheckBasePrint {{did you mean to provide an explicit length?}} + + snprintf((char*)a, sizeof(a), "%s", b); // #CheckCastPrint + // expected-warning@#CheckCastPrint {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#CheckCastPrint {{did you mean to provide an explicit length?}} + + snprintf((char*)(char*)a, sizeof(a), "%s", b); // #CheckDoubleCastPrint + // expected-warning@#CheckDoubleCastPrint {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#CheckDoubleCastPrint {{did you mean to provide an explicit length?}} + + __builtin_va_list list; + vsnprintf(a, sizeof(a), "%s", list); // #VSNprintCheck + // expected-warning@#VSNprintCheck {{'vsnprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#VSNprintCheck {{did you mean to provide an explicit length?}} + + vsnprintf((char*)a, sizeof(a), "%s", list); // #VSNprintCastCheck + // expected-warning@#VSNprintCastCheck {{'vsnprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#VSNprintCastCheck {{did you mean to provide an explicit length?}} + + vsnprintf((char*)(char*)a, sizeof(a), "%s", list); // #VSNprintDoubleCastCheck + // expected-warning@#VSNprintDoubleCastCheck {{'vsnprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#VSNprintDoubleCastCheck {{did you mean to provide an explicit length?}} + + //No diagnostic output when dest is an array + char c[20]; + const char* d = "Hello World"; + snprintf(c, sizeof(c), "%s", d); // expected-none + + //No diagnostic output when len is a exact number + char* e = (char*) malloc(20); + const char* f = "Hello World"; + snprintf(e, 20, "%s", f); // expected-none + + //Template tests + fooNone<int>(); // #fooNone_int_call + // expected-warning@#fooNone_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooNone_diagnostic {{did you mean to provide an explicit length?}} + // expected-note@#fooNone_int_call {{in instantiation of function template specialization 'fooNone<int>' requested here}} + + fooNone<char>();// #fooNone_char_call + // expected-warning@#fooNone_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooNone_diagnostic {{did you mean to provide an explicit length?}} + // expected-note@#fooNone_char_call {{in instantiation of function template specialization 'fooNone<char>' requested here}} + + fooNone<char*>();// #fooNone_charptr_call + // expected-warning@#fooNone_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooNone_diagnostic {{did you mean to provide an explicit length?}} + // expected-note@#fooNone_charptr_call {{in instantiation of function template specialization 'fooNone<char *>' requested here}} + + + fooNone<void>();// #fooNone_void_call + // expected-warning@#fooNone_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooNone_diagnostic {{did you mean to provide an explicit length?}} + // expected-note@#fooNone_void_call {{in instantiation of function template specialization 'fooNone<void>' requested here}} + + fooType<char*>();// #fooType_charptr_call + // expected-warning@#fooType_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooType_diagnostic {{did you mean to provide an explicit length?}} + // expected-note@#fooType_charptr_call {{in instantiation of function template specialization 'fooType<char *>' requested here}} + + fooType<void*>();// #fooType_voidptr_call + // expected-warning@#fooType_diagnostic {{'snprintf' call operates on objects of type 'void' while the size is based on a diff erent type 'void *'}} + // expected-note@#fooType_diagnostic {{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}} + // expected-note@#fooType_voidptr_call {{in instantiation of function template specialization 'fooType<void *>' requested here}} + + fooType<char>(); // #fooType_char_call + // expected-error@#fooType_error {{cast from pointer to smaller type 'char' loses information}} + // expected-note@#fooType_char_call {{in instantiation of function template specialization 'fooType<char>' requested here}} + + fooTypePtr<char>(); // #fooTypePtr_char_call + // expected-warning@#fooTypePtr_diagnostic {{'snprintf' call operates on objects of type 'char' while the size is based on a diff erent type 'char *'}} + // expected-note@#fooTypePtr_diagnostic {{did you mean to provide an explicit length?}} + // expected-note@#fooTypePtr_char_call {{in instantiation of function template specialization 'fooTypePtr<char>' requested here}} + + fooTypePtr<char*>(); // #fooTypePtr_charptr_call + // expected-warning@#fooTypePtr_diagnostic {{'snprintf' call operates on objects of type 'char *' while the size is based on a diff erent type 'char **'}} + // expected-note@#fooTypePtr_diagnostic {{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}} + // expected-note@#fooTypePtr_charptr_call {{in instantiation of function template specialization 'fooTypePtr<char *>' requested here}} + + fooTypePtr<void*>(); // #fooTypePtr_void_call + // expected-warning@#fooTypePtr_diagnostic {{'snprintf' call operates on objects of type 'void *' while the size is based on a diff erent type 'void **'}} + // expected-note@#fooTypePtr_diagnostic {{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}} + // expected-note@#fooTypePtr_void_call {{in instantiation of function template specialization 'fooTypePtr<void *>' requested here}} + +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
