From 41721e3ad52e643c999836c367cfec4d7a7763e4 Mon Sep 17 00:00:00 2001 From: David Stenberg Date: Thu, 7 Mar 2019 13:01:17 +0000 Subject: [PATCH] [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@355592 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp | 10 +++++++++- test/Analysis/ptr-cmp-const-trunc.cl | 11 +++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 test/Analysis/ptr-cmp-const-trunc.cl diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 1928581864..aaf29abd47 100644 --- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/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().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/test/Analysis/ptr-cmp-const-trunc.cl b/test/Analysis/ptr-cmp-const-trunc.cl new file mode 100644 index 0000000000..4483ef6839 --- /dev/null +++ b/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 + +void bar(__global int *p) __attribute__((nonnull(1))); + +void foo(__global int *p) { + if ((uint64_t)p <= 1UL << 32) + bar(p); // no-warning +} -- 2.50.1