r355592 - [analyzer] Handle comparison between non-default AS symbol and constant

[David Stenberg via cfe-commits]

Author: dstenb
Date: Thu Mar  7 05:01:17 2019
New Revision: 355592

URL: http://llvm.org/viewvc/llvm-project?rev=355592&view=rev
Log:
[analyzer] Handle comparison between non-default AS symbol and constant

Summary:
When comparing a symbolic region and a constant, the constant would be
widened or truncated to the width of a void pointer, meaning that the
constant could be incorrectly truncated when handling symbols for
non-default address spaces. In the attached test case this resulted in a
false positive since the constant was truncated to zero. To fix this,
widen/truncate the constant to the width of the symbol expression's
type.

This commit does not consider non-symbolic regions as I'm not sure how
to generalize getting the type there.

This fixes PR40814.

Reviewers: NoQ, zaks.anna, george.karpenkov

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, jdoerfert, Charusso, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D58665

Added:
    cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=355592&r1=355591&r2=355592&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Thu Mar  7 05:01:17 2019
@@ -571,7 +571,15 @@ SVal SimpleSValBuilder::evalBinOpNN(Prog
           // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
           // then pack it back into a LocAsInteger.
           llvm::APSInt i = rhs.castAs<nonloc::ConcreteInt>().getValue();
-          BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
+          // If the region has a symbolic base, pay attention to the type; it
+          // might be coming from a non-default address space. For non-symbolic
+          // regions it doesn't matter that much because such comparisons would
+          // most likely evaluate to concrete false anyway. FIXME: We might
+          // still need to handle the non-comparison case.
+          if (SymbolRef lSym = lhs.getAsLocSymbol(true))
+            BasicVals.getAPSIntType(lSym->getType()).apply(i);
+          else
+            BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
           return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
         }
         default:

Added: cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl?rev=355592&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl (added)
+++ cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl Thu Mar  7 05:01:17 2019
@@ -0,0 +1,11 @@
+//RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown -analyze -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+#include <stdint.h>
+
+void bar(__global int *p) __attribute__((nonnull(1)));
+
+void foo(__global int *p) {
+  if ((uint64_t)p <= 1UL << 32)
+    bar(p); // no-warning
+}