[analyzer] Resolve the crash in ReturnUndefChecker

[george.karpenkov]

Summary

By making sure the returned value from getKnownSVal is consistent with the value used inside expression engine.

PR38427

[NoQ]

clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp

66–67

Yup, this function, by contract, trying to explain why does ExprEngine think that the value is undefined, and it knows that ExprEngine would only decide that if the operands are known.

The assertion is good but i guess it'd be great to document this contract more explicitly.

clang/test/Analysis/casts.c

192–200

Emm, how come that test was already there?

clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp

static bool isArrayIndexOutOfBounds(CheckerContext &C, const Expr *Ex) {
   return StOutBound && !StInBound;
 }
 
 static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) {
   return C.isGreaterOrEqual(
       B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType()));
 }
 
 static bool isLeftShiftResultUnrepresentable(const BinaryOperator *B,
                                              CheckerContext &C) {
   SValBuilder &SB = C.getSValBuilder();
   ProgramStateRef State = C.getState();
   const llvm::APSInt *LHS = SB.getKnownValue(State, C.getSVal(B->getLHS()));
   const llvm::APSInt *RHS = SB.getKnownValue(State, C.getSVal(B->getRHS()));
+  assert(LHS && RHS && "Values unknown, inconsistent state");
   return (unsigned)RHS->getZExtValue() > LHS->countLeadingZeros();
 }
 
 void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
                                        CheckerContext &C) const {
   if (C.getSVal(B).isUndef()) {
 
     // Do not report assignments of uninitialized values inside swap functions.

clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

                                                        superR, getContext()));
     }
   }
   return UnknownVal();
 }
 
 const llvm::APSInt *SimpleSValBuilder::getKnownValue(ProgramStateRef state,
                                                    SVal V) {
+  V = simplifySVal(state, V);
   if (V.isUnknownOrUndef())
     return nullptr;
 
   if (Optional<loc::ConcreteInt> X = V.getAs<loc::ConcreteInt>())
     return &X->getValue();
 
   if (Optional<nonloc::ConcreteInt> X = V.getAs<nonloc::ConcreteInt>())
     return &X->getValue();

clang/test/Analysis/casts.c

-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -DEAGERLY_ASSUME=1 -w %s
 
 extern void clang_analyzer_eval(_Bool);
 
 // Test if the 'storage' region gets properly initialized after it is cast to
 // 'struct sockaddr *'. 
 
 typedef unsigned char __uint8_t;
 typedef unsigned int __uint32_t;
 typedef __uint32_t __darwin_socklen_t;
 typedef __uint8_t sa_family_t;
 typedef __darwin_socklen_t socklen_t;
 struct sockaddr { sa_family_t sa_family; };
 struct sockaddr_storage {};
 
 void getsockname();
 
+#ifndef EAGERLY_ASSUME
+
 void f(int sock) {
   struct sockaddr_storage storage;
   struct sockaddr* sockaddr = (struct sockaddr*)&storage; // expected-warning{{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}}
   socklen_t addrlen = sizeof(storage);
   getsockname(sock, sockaddr, &addrlen);
   switch (sockaddr->sa_family) { // no-warning
   default:
     ;

 }
 
 void testSwitchWithSizeofs() {
   switch (sizeof(char) == 1) { // expected-warning{{switch condition has boolean value}}
   case sizeof(char):; // no-crash
   }
 }
 
-int globalA;
+#endif
+
+#ifdef EAGERLY_ASSUME
+
+// expected-no-diagnostics
+
+int globalA; // TODO: the example is not representative.
 extern int globalFunc();
 void no_crash_on_symsym_cast_to_long() {
   char c = globalFunc() - 5;
-  (void)(c == 0);
+  c == 0;
   globalA -= c;
-  (void)(globalA == 3);
-  (void)((long)globalA << 48); // no-crash
+  globalA == 3;
+  (long)globalA << 48; // no-crash
 }
+
+#endif