[clang-tidy] add ctime and localtime to clang-tidy

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

@@ -50,6 +50,8 @@ static StringRef getReplacementFor(StringRef FunctionName,
     StringRef AnnexKReplacementFunction =
         StringSwitch<StringRef>(FunctionName)
             .Cases("asctime", "asctime_r", "asctime_s")
+            .Cases("ctime", "ctime_r", "ctime_s")
+            .Cases("localtime", "localtime_r", "localtime_s")
             .Case("gets", "gets_s")
             .Default({});
     if (!AnnexKReplacementFunction.empty())
@@ -60,6 +62,8 @@ static StringRef getReplacementFor(StringRef FunctionName,
   // should be matched and suggested.
   return StringSwitch<StringRef>(FunctionName)
       .Cases("asctime", "asctime_r", "strftime")
+      .Cases("ctime", "ctime_r", "ctime_s")
+      .Cases("localtime", "localtime_r", "localtime_s")
       .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<StringRef>(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)))

clang-tools-extra/docs/ReleaseNotes.rst

@@ -134,6 +134,9 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- New unsafe functions checks :doc:`bugprone-unsafe-functions-check`
+  <clang-tidy/bugprone/UnsafeFunctionsCheck.cpp> were added to clang-tidy.
+
 - Improved :doc:`bugprone-optional-value-conversion
   <clang-tidy/checks/bugprone/optional-value-conversion>` check to detect
   conversion in argument of ``std::make_optional``.
@@ -152,7 +155,8 @@ Changes in existing checks
 
 - Improved :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/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
   <clang-tidy/checks/cert/err33-c>` check by fixing false positives when

clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst

@@ -34,12 +34,15 @@ 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:
 
  - ``asctime``, ``asctime_r``, suggested replacement: ``strftime``
+ - ``ctime``, ``ctime_r``, suggested replacement: ``ctime_s``
+ - ``localtime``, ``localtime_r``, suggested replacement: ``localtime_s``
  - ``gets``, suggested replacement: ``fgets``
 
 The following functions are always checked, regardless of *Annex K* availability:

clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c

@@ -155,15 +155,25 @@ void fOptional() {
 typedef int errno_t;
 typedef size_t rsize_t;
 errno_t asctime_s(char *S, rsize_t Maxsize, const struct tm *TimePtr);
+errno_t ctime_s(char *S, rsize_t Maxsize, const time_t *Timep);
+errno_t localtime_s(const time_t *Timep, 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
   if (asctime_s(Buf, BUFSIZ, Time) != 0)
     return;
 
+  // no-warning, safe function from annex K is used
+  if (ctime_s(Buf, BUFSIZ, Time) != 0)
+    return;
+
+  // no-warning, safe function from annex K is used
+  if (localtime_s(Timep, BUFSIZ, Time) != 0)
+    return;
+
   // no-warning, safe function from annex K is used
   if (strcat_s(Buf, BUFSIZ, "something") != 0)
     return;

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

@@ -3533,10 +3533,19 @@ 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 *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))));
+
+    // struct tm *localtime_s(const time_t *restrict timer,
     //                        struct tm *restrict result);
     addToFunctionSummaryMap(
-        "localtime_r",
+        "localtime_s",
         Signature(ArgTypes{ConstTime_tPtrRestrictTy, StructTmPtrRestrictTy},
                   RetType{StructTmPtrTy}),
         Summary(NoEvalCall)
@@ -3565,15 +3574,29 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                 /*Buffer=*/ArgNo(1),
                 /*MinBufSize=*/BVF.getValue(26, IntTy))));
 
-    // struct tm *gmtime_r(const time_t *restrict timer,
-    //                     struct tm *restrict result);
+    // char *ctime_s(char *buf, rsize_t buf_size, const time_t *timep);
     addToFunctionSummaryMap(
-        "gmtime_r",
-        Signature(ArgTypes{ConstTime_tPtrRestrictTy, StructTmPtrRestrictTy},
-                  RetType{StructTmPtrTy}),
+        "ctime_s",
+        Signature(ArgTypes{CharPtrTy,
+                           BufferSize(ArgNo(0), BVF.getValue(26, IntTy)),
+                           ConstTime_tPtrTy},
+                  RetType{CharPtrTy}),
         Summary(NoEvalCall)
             .ArgConstraint(NotNull(ArgNo(0)))
-            .ArgConstraint(NotNull(ArgNo(1))));
+            .ArgConstraint(BufferSize(
+                /*Buffer=*/ArgNo(0),
+                /*MinBufSize=*/BVF.getValue(26, IntTy))));
+    .ArgConstraint(NotNull(ArgNo(2)))
+
+        // struct tm *gmtime_r(const time_t *restrict timer,
+        //                     struct tm *restrict result);
+        addToFunctionSummaryMap(
+            "gmtime_r",
+            Signature(ArgTypes{ConstTime_tPtrRestrictTy, StructTmPtrRestrictTy},
+                      RetType{StructTmPtrTy}),
+            Summary(NoEvalCall)
+                .ArgConstraint(NotNull(ArgNo(0)))
+                .ArgConstraint(NotNull(ArgNo(1))));
 
     // struct tm * gmtime(const time_t *tp);
     addToFunctionSummaryMap(

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

clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc

@@ -2032,8 +2032,8 @@ SYMBOL(locale, std::, <locale>)
 SYMBOL(localeconv, std::, <clocale>)
 SYMBOL(localeconv, None, <clocale>)
 SYMBOL(localeconv, None, <locale.h>)
-SYMBOL(localtime, std::, <ctime>)
-SYMBOL(localtime, None, <ctime>)
+SYMBOL(localtime, std::, <localtime>)
+SYMBOL(localtime, None, <localtime>)
 SYMBOL(localtime, None, <time.h>)
 SYMBOL(lock, std::, <mutex>)
 SYMBOL(lock_guard, std::, <mutex>)

clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h

@@ -174,9 +174,11 @@ 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);
+struct tm *localtime_s(const time_t *restrict timer, struct tm *restrict result);
 char *asctime_r(const struct tm *restrict tm, char *restrict buf);
 char *ctime_r(const time_t *timep, char *buf);
+char *ctime_s(char *buf, rsize_t buf_size, const time_t *timep);
 struct tm *gmtime_r(const time_t *restrict timer, struct tm *restrict result);
 struct tm *gmtime(const time_t *tp);
 int clock_gettime(clockid_t clock_id, struct timespec *tp);

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}}

clang/test/Analysis/std-c-library-functions-POSIX.c

@@ -129,9 +129,11 @@
 // 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: struct tm *localtime_s(const time_t *restrict timer, struct tm *restrict 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: char *ctime_s(char *buf, rsize_t buf_size, const time_t *timep)
 // CHECK: Loaded summary for: struct tm *gmtime_r(const time_t *restrict timer, struct tm *restrict result)
 // CHECK: Loaded summary for: struct tm *gmtime(const time_t *tp)
 // CHECK: Loaded summary for: int clock_gettime(clockid_t clock_id, struct timespec *tp)

compiler-rt/lib/dfsan/libc_ubuntu1404_abilist.txt

@@ -1547,6 +1547,7 @@ fun:ctanl=uninstrumented
 fun:ctermid=uninstrumented
 fun:ctime=uninstrumented
 fun:ctime_r=uninstrumented
+fun:ctime_s=uninstrumented
 fun:cuserid=uninstrumented
 fun:daemon=uninstrumented
 fun:dcgettext=uninstrumented
@@ -2205,6 +2206,7 @@ fun:llseek=uninstrumented
 fun:localeconv=uninstrumented
 fun:localtime=uninstrumented
 fun:localtime_r=uninstrumented
+fun:localtime_s=uninstrumented
 fun:lockf=uninstrumented
 fun:lockf64=uninstrumented
 fun:log=uninstrumented

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

@@ -34,6 +34,7 @@
 //===----------------------------------------------------------------------===//
 
 #include <stdarg.h>
+#include <stddef.h>
 
 #include "interception/interception.h"
 #include "sanitizer_addrhashmap.h"
@@ -1375,6 +1376,16 @@ INTERCEPTOR(__sanitizer_tm *, localtime_r, unsigned long *timep, void *result) {
   }
   return res;
 }
+INTERCEPTOR(__sanitizer_tm *, localtime_s, unsigned long *timep, void *result) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, localtime_r, timep, result);
+  __sanitizer_tm *res = REAL(localtime_r)(timep, result);
+  if (res) {
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, timep, sizeof(*timep));
+    unpoison_tm(ctx, res);
+  }
+  return res;
+}
 INTERCEPTOR(__sanitizer_tm *, gmtime, unsigned long *timep) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, gmtime, timep);
@@ -1421,6 +1432,20 @@ INTERCEPTOR(char *, ctime_r, unsigned long *timep, char *result) {
   }
   return res;
 }
+INTERCEPTOR(char *, ctime_s, char *result, size_t result_size,
+            unsigned long *timep) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, ctime_s, result, result_size, timep);
+  // FIXME: under ASan the call below may write to freed memory and corrupt
+  // its metadata. See
+  // https://github.com/google/sanitizers/issues/321.
+  char *res = REAL(ctime_s)(result, result_size, timep);
+  if (res) {
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, timep, sizeof(*timep));
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, res, internal_strlen(res) + 1);
+  }
+  return res;
+}
 INTERCEPTOR(char *, asctime, __sanitizer_tm *tm) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, asctime, tm);