From: George Karpenkov Date: Tue, 27 Feb 2018 00:05:04 +0000 (+0000) Subject: Revert "Revert "[analyzer] Quickfix: do not overflow in calculating offset in RegionM... X-Git-Tag: llvmorg-7.0.0-rc1~11929 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=50339a2e84622f6c492a4b2f3dc5e79a6769399a;p=platform%2Fupstream%2Fllvm.git Revert "Revert "[analyzer] Quickfix: do not overflow in calculating offset in RegionManager"" This reverts commit c4cc41166d93178a3ddd4b2b5a685cf74a459247. Revert and fix uninitialized read. llvm-svn: 326152 --- diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index aa54544..44099f4 100644 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -23,6 +23,11 @@ #include "clang/Basic/SourceManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/Debug.h" + +#include + +#define DEBUG_TYPE "MemRegion" using namespace clang; using namespace ento; @@ -1149,6 +1154,36 @@ const SymbolicRegion *MemRegion::getSymbolicBase() const { return nullptr; } +/// Perform a given operation on two integers, return whether it overflows. +/// Optionally write the resulting output into \p Res. +static bool checkedOp( + int64_t LHS, + int64_t RHS, + std::function Op, + int64_t *Res = nullptr) { + llvm::APInt ALHS(/*BitSize=*/64, LHS, /*Signed=*/true); + llvm::APInt ARHS(/*BitSize=*/64, RHS, /*Signed=*/true); + bool Overflow; + llvm::APInt Out = Op(&ALHS, ARHS, Overflow); + if (!Overflow && Res) + *Res = Out.getSExtValue(); + return Overflow; +} + +static bool checkedAdd( + int64_t LHS, + int64_t RHS, + int64_t *Res=nullptr) { + return checkedOp(LHS, RHS, &llvm::APInt::sadd_ov, Res); +} + +static bool checkedMul( + int64_t LHS, + int64_t RHS, + int64_t *Res=nullptr) { + return checkedOp(LHS, RHS, &llvm::APInt::smul_ov, Res); +} + RegionRawOffset ElementRegion::getAsArrayOffset() const { CharUnits offset = CharUnits::Zero(); const ElementRegion *ER = this; @@ -1176,6 +1211,18 @@ RegionRawOffset ElementRegion::getAsArrayOffset() const { } CharUnits size = C.getTypeSizeInChars(elemType); + + int64_t Mult; + bool Overflow = checkedAdd(i, size.getQuantity(), &Mult); + if (!Overflow) + Overflow = checkedMul(Mult, offset.getQuantity()); + if (Overflow) { + DEBUG(llvm::dbgs() << "MemRegion::getAsArrayOffset: " + << "offset overflowing, returning unknown\n"); + + return nullptr; + } + offset += (i * size); } diff --git a/clang/test/Analysis/region-store.cpp b/clang/test/Analysis/region-store.cpp index cb49f48..ab179ce 100644 --- a/clang/test/Analysis/region-store.cpp +++ b/clang/test/Analysis/region-store.cpp @@ -25,4 +25,4 @@ int radar13445834(Derived *Builder, Loc l) { Builder->setLoc(l); return Builder->accessBase(); -} \ No newline at end of file +} diff --git a/clang/test/Analysis/region_store_overflow.c b/clang/test/Analysis/region_store_overflow.c new file mode 100644 index 0000000..81acd41 --- /dev/null +++ b/clang/test/Analysis/region_store_overflow.c @@ -0,0 +1,13 @@ +// REQUIRES: asserts +// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core -mllvm -debug %s 2>&1 | FileCheck %s + +int **h; +int overflow_in_memregion(long j) { + for (int l = 0;; ++l) { + if (j - l > 0) + return h[j - l][0]; // no-crash + } + return 0; +} +// CHECK: {{.*}} +// CHECK: MemRegion::getAsArrayOffset: offset overflowing, returning unknown