diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index a45949314a4ca..b00cd81c36556 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -50,6 +50,8 @@ static StringRef getReplacementFor(StringRef FunctionName, StringRef AnnexKReplacementFunction = StringSwitch(FunctionName) .Cases("asctime", "asctime_r", "asctime_s") + .Cases("ctime", "ctime_r") + .Cases("localtime", "localtime_r") .Case("gets", "gets_s") .Default({}); if (!AnnexKReplacementFunction.empty()) @@ -60,6 +62,8 @@ static StringRef getReplacementFor(StringRef FunctionName, // should be matched and suggested. return StringSwitch(FunctionName) .Cases("asctime", "asctime_r", "strftime") + .Cases("ctime", "ctime_r") + .Cases("localtime", "localtime_r") .Case("gets", "fgets") .Case("rewind", "fseek") .Case("setbuf", "setvbuf"); @@ -90,8 +94,8 @@ static StringRef getReplacementForAdditional(StringRef FunctionName, /// safer alternative. static StringRef getRationaleFor(StringRef FunctionName) { return StringSwitch(FunctionName) - .Cases("asctime", "asctime_r", "ctime", - "is not bounds-checking and non-reentrant") + .Cases("asctime", "asctime_r", "ctime", "ctime_r", "localtime", + "localtime_r", "is not bounds-checking and non-reentrant") .Cases("bcmp", "bcopy", "bzero", "is deprecated") .Cases("fopen", "freopen", "has no exclusive access to the opened file") .Case("gets", "is insecure, was deprecated and removed in C11 and C++14") @@ -223,8 +227,9 @@ void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) { } // Matching functions with replacements without Annex K. - auto FunctionNamesMatcher = - hasAnyName("::asctime", "asctime_r", "::gets", "::rewind", "::setbuf"); + auto FunctionNamesMatcher = hasAnyName( + "::asctime", "asctime_r", "::ctime", "ctime_r", "::localtime", + "localtime_r", "::gets", "::rewind", "::setbuf"); Finder->addMatcher( declRefExpr( to(functionDecl(FunctionNamesMatcher).bind(FunctionNamesId))) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8f61839af2c80..34645ce7ee5d5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -152,7 +152,8 @@ Changes in existing checks - Improved :doc:`bugprone-unsafe-functions ` check to allow specifying - additional C++ member functions to match. + additional C++ member functions to match and by adding ``ctime`` and + ``localtime`` to unsafe functions list. - Improved :doc:`cert-err33-c ` check by fixing false positives when diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst index 317db9c5564e2..17c5f9e66c74a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst @@ -34,7 +34,8 @@ following functions: ``vsnprintf``, ``vsprintf``, ``vsscanf``, ``vswprintf``, ``vswscanf``, ``vwprintf``, ``vwscanf``, ``wcrtomb``, ``wcscat``, ``wcscpy``, ``wcslen``, ``wcsncat``, ``wcsncpy``, ``wcsrtombs``, ``wcstok``, ``wcstombs``, -``wctomb``, ``wmemcpy``, ``wmemmove``, ``wprintf``, ``wscanf``. +``wctomb``, ``wmemcpy``, ``wmemmove``, ``wprintf``, ``wscanf``. ``ctime_r``, +``localtime_r`` If *Annex K.* is not available, replacements are suggested only for the following functions from the previous list: diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c index 0409dd6bfcaa3..031db4a81ffc3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c @@ -108,8 +108,6 @@ 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 not bounds-checking and non-reentrant; 'ctime_s' should be used instead - // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'ctime' is not bounds-checking and non-reentrant; 'ctime_s' should be used instead // no-warning WITHOUT-ANNEX-K } @@ -157,7 +155,7 @@ 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); -void fUsingSafeFunctions(const struct tm *Time, FILE *F) { +void fUsingSafeFunctions(const struct tm *Time, FILE *F, time_t *Timep) { char Buf[BUFSIZ] = {0}; // no-warning, safe function from annex K is used diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 17227a23571ab..b01c2d3b5119f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -3533,15 +3533,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( Signature(ArgTypes{ConstTime_tPtrTy}, RetType{StructTmPtrTy}), Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0)))); - // struct tm *localtime_r(const time_t *restrict timer, - // struct tm *restrict result); - addToFunctionSummaryMap( - "localtime_r", - Signature(ArgTypes{ConstTime_tPtrRestrictTy, StructTmPtrRestrictTy}, - RetType{StructTmPtrTy}), - Summary(NoEvalCall) - .ArgConstraint(NotNull(ArgNo(0))) - .ArgConstraint(NotNull(ArgNo(1)))); + // struct tm *localtime_r(const time_t *timer, + // struct tm *result); + addToFunctionSummaryMap("localtime_r", + Signature(ArgTypes{ConstTime_tPtrTy, StructTmPtrTy}, + RetType{StructTmPtrTy}), + Summary(NoEvalCall) + .ArgConstraint(NotNull(ArgNo(0))) + .ArgConstraint(NotNull(ArgNo(1)))); // char *asctime_r(const struct tm *restrict tm, char *restrict buf); addToFunctionSummaryMap( diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp index 7d465eee7cc0b..72d20b6beb08f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp @@ -76,6 +76,10 @@ class InvalidPtrChecker &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, {{CDM::CLibrary, {"asctime"}, 1}, &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + {{CDM::CLibrary, {"ctime"}, 1}, + &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + {{CDM::CLibrary, {"localtime"}, 1}, + &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, }; // The private members of this checker corresponding to commandline options diff --git a/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc b/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc index c1927180d3397..65e3573282712 100644 --- a/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc +++ b/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc @@ -2032,8 +2032,8 @@ SYMBOL(locale, std::, ) SYMBOL(localeconv, std::, ) SYMBOL(localeconv, None, ) SYMBOL(localeconv, None, ) -SYMBOL(localtime, std::, ) -SYMBOL(localtime, None, ) +SYMBOL(localtime, std::, ) +SYMBOL(localtime, None, ) SYMBOL(localtime, None, ) SYMBOL(lock, std::, ) SYMBOL(lock_guard, std::, ) diff --git a/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h b/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h index b146068eedb08..63403bd1669b1 100644 --- a/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h +++ b/clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h @@ -174,7 +174,7 @@ int utimensat(int dirfd, const char *pathname, const struct timespec times[2], i int utimes(const char *filename, const struct timeval times[2]); int nanosleep(const struct timespec *rqtp, struct timespec *rmtp); struct tm *localtime(const time_t *tp); -struct tm *localtime_r(const time_t *restrict timer, struct tm *restrict result); +struct tm *localtime_r(const time_t *timer, struct tm *result); char *asctime_r(const struct tm *restrict tm, char *restrict buf); char *ctime_r(const time_t *timep, char *buf); struct tm *gmtime_r(const time_t *restrict timer, struct tm *restrict result); diff --git a/clang/test/Analysis/cert/env34-c.c b/clang/test/Analysis/cert/env34-c.c index d307f0d8f4bb0..3e764543b7ed0 100644 --- a/clang/test/Analysis/cert/env34-c.c +++ b/clang/test/Analysis/cert/env34-c.c @@ -16,6 +16,8 @@ lconv *localeconv(void); typedef struct { } tm; char *asctime(const tm *timeptr); +char *ctime(const time_t *time); +struct tm *localtime(const time_t *time); int strcmp(const char*, const char*); extern void foo(char *e); @@ -313,6 +315,34 @@ void asctime_test(void) { // expected-note@-2{{dereferencing an invalid pointer}} } +void ctime_test(void) { + const time_t *t; + const time_t *tt; + + char* p = ctime(t); + // expected-note@-1{{previous function call was here}} + char* pp = ctime(tt); + // expected-note@-1{{'ctime' call may invalidate the result of the previous 'ctime'}} + + *p; + // expected-warning@-1{{dereferencing an invalid pointer}} + // expected-note@-2{{dereferencing an invalid pointer}} +} + +void localtime_test(void) { + const time_t *t; + const time_t *tt; + + struct tm* p = localtime(t); + // expected-note@-1{{previous function call was here}} + struct tm* pp = localtime(tt); + // expected-note@-1{{'localtime' call may invalidate the result of the previous 'localtime'}} + + *p; + // expected-warning@-1{{dereferencing an invalid pointer}} + // expected-note@-2{{dereferencing an invalid pointer}} +} + void localeconv_test1(void) { lconv *lc1 = localeconv(); // expected-note@-1{{previous function call was here}} diff --git a/clang/test/Analysis/std-c-library-functions-POSIX.c b/clang/test/Analysis/std-c-library-functions-POSIX.c index f6d88e6c1502d..718076f779ba7 100644 --- a/clang/test/Analysis/std-c-library-functions-POSIX.c +++ b/clang/test/Analysis/std-c-library-functions-POSIX.c @@ -129,7 +129,7 @@ // CHECK: Loaded summary for: int utimes(const char *filename, const struct timeval times[2]) // CHECK: Loaded summary for: int nanosleep(const struct timespec *rqtp, struct timespec *rmtp) // CHECK: Loaded summary for: struct tm *localtime(const time_t *tp) -// CHECK: Loaded summary for: struct tm *localtime_r(const time_t *restrict timer, struct tm *restrict result) +// CHECK: Loaded summary for: struct tm *localtime_r(const time_t *timer, struct tm *result) // CHECK: Loaded summary for: char *asctime_r(const struct tm *restrict tm, char *restrict buf) // CHECK: Loaded summary for: char *ctime_r(const time_t *timep, char *buf) // CHECK: Loaded summary for: struct tm *gmtime_r(const time_t *restrict timer, struct tm *restrict result)