llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Pppp1116 (Pppp1116) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/190447.diff 3 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4) - (modified) clang/lib/Sema/SemaChecking.cpp (+101) - (added) clang/test/Sema/warn-fortify-missing-null-terminator-space.c (+19) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index eddf9c50033e1..adab998cd7d7f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -965,6 +965,10 @@ def warn_fortify_strlen_overflow: Warning< " but the source string has length %2 (including NUL byte)">, InGroup<FortifySource>; +def warn_fortify_missing_null_terminator_space : Warning< + "%select{allocation size|copy size}0 does not include space for null " + "terminator; consider '%1 + 1'">, InGroup<FortifySource>; + def subst_format_overflow : TextSubstitution< "'%0' will always overflow; destination buffer has size %1," " but format string expands to at least %2">; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index de8b965144971..9a207a407e28c 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1141,6 +1141,106 @@ static bool ProcessFormatStringLiteral(const Expr *FormatExpr, return false; } +static unsigned getDiagnosedBuiltinID(const FunctionDecl *FD) { + if (!FD) + return 0; + + if (const auto *DABAttr = FD->getAttr<DiagnoseAsBuiltinAttr>()) + FD = DABAttr->getFunction(); + + return FD ? FD->getBuiltinID(/*ConsiderWrappers=*/true) : 0; +} + +static const Expr *getDirectStrlenCallArgument(const Expr *E) { + const auto *CE = dyn_cast<CallExpr>(E->IgnoreParenCasts()); + if (!CE || CE->getNumArgs() != 1) + return nullptr; + + switch (getDiagnosedBuiltinID(CE->getDirectCallee())) { + case Builtin::BIstrlen: + case Builtin::BI__builtin_strlen: + return CE->getArg(0)->IgnoreParenCasts(); + default: + return nullptr; + } +} + +static bool areSameSimpleDeclRef(const Expr *E1, const Expr *E2) { + const auto *D1 = dyn_cast<DeclRefExpr>(E1->IgnoreParenCasts()); + const auto *D2 = dyn_cast<DeclRefExpr>(E2->IgnoreParenCasts()); + return D1 && D2 && D1->getDecl() == D2->getDecl(); +} + +static std::string getExprAsString(const Expr *E, + const PrintingPolicy &Policy) { + SmallString<64> Text; + llvm::raw_svector_ostream OS(Text); + E->printPretty(OS, nullptr, Policy); + return std::string(OS.str()); +} + +static FixItHint getPlusOneFixIt(const Expr *E, const PrintingPolicy &Policy) { + const Expr *SimpleExpr = E->IgnoreParenCasts(); + if (!isa<DeclRefExpr, CallExpr>(SimpleExpr)) + return {}; + + SourceRange Range = E->getSourceRange(); + if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID()) + return {}; + + SmallString<64> Replacement; + llvm::raw_svector_ostream OS(Replacement); + E->printPretty(OS, nullptr, Policy); + OS << " + 1"; + return FixItHint::CreateReplacement(Range, OS.str()); +} + +static void DiagnoseMissingNullTerminatorSpace(Sema &S, SourceLocation Loc, + const Expr *SizeExpr, + unsigned MissingKind) { + std::string SizeText = getExprAsString(SizeExpr, S.getPrintingPolicy()); + S.Diag(Loc, diag::warn_fortify_missing_null_terminator_space) + << MissingKind << SizeText + << getPlusOneFixIt(SizeExpr, S.getPrintingPolicy()); +} + +static void CheckCStringNullTerminatorSizeArguments(Sema &S, + const FunctionDecl *FD, + const CallExpr *TheCall) { + if (TheCall->isValueDependent() || TheCall->isTypeDependent() || + S.isConstantEvaluatedContext() || + S.Diags.isIgnored(diag::warn_fortify_missing_null_terminator_space, + TheCall->getBeginLoc())) + return; + + switch (getDiagnosedBuiltinID(FD)) { + case Builtin::BImalloc: + case Builtin::BI__builtin_malloc: { + if (TheCall->getNumArgs() != 1) + return; + const Expr *SizeArg = TheCall->getArg(0)->IgnoreParenCasts(); + if (getDirectStrlenCallArgument(SizeArg)) + DiagnoseMissingNullTerminatorSpace(S, TheCall->getBeginLoc(), SizeArg, + /*MissingKind=*/0); + return; + } + case Builtin::BImemcpy: + case Builtin::BI__builtin_memcpy: + if (TheCall->getNumArgs() != 3) + return; + break; + default: + return; + } + + const Expr *SrcArg = TheCall->getArg(1)->IgnoreParenCasts(); + const Expr *SizeArg = TheCall->getArg(2)->IgnoreParenCasts(); + const Expr *StrlenArg = getDirectStrlenCallArgument(SizeArg); + if (StrlenArg && areSameSimpleDeclRef(StrlenArg, SrcArg)) + DiagnoseMissingNullTerminatorSpace(S, TheCall->getBeginLoc(), SizeArg, + /*MissingKind=*/1); +} + void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall) { if (TheCall->isValueDependent() || TheCall->isTypeDependent() || @@ -4449,6 +4549,7 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall, CheckAbsoluteValueFunction(TheCall, FDecl); CheckMaxUnsignedZero(TheCall, FDecl); CheckInfNaNFunction(TheCall, FDecl); + CheckCStringNullTerminatorSizeArguments(*this, FDecl, TheCall); if (getLangOpts().ObjC) ObjC().DiagnoseCStringFormatDirectiveInCFAPI(FDecl, Args, NumArgs); diff --git a/clang/test/Sema/warn-fortify-missing-null-terminator-space.c b/clang/test/Sema/warn-fortify-missing-null-terminator-space.c new file mode 100644 index 0000000000000..2a79de4d93a7b --- /dev/null +++ b/clang/test/Sema/warn-fortify-missing-null-terminator-space.c @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -fsyntax-only -Wfortify-source -verify %s + +typedef __SIZE_TYPE__ size_t; + +void *malloc(size_t); +void *memcpy(void *, const void *, size_t); +size_t strlen(const char *); + +void direct_malloc_strlen(const char *src) { + char *p = malloc(strlen(src)); // expected-warning{{allocation size does not include space for null terminator; consider 'strlen(src) + 1'}} + char *ok = malloc(strlen(src) + 1); + (void)p; + (void)ok; +} + +void direct_memcpy_strlen(char *dst, const char *src) { + memcpy(dst, src, strlen(src)); // expected-warning{{copy size does not include space for null terminator; consider 'strlen(src) + 1'}} + memcpy(dst, src, strlen(src) + 1); +} `````````` </details> https://github.com/llvm/llvm-project/pull/190447 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
