Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -32,6 +32,7 @@ #include "LimitedRandomnessCheck.h" #include "MutatingCopyCheck.h" #include "NonTrivialTypesLibcMemoryCallsCheck.h" +#include "ObsolescentFunctionsCheck.h" #include "PostfixOperatorCheck.h" #include "ProperlySeededRandomGeneratorCheck.h" #include "SetLongJmpCheck.h" @@ -301,6 +302,7 @@ // FIO CheckFactories.registerCheck("cert-fio38-c"); // MSC + CheckFactories.registerCheck("cert-msc24-c"); CheckFactories.registerCheck("cert-msc30-c"); CheckFactories.registerCheck( "cert-msc32-c"); Index: clang-tools-extra/clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -12,6 +12,7 @@ LimitedRandomnessCheck.cpp MutatingCopyCheck.cpp NonTrivialTypesLibcMemoryCallsCheck.cpp + ObsolescentFunctionsCheck.cpp PostfixOperatorCheck.cpp ProperlySeededRandomGeneratorCheck.cpp SetLongJmpCheck.cpp Index: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h @@ -0,0 +1,50 @@ +//===--- ObsolescentFunctionsCheck.h - clang-tidy ---------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_OBSOLESCENTFUNCTIONSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_OBSOLESCENTFUNCTIONSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cert { + +/// Checks for deprecated and obsolescent function listed in +/// CERT C Coding Standard Recommendation MSC24 - C. For the listed functions, +/// an alternative, safe replacement is suggested if available. +/// The checker heavily relies on the availability of annexK(Bounds - checking +/// interfaces) from C11. For the user-facing documentation see: +/// +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-msc24-c.html +class ObsolescentFunctionsCheck : public ClangTidyCheck { +public: + ObsolescentFunctionsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + +private: + bool useSafeFunctionsFromAnnexK() const; + Optional + getReplacementFunctionNameFor(StringRef FunctionName, + bool FunctionHasAnnexKReplacement) const; + + Preprocessor *PP = nullptr; + + // Used for caching the result of useSafeFunctionsFromAnnexK. + mutable llvm::Optional IsAnnexKAvailable; +}; + +} // namespace cert +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_OBSOLESCENTFUNCTIONSCHECK_H Index: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp @@ -0,0 +1,156 @@ +//===--- ObsolescentFunctionsCheck.cpp - clang-tidy -----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ObsolescentFunctionsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include + +using namespace clang::ast_matchers; +using namespace llvm; + +namespace clang { +namespace tidy { +namespace cert { + +static constexpr llvm::StringLiteral DeprecatedFunctionNamesId = + "DeprecatedFunctionNamesId"; +static constexpr llvm::StringLiteral FunctionNamesId = "FunctionNamesId"; +static constexpr llvm::StringLiteral FunctionNamesWithAnnexKReplacementId = + "FunctionNamesWithAnnexKReplacementExpr"; + +static constexpr llvm::StringLiteral DeclRefId = "DeclRefId"; + +static StringRef getRationale(StringRef FunctionName) { + return StringSwitch(FunctionName) + .Cases("asctime", "ctime", "is non-reentrant") + .Cases("fopen", "freopen", "has no exclusive access to file") + .Cases("rewind", "setbuf", "has no error detection") + .Default("is obsolescent"); +} + +void ObsolescentFunctionsCheck::registerMatchers(MatchFinder *Finder) { + + // Matching the `gets` deprecated function without replacement. + auto DeprecatedFunctionNamesMatcher = hasAnyName("::gets"); + + // Matching functions with replacements without Annex K. + auto FunctionNamesMatcher = hasAnyName("::rewind", "::setbuf"); + + // Matching functions with safe replacements in Annex K. + auto FunctionNamesWithAnnexKReplacementMatcher = hasAnyName( + "::asctime", "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen", + "::fscanf", "::fwprintf", "::fwscanf", "::getenv", "::gmtime", + "::localtime", "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove", + "::printf", "::qsort", "::snprintf", "::sprintf", "::sscanf", "::strcat", + "::strcpy", "::strerror", "::strncat", "::strncpy", "::strtok", + "::swprintf", "::swscanf", "::vfprintf", "::vfscanf", "::vfwprintf", + "::vfwscanf", "::vprintf", "::vscanf", "::vsnprintf", "::vsprintf", + "::vsscanf", "::vswprintf", "::vswscanf", "::vwprintf", "::vwscanf", + "::wcrtomb", "::wcscat", "::wcscpy", "::wcsncat", "::wcsncpy", + "::wcsrtombs", "::wcstok", "::wcstombs", "::wctomb", "::wmemcpy", + "::wmemmove", "::wprintf", "::wscanf"); + + Finder->addMatcher( + declRefExpr( + to(anyOf(functionDecl(DeprecatedFunctionNamesMatcher) + .bind(DeprecatedFunctionNamesId), + functionDecl(FunctionNamesMatcher).bind(FunctionNamesId), + functionDecl(FunctionNamesWithAnnexKReplacementMatcher) + .bind(FunctionNamesWithAnnexKReplacementId)))) + .bind(DeclRefId), + this); +} + +void ObsolescentFunctionsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *DeclRef = Result.Nodes.getNodeAs(DeclRefId); + assert(DeclRef && "No matching declaration reference found."); + SourceRange Range = SourceRange(DeclRef->getBeginLoc(), DeclRef->getEndLoc()); + + // FIXME: I'm not sure if this can ever happen, but to be on the safe side, + // validity is checked. + if (Range.isInvalid()) + return; + + if (const auto *Deprecated = + Result.Nodes.getNodeAs(DeprecatedFunctionNamesId)) { + diag(Range.getBegin(), + "function '%0' is deprecated as of C99, removed from C11.") + << Deprecated->getName() << Range; + return; + } + + const auto *Normal = Result.Nodes.getNodeAs(FunctionNamesId); + const auto *AnnexK = Normal ? nullptr + : Result.Nodes.getNodeAs( + FunctionNamesWithAnnexKReplacementId); + assert((Normal && !AnnexK) || (!Normal && AnnexK)); + + StringRef FunctionName = (Normal ? Normal : AnnexK)->getName(); + Optional ReplacementFunctionName = + getReplacementFunctionNameFor(FunctionName, AnnexK); + + if (!ReplacementFunctionName.hasValue()) + return; + + diag(Range.getBegin(), "function '%0' %1; '%2' should be used instead.") + << FunctionName << getRationale(FunctionName) + << ReplacementFunctionName.getValue() << Range; +} + +void ObsolescentFunctionsCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + ObsolescentFunctionsCheck::PP = PP; +} + +bool ObsolescentFunctionsCheck::useSafeFunctionsFromAnnexK() const { + if (IsAnnexKAvailable.hasValue()) + return IsAnnexKAvailable.getValue(); + + assert(PP && "No Preprocessor registered."); + + if (!PP->isMacroDefined("__STDC_LIB_EXT1__") || + !PP->isMacroDefined("__STDC_WANT_LIB_EXT1__")) { + // Caching the result. + return (IsAnnexKAvailable = false).getValue(); + } + + const auto *MI = + PP->getMacroInfo(PP->getIdentifierInfo("__STDC_WANT_LIB_EXT1__")); + if (!MI || MI->tokens_empty()) { + // Caching the result. + return (IsAnnexKAvailable = false).getValue(); + } + + const Token &T = MI->tokens().back(); + if (!T.isLiteral() || !T.getLiteralData()) { + // Caching the result. + return (IsAnnexKAvailable = false).getValue(); + } + + // Caching the result. + IsAnnexKAvailable = StringRef(T.getLiteralData(), T.getLength()) == "1"; + return IsAnnexKAvailable.getValue(); +} + +Optional ObsolescentFunctionsCheck::getReplacementFunctionNameFor( + StringRef FunctionName, bool FunctionHasAnnexKReplacement) const { + if (FunctionHasAnnexKReplacement && useSafeFunctionsFromAnnexK()) + return (Twine{FunctionName} + "_s").str(); + + return StringSwitch>(FunctionName) + .Case("rewind", Optional{"fseek"}) + .Case("setbuf", Optional{"setvbuf"}) + .Default(None); +} + +} // namespace cert +} // namespace tidy +} // namespace clang Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -89,6 +89,14 @@ Finds potentially incorrect calls to ``memcmp()`` based on properties of the arguments. +- New :doc:`cert-msc24-c + ` check. + + Checks for deprecated and obsolescent functions listed in + CERT C Coding Standard Recommendation MSC24-C. For the listed functions, + an alternative, more secure replacement is suggested, if available. The checker heavily + relies on the functions from Annex K (Bounds-checking interfaces) of C11. + - New :doc:`cppcoreguidelines-virtual-class-destructor ` check. Index: clang-tools-extra/docs/clang-tidy/checks/cert-msc24-c.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/cert-msc24-c.rst @@ -0,0 +1,91 @@ +.. title:: clang-tidy - cert-msc24-c + +cert-msc24-c +============ + +Checks for deprecated and obsolescent functions listed in +CERT C Coding Standard Recommendation MSC24-C (`MSC24-C. Do not use deprecated or obsolescent functions +`_.). +For the listed functions, an alternative, more secure replacement is suggested, if available. +The checker heavily relies on the functions from Annex K (Bounds-checking interfaces) of C11. + +For the following functions, replacement is suggested from Annex K: `asctime`, +`ctime`, `fopen`, `freopen`, `bsearch`, `fprintf`, `fscanf`, `fwprintf`, `fwscanf`, +`getenv`, `gmtime`, `localtime`, `mbsrtowcs`, `mbstowcs`, `memcpy`, `memmove`, `printf`, +`qsort`, `snprintf`, `sprintf`, `sscanf`, `strcat`, `strcpy`, `strerror`, +`strncat`, `strncpy`, `strtok`, `swprintf`, `swscanf`, `vfprintf`, `vfscanf`, `vfwprintf`, +`vfwscanf`, `vprintf`, `vscanf`, `vsnprintf`, `vsprintf`, `vsscanf`, `vswprintf`, +`vswscanf`, `vwprintf`, `vwscanf`, `wcrtomb`, `wcscat`, `wcscpy`, `wcsncat`, `wcsncpy`, +`wcsrtombs`, `wcstok`, `wcstombs`, `wctomb`, `wmemcpy`, `wmemmove`, `wprintf`, `wscanf`. +If Annex K is not available, the checker ignores these functions. + +The availability of Annex K is checked based on the following macros. + - `__STDC_LIB_EXT1__`: feature macro, which indicates the presence of + Annex K (Bounds-checking interfaces) in the library implementation + - `__STDC_WANT_LIB_EXT1__`: user defined macro, which indicates if the user wants the functions from + Annex K to be defined. + +Both macros have to be defined to suggest replacement functions from Annex K. `__STDC_LIB_EXT1__` is +defined by the library implementation, and `__STDC_WANT_LIB_EXT1__` must be defined to "1" by the user +before including any system headers. + +Deprecated function `gets` is checked, and a warning is issued if used. + +The following functions are also checked, and an alternative replacement function is suggested: + - `rewind`, suggested replacement: `fseek` + - `setbuf`, suggested replacement: `setvbuf` + +The following functions are covered in the check `cert-err34-c `_, +so this checker **ignores** them: `atof`, `atoi`, `atol`, `atoll`. + +Examples: + +.. code-block:: c++ + + //__STDC_LIB_EXT1__ is defined by the library implementation + #define __STDC_WANT_LIB_EXT1__ 1 + + #include // defines the functions from Annex K + #include + + enum { BUFSIZE = 32 }; + + void fWarning(const char *msg) { + static const char prefix[] = "Error: "; + static const char suffix[] = "\n"; + char buf[BUFSIZE] = {0}; + + strcpy(buf, prefix); // warning: function 'strcpy' is obsolescent; 'strcpy_s' should be used instead. + strcat(buf, msg); // warning: function 'strcat' is obsolescent; 'strcat_s' should be used instead. + strcat(buf, suffix); // warning: function 'strcat' is obsolescent; 'strcat_s' should be used instead. + if (fputs(buf, stderr) < 0) { + // error handling + return; + } + } + + void fUsingSafeFunctions(const char *msg) { + static const char prefix[] = "Error: "; + static const char suffix[] = "\n"; + char buf[BUFSIZE] = {0}; + + if (strcpy_s(buf, prefix) != 0) { + // error handling + return; + } + + if (strcat_s(buf, msg) != 0) { + // error handling + return; + } + + if (strcat_s(buf, suffix) != 0) { + // error handling + return; + } + + if (fputs(buf, stderr) < 0) { + // error handling + return; + } + } Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -123,6 +123,7 @@ `cert-flp30-c `_, `cert-flp37-c `_, `cert-mem57-cpp `_, + `cert-msc24-c `_, `cert-msc50-cpp `_, `cert-msc51-cpp `_, `cert-oop57-cpp `_, Index: clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cert-msc24-c.c @@ -0,0 +1,97 @@ +// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K %s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s cert-msc24-c %t -- -- -U__STDC_LIB_EXT1__ -D__STDC_WANT_LIB_EXT1__=1 + +typedef void *FILE; +char *gets(char *s); +void rewind(FILE *stream); +void setbuf(FILE *stream, char *buf); + +void f1(char *s, FILE *f) { + gets(s); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'gets' is deprecated as of C99, removed from C11. + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'gets' is deprecated as of C99, removed from C11. + + rewind(f); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead. + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead. + + setbuf(f, s); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead. + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead. +} + +struct tm; +char *asctime(const struct tm *timeptr); + +void f2(const struct tm *timeptr) { + asctime(timeptr); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead. + // no-warning WITHOUT-ANNEX-K + + char *(*f_ptr1)(const struct tm *) = asctime; + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:40: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead. + // no-warning WITHOUT-ANNEX-K + + char *(*f_ptr2)(const struct tm *) = &asctime; + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:41: warning: function 'asctime' is non-reentrant; 'asctime_s' should be used instead. + // no-warning WITHOUT-ANNEX-K +} + +FILE *fopen(const char *filename, const char *mode); +FILE *freopen(const char *filename, const char *mode, FILE *stream); +int fscanf(FILE *stream, const char *format, ...); + +void f3(char *s, FILE *f) { + fopen(s, s); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fopen' has no exclusive access to file; 'fopen_s' should be used instead. + // no-warning WITHOUT-ANNEX-K + + freopen(s, s, f); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'freopen' has no exclusive access to file; 'freopen_s' should be used instead. + // no-warning WITHOUT-ANNEX-K + + int i; + fscanf(f, "%d", &i); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fscanf' is obsolescent; 'fscanf_s' should be used instead. + // no-warning WITHOUT-ANNEX-K +} + +typedef int time_t; +char *ctime(const time_t *timer); + +void f4(const time_t *timer) { + ctime(timer); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'ctime' is non-reentrant; 'ctime_s' should be used instead. + // no-warning WITHOUT-ANNEX-K +} + +typedef int errno_t; +typedef __SIZE_TYPE__ size_t; +typedef size_t rsize_t; +errno_t asctime_s(char *s, rsize_t maxsize, const struct tm *timeptr); +errno_t strcat_s(char *s1, rsize_t s1max, const char *s2); +int fseek(FILE *stream, long int offset, int whence); +int setvbuf(FILE *stream, char *buf, int mode, size_t size); + +void fUsingSafeFunctions(const struct tm *timeptr, FILE *f) { + const size_t BUFFSIZE = 32; + char buf[BUFFSIZE] = {0}; + + // no-warning, safe function from annex K is used + if (asctime_s(buf, BUFFSIZE, timeptr) != 0) + return; + + // no-warning, safe function from annex K is used + if (strcat_s(buf, BUFFSIZE, "something") != 0) + return; + + // no-warning, fseeks supports error checking + if (fseek(f, 0, 0) != 0) + return; + + // no-warning, setvbuf supports error checking + if (setvbuf(f, buf, 0, BUFFSIZE) != 0) + return; +}