[analyzer] LibraryFunctions: Fix errors due to different integral types and typedefs on different architectures.

[NoQ]

Summary

The mechanism for filtering out wrong functions with the same name was too aggressive to filter out eg. int vs. long, when sizes of both are equal. Such issues were detected by several buildbots.

In fact, our function summaries are not precise enough to deal with such differences, and they do not need to be. This patch fixes the issue by only taking size and signedness into account. Also minor cleanups.

[dcoughlin]

Are the parameter types actually needed? I think in general the rest of the analyzer uses arity alone.

Is the idea to allow for overloads in C++? If so, then I think this equivalent-up-to-size-and-sign approach will disallow those overloads.

lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

393 ↗

(On Diff #75695)

Maybe this should be named something like "typesMatch" since it is not actually return whether the types are equal?

418 ↗

(On Diff #75695)

If the problem is only for ssize_t, would it make sense to only do this if both the lhs and rhs are signed?

Another possibility is to have an lhs of what we think is the ssize_t type match any signed type.

[NoQ]

In D25940#579227, @dcoughlin wrote:

Are the parameter types actually needed? I think in general the rest of the analyzer uses arity alone.

Arity checks are to avoid crashes getArg...(). I'm also avoiding assertion failures on APSInt manipulations, which must be of the same type. And even if APSInts didn't assert-fail and were promoted correctly automatically in all operations, we would still want to avoid mismatching types, because results may be completely unexpected.

All right, yep, i'm getting the point about overrides. It's true that we need to find the correct functions better, rather than match more functions. Trying that in the new revision.

[zaks.anna]

How about this imperfect solution that will work quite well in practice? For the ssize_t case, where type size cannot be used, we check the function name, # of arguments , and check that the functions are coming from the system header.

[NoQ]

Try out a completely different approach which was also suggested by Anna.

Allow providing multiple variants of summaries for each function identifier, with different type specifications and branches. This way we preserve type checks and support overloads fairly well, however we introduce a bit of summary data duplication. This solves the ssize_t problem by providing multiple variants for every summary that uses it: one summary for the case when ssize_t is int, one for the case when it's long, one for the case when it's long long.

[zaks.anna]

Please, explain what variants are for in comments.

[zaks.anna]

LGTM other than the missing explanation in comments.

[NoQ]

Comment up on variants.

cfe/trunk/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

class StdLibraryFunctionsChecker : public Checker<check::PostCall, eval::Call> {
   /// A pair of ValueRangeKindTy and IntRangeVectorTy would describe a range
   /// imposed on a particular argument or return value symbol.
   ///
   /// Given a range, should the argument stay inside or outside this range?
   /// The special `ComparesToArgument' value indicates that we should
   /// impose a constraint that involves other argument or return value symbols.
   enum ValueRangeKindTy { OutOfRange, WithinRange, ComparesToArgument };
 
+  // The universal integral type to use in value range descriptions.
+  // Unsigned to make sure overflows are well-defined.
+  typedef uint64_t RangeIntTy;
+
   /// Normally, describes a single range constraint, eg. {{0, 1}, {3, 4}} is
   /// a non-negative integer, which less than 5 and not equal to 2. For
   /// `ComparesToArgument', holds information about how exactly to compare to
   /// the argument.
-  typedef std::vector<std::pair<uint64_t, uint64_t>> IntRangeVectorTy;
+  typedef std::vector<std::pair<RangeIntTy, RangeIntTy>> IntRangeVectorTy;
 
   /// A reference to an argument or return value by its number.
   /// ArgNo in CallExpr and CallEvent is defined as Unsigned, but
   /// obviously uint32_t should be enough for all practical purposes.
   typedef uint32_t ArgNoTy;
   static const ArgNoTy Ret = std::numeric_limits<ArgNoTy>::max();
 
   /// Incapsulates a single range on a single symbol within a branch.
QualType getArgType(ArgNoTy ArgNo) const {
       return T;
     }
 
     /// Try our best to figure out if the call expression is the call of
     /// *the* library function to which this specification applies.
     bool matchesCall(const CallExpr *CE) const;
   };
 
+  // The same function (as in, function identifier) may have different
+  // summaries assigned to it, with different argument and return value types.
+  // We call these "variants" of the function. This can be useful for handling
+  // C++ function overloads, and also it can be used when the same function
+  // may have different definitions on different platforms.
+  typedef std::vector<FunctionSummaryTy> FunctionVariantsTy;
+
   // The map of all functions supported by the checker. It is initialized
   // lazily, and it doesn't change after initialization.
-  typedef llvm::StringMap<FunctionSummaryTy> FunctionSummaryMapTy;
+  typedef llvm::StringMap<FunctionVariantsTy> FunctionSummaryMapTy;
   mutable FunctionSummaryMapTy FunctionSummaryMap;
 
   // Auxiliary functions to support ArgNoTy within all structures
   // in a unified manner.
   static QualType getArgType(const FunctionSummaryTy &Summary, ArgNoTy ArgNo) {
     return Summary.getArgType(ArgNo);
   }
   static QualType getArgType(const CallEvent &Call, ArgNoTy ArgNo) {
StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
   if (FSMI == FunctionSummaryMap.end())
     return None;
 
   // Verify that function signature matches the spec in advance.
   // Otherwise we might be modeling the wrong function.
   // Strict checking is important because we will be conducting
   // very integral-type-sensitive operations on arguments and
   // return values.
-  const FunctionSummaryTy &Spec = FSMI->second;
-  if (!Spec.matchesCall(CE))
-    return None;
-
+  const FunctionVariantsTy &SpecVariants = FSMI->second;
+  for (const FunctionSummaryTy &Spec : SpecVariants)
+    if (Spec.matchesCall(CE))
       return Spec;
+
+  return None;
 }
 
 void StdLibraryFunctionsChecker::initFunctionSummaries(
     BasicValueFactory &BVF) const {
   if (!FunctionSummaryMap.empty())
     return;
 
   ASTContext &ACtx = BVF.getContext();
 
   // These types are useful for writing specifications quickly,
   // New specifications should probably introduce more types.
+  // Some types are hard to obtain from the AST, eg. "ssize_t".
+  // In such cases it should be possible to provide multiple variants
+  // of function summary for common cases (eg. ssize_t could be int or long
+  // or long long, so three summary variants would be enough).
+  // Of course, function variants are also useful for C++ overloads.
   QualType Irrelevant; // A placeholder, whenever we do not care about the type.
   QualType IntTy = ACtx.IntTy;
+  QualType LongTy = ACtx.LongTy;
+  QualType LongLongTy = ACtx.LongLongTy;
   QualType SizeTy = ACtx.getSizeType();
-  QualType SSizeTy = ACtx.getIntTypeForBitwidth(ACtx.getTypeSize(SizeTy), true);
 
-  // Don't worry about truncation here, it'd be cast back to SIZE_MAX when used.
-  int64_t SizeMax =
-      BVF.getMaxValue(SizeTy).getLimitedValue();
-  int64_t SSizeMax =
-    BVF.getMaxValue(SSizeTy).getLimitedValue();
-  (void)SizeMax;
+  RangeIntTy IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
+  RangeIntTy LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
+  RangeIntTy LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
 
   // We are finally ready to define specifications for all supported functions.
   //
   // The signature needs to have the correct number of arguments.
   // However, we insert `Irrelevant' when the type is insignificant.
   //
   // Argument ranges should always cover all variants. If return value
   // is completely unknown, omit it from the respective range set.
void StdLibraryFunctionsChecker::initFunctionSummaries(
   //        { argument index, within or out of, {{from, to}, ...} },
   //        { argument index, compares to argument, {{how, which}} },
   //        ...
   //      }
   //    }
   //  }
   //}
 
+#define SUMMARY_WITH_VARIANTS(identifier) {#identifier, {
+#define END_SUMMARY_WITH_VARIANTS }},
+#define VARIANT(argument_types, return_type, invalidation_approach)            \
+  { argument_types, return_type, invalidation_approach, {
+#define END_VARIANT } },
 #define SUMMARY(identifier, argument_types, return_type,                       \
                 invalidation_approach)                                         \
-  {#identifier, {argument_types, return_type, invalidation_approach, {
-#define END_SUMMARY }}},
+  { #identifier, { { argument_types, return_type, invalidation_approach, {
+#define END_SUMMARY } } } },
 #define ARGUMENT_TYPES(...) { __VA_ARGS__ }
 #define RETURN_TYPE(x) x
 #define INVALIDATION_APPROACH(x) x
 #define CASE {
 #define END_CASE },
 #define ARGUMENT_CONDITION(argument_number, condition_kind)                    \
-  {argument_number, condition_kind, {
+  { argument_number, condition_kind, {
 #define END_ARGUMENT_CONDITION }},
 #define RETURN_VALUE_CONDITION(condition_kind)                                 \
   { Ret, condition_kind, {
 #define END_RETURN_VALUE_CONDITION }},
 #define ARG_NO(x) x##U
 #define RANGE(x, y) { x, y },
 #define SINGLE_VALUE(x) RANGE(x, x)
 #define IS_LESS_THAN(arg) { BO_LE, arg }
SUMMARY(getchar, ARGUMENT_TYPES(), RETURN_TYPE(IntTy),
       CASE // FIXME: EOF is assumed to be defined as -1.
         RETURN_VALUE_CONDITION(WithinRange)
           RANGE(-1, 255)
         END_RETURN_VALUE_CONDITION
       END_CASE
     END_SUMMARY
 
     // read()-like functions that never return more than buffer size.
-    SUMMARY(read, ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
-            RETURN_TYPE(SSizeTy), INVALIDATION_APPROACH(NoEvalCall))
+    // We are not sure how ssize_t is defined on every platform, so we provide
+    // three variants that should cover common cases.
+    SUMMARY_WITH_VARIANTS(read)
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
+              RETURN_TYPE(IntTy), INVALIDATION_APPROACH(NoEvalCall))
         CASE
           RETURN_VALUE_CONDITION(ComparesToArgument)
             IS_LESS_THAN(ARG_NO(2))
           END_RETURN_VALUE_CONDITION
           RETURN_VALUE_CONDITION(WithinRange)
-          RANGE(-1, SSizeMax)
+            RANGE(-1, IntMax)
           END_RETURN_VALUE_CONDITION
         END_CASE
-    END_SUMMARY
-    SUMMARY(write, ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
-            RETURN_TYPE(SSizeTy), INVALIDATION_APPROACH(NoEvalCall))
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
+              RETURN_TYPE(LongTy), INVALIDATION_APPROACH(NoEvalCall))
         CASE
           RETURN_VALUE_CONDITION(ComparesToArgument)
             IS_LESS_THAN(ARG_NO(2))
           END_RETURN_VALUE_CONDITION
           RETURN_VALUE_CONDITION(WithinRange)
-          RANGE(-1, SSizeMax)
+            RANGE(-1, LongMax)
           END_RETURN_VALUE_CONDITION
         END_CASE
-    END_SUMMARY
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
+              RETURN_TYPE(LongLongTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(ComparesToArgument)
+            IS_LESS_THAN(ARG_NO(2))
+          END_RETURN_VALUE_CONDITION
+          RETURN_VALUE_CONDITION(WithinRange)
+            RANGE(-1, LongLongMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+    END_SUMMARY_WITH_VARIANTS
+    SUMMARY_WITH_VARIANTS(write)
+      // Again, due to elusive nature of ssize_t, we have duplicate
+      // our summaries to cover different variants.
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
+              RETURN_TYPE(IntTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(ComparesToArgument)
+            IS_LESS_THAN(ARG_NO(2))
+          END_RETURN_VALUE_CONDITION
+          RETURN_VALUE_CONDITION(WithinRange)
+            RANGE(-1, IntMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
+              RETURN_TYPE(LongTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(ComparesToArgument)
+            IS_LESS_THAN(ARG_NO(2))
+          END_RETURN_VALUE_CONDITION
+          RETURN_VALUE_CONDITION(WithinRange)
+            RANGE(-1, LongMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy),
+              RETURN_TYPE(LongLongTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(ComparesToArgument)
+            IS_LESS_THAN(ARG_NO(2))
+          END_RETURN_VALUE_CONDITION
+          RETURN_VALUE_CONDITION(WithinRange)
+            RANGE(-1, LongLongMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+    END_SUMMARY_WITH_VARIANTS
     SUMMARY(fread,
             ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy, Irrelevant),
             RETURN_TYPE(SizeTy), INVALIDATION_APPROACH(NoEvalCall))
       CASE
         RETURN_VALUE_CONDITION(ComparesToArgument)
           IS_LESS_THAN(ARG_NO(2))
         END_RETURN_VALUE_CONDITION
       END_CASE
     END_SUMMARY
     SUMMARY(fwrite,
             ARGUMENT_TYPES(Irrelevant, Irrelevant, SizeTy, Irrelevant),
             RETURN_TYPE(SizeTy), INVALIDATION_APPROACH(NoEvalCall))
       CASE
         RETURN_VALUE_CONDITION(ComparesToArgument)
           IS_LESS_THAN(ARG_NO(2))
         END_RETURN_VALUE_CONDITION
       END_CASE
     END_SUMMARY
 
     // getline()-like functions either fail or read at least the delimiter.
-    SUMMARY(getline, ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant),
-            RETURN_TYPE(SSizeTy), INVALIDATION_APPROACH(NoEvalCall))
+    SUMMARY_WITH_VARIANTS(getline)
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant),
+              RETURN_TYPE(IntTy), INVALIDATION_APPROACH(NoEvalCall))
         CASE
           RETURN_VALUE_CONDITION(WithinRange)
             SINGLE_VALUE(-1)
-          RANGE(1, SSizeMax)
+            RANGE(1, IntMax)
           END_RETURN_VALUE_CONDITION
         END_CASE
-    END_SUMMARY
-    SUMMARY(getdelim,
-            ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant, Irrelevant),
-            RETURN_TYPE(SSizeTy), INVALIDATION_APPROACH(NoEvalCall))
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant),
+              RETURN_TYPE(LongTy), INVALIDATION_APPROACH(NoEvalCall))
         CASE
           RETURN_VALUE_CONDITION(WithinRange)
             SINGLE_VALUE(-1)
-          RANGE(1, SSizeMax)
+            RANGE(1, LongMax)
           END_RETURN_VALUE_CONDITION
         END_CASE
-    END_SUMMARY
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant),
+              RETURN_TYPE(LongLongTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(WithinRange)
+            SINGLE_VALUE(-1)
+            RANGE(1, LongLongMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+    END_SUMMARY_WITH_VARIANTS
+    SUMMARY_WITH_VARIANTS(getdelim)
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant, Irrelevant),
+            RETURN_TYPE(IntTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(WithinRange)
+            SINGLE_VALUE(-1)
+            RANGE(1, IntMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant, Irrelevant),
+            RETURN_TYPE(LongTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(WithinRange)
+            SINGLE_VALUE(-1)
+            RANGE(1, LongMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+      VARIANT(ARGUMENT_TYPES(Irrelevant, Irrelevant, Irrelevant, Irrelevant),
+            RETURN_TYPE(LongLongTy), INVALIDATION_APPROACH(NoEvalCall))
+        CASE
+          RETURN_VALUE_CONDITION(WithinRange)
+            SINGLE_VALUE(-1)
+            RANGE(1, LongLongMax)
+          END_RETURN_VALUE_CONDITION
+        END_CASE
+      END_VARIANT
+    END_SUMMARY_WITH_VARIANTS
   };
 }
 
 void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) {
   // If this checker grows large enough to support C++, Objective-C, or other
   // standard libraries, we could use multiple register...Checker() functions,
   // which would register various checkers with the help of the same Checker
   // class, turning on different function summaries.
   mgr.registerChecker<StdLibraryFunctionsChecker>();
 }

cfe/trunk/test/Analysis/std-c-library-functions.c

+// RUN: %clang_cc1 -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple i686-unknown-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple armv7-a15-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple thumbv7-a15-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
 
 void clang_analyzer_eval(int);
 
 int glob;
 
 typedef struct FILE FILE;
 #define EOF -1
 
 int getc(FILE *);
 void test_getc(FILE *fp) {
   int x;
   while ((x = getc(fp)) != EOF) {
     clang_analyzer_eval(x > 255); // expected-warning{{FALSE}}
     clang_analyzer_eval(x >= 0); // expected-warning{{TRUE}}
   }
 }
 
 int fgetc(FILE *);
 void test_fgets(FILE *fp) {
   clang_analyzer_eval(fgetc(fp) < 256); // expected-warning{{TRUE}}
   clang_analyzer_eval(fgetc(fp) >= 0); // expected-warning{{UNKNOWN}}
 }
 
 
-typedef unsigned long size_t;
+typedef typeof(sizeof(int)) size_t;
 typedef signed long ssize_t;
 ssize_t read(int, void *, size_t);
 ssize_t write(int, const void *, size_t);
 void test_read_write(int fd, char *buf) {
   glob = 1;
   ssize_t x = write(fd, buf, 10);
   clang_analyzer_eval(glob); // expected-warning{{UNKNOWN}}
   if (x >= 0) {