[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

llvm-svn: 355592

diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 19285818645e310c7811a522b77bcd5d61f61568..aaf29abd47309691b8d4d1605735bb940bdbdc3e 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -571,7 +571,15 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
           // 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:

diff --git a/clang/test/Analysis/ptr-cmp-const-trunc.cl b/clang/test/Analysis/ptr-cmp-const-trunc.cl
new file mode 100644 (file)
index 0000000..4483ef6
--- /dev/null
+++ b/clang/test/Analysis/ptr-cmp-const-trunc.cl
@@ -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
+}