Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -147,6 +147,8 @@ /// Compute the offset within the top level memory object. RegionOffset getAsOffset() const; + uint64_t getExtent() const; + /// Get a string representation of a region for debug use. std::string getString() const; Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -1602,6 +1602,23 @@ return *cachedOffset; } +uint64_t MemRegion::getExtent() const { + const MemRegion *R = this; + uint64_t Extent = 0; + + if (const auto *TVR = dyn_cast(R)) { + const auto Ty = TVR->getDesugaredValueType(getContext()); + if (!Ty->isIncompleteType()) + Extent = getContext().getTypeSize(Ty); + } else if (const auto *SR = dyn_cast(R)) { + const auto Ty = SR->getSymbol()->getType().getDesugaredType(getContext()); + if (!Ty->isIncompleteType()) + Extent = getContext().getTypeSize(Ty); + } + + return Extent; +} + //===----------------------------------------------------------------------===// // BlockDataRegion //===----------------------------------------------------------------------===// Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -48,24 +48,35 @@ llvm::PointerIntPair P; uint64_t Data; + uint64_t Extent; + bool CompareExtent = false; /// Create a key for a binding to region \p r, which has a symbolic offset /// from region \p Base. - explicit BindingKey(const SubRegion *r, const SubRegion *Base, Kind k) - : P(r, k | Symbolic), Data(reinterpret_cast(Base)) { + explicit BindingKey(const SubRegion *r, const SubRegion *Base, Kind k, + uint64_t extent) + : P(r, k | Symbolic), Data(reinterpret_cast(Base)), + Extent(extent) { assert(r && Base && "Must have known regions."); assert(getConcreteOffsetRegion() == Base && "Failed to store base region"); } /// Create a key for a binding at \p offset from base region \p r. - explicit BindingKey(const MemRegion *r, uint64_t offset, Kind k) - : P(r, k), Data(offset) { + explicit BindingKey(const MemRegion *r, uint64_t offset, Kind k, + uint64_t extent) + : P(r, k), Data(offset), Extent(extent) { assert(r && "Must have known regions."); assert(getOffset() == offset && "Failed to store offset"); assert((r == r->getBaseRegion() || isa(r)) && "Not a base"); } + + /// A helper constructor that makes it possible to compare keys while ignoring + /// the extent. + explicit BindingKey(const BindingKey &K, bool CompareExtent) + : P(K.P), Data(K.Data), Extent(K.Extent), CompareExtent(CompareExtent) {} + public: bool isDirect() const { return P.getInt() & Direct; } @@ -77,6 +88,8 @@ return Data; } + uint64_t getExtent() const { return Extent; } + const SubRegion *getConcreteOffsetRegion() const { assert(hasSymbolicOffset()); return reinterpret_cast(static_cast(Data)); @@ -91,6 +104,7 @@ void Profile(llvm::FoldingSetNodeID& ID) const { ID.AddPointer(P.getOpaqueValue()); ID.AddInteger(Data); + ID.AddInteger(Extent); } static BindingKey Make(const MemRegion *R, Kind k); @@ -104,10 +118,15 @@ } bool operator==(const BindingKey &X) const { + if (CompareExtent && Extent != X.Extent) + return false; + return P.getOpaqueValue() == X.P.getOpaqueValue() && Data == X.Data; } + BindingKey compareWithExtent() const { return BindingKey(*this, true); } + LLVM_DUMP_METHOD void dump() const; }; } // end anonymous namespace @@ -115,9 +134,10 @@ BindingKey BindingKey::Make(const MemRegion *R, Kind k) { const RegionOffset &RO = R->getAsOffset(); if (RO.hasSymbolicOffset()) - return BindingKey(cast(R), cast(RO.getRegion()), k); + return BindingKey(cast(R), cast(RO.getRegion()), k, + R->getExtent()); - return BindingKey(RO.getRegion(), RO.getOffset(), k); + return BindingKey(RO.getRegion(), RO.getOffset(), k, R->getExtent()); } namespace llvm { @@ -130,6 +150,9 @@ else Out << "null"; + Out << "\", \"extent\": "; + Out << K.getExtent(); + return Out; } @@ -277,7 +300,8 @@ ClusterBindings Cluster = (ExistingCluster ? *ExistingCluster : CBFactory->getEmptyMap()); - ClusterBindings NewCluster = CBFactory->add(Cluster, K, V); + ClusterBindings NewCluster = + CBFactory->add(Cluster, K.compareWithExtent(), V); return add(Base, NewCluster); } @@ -292,7 +316,16 @@ const ClusterBindings *Cluster = lookup(K.getBaseRegion()); if (!Cluster) return nullptr; - return Cluster->lookup(K); + + // Try looking up the value. + const auto *ResVal = Cluster->lookup(K.compareWithExtent()); + + // If we fail to find a value, assume there have been a cast and + // try to look it up again without comparing the region extent. + if (!ResVal) + ResVal = Cluster->lookup(K); + + return ResVal; } const SVal *RegionBindingsRef::lookup(const MemRegion *R, Index: clang/test/Analysis/copy-elision.cpp =================================================================== --- clang/test/Analysis/copy-elision.cpp +++ clang/test/Analysis/copy-elision.cpp @@ -57,13 +57,8 @@ C() : t(T(4)) { S s = {1, 2, 3}; t.s = s; - // FIXME: Should be TRUE regardless of copy elision. clang_analyzer_eval(t.w == 4); -#ifdef ELIDE - // expected-warning@-2{{TRUE}} -#else - // expected-warning@-4{{UNKNOWN}} -#endif + // expected-warning@-1{{TRUE}} } }; Index: clang/test/Analysis/store-extent-rework.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/store-extent-rework.cpp @@ -0,0 +1,41 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection,alpha.unix.cstring -verify %s + +void clang_analyzer_eval(bool); + +typedef __typeof__(sizeof(int)) size_t; +void *memset(void *dest, int ch, size_t count); + +struct Point +{ + int x; + int y; + int z; +}; + +struct Circle +{ + Point origin; + int size; +}; + +Point makePoint(int x, int y) +{ + Point result; + result.x = x; + result.y = y; + result.z = 3; + return result; +} + +void multipleDefaultOnStack() { + Circle testObj; + memset(&testObj, 0, sizeof(testObj)); + + clang_analyzer_eval(testObj.size == 0); //expected-warning{{TRUE}} + testObj.origin = makePoint(1, 2); + clang_analyzer_eval(testObj.size == 0); //expected-warning{{TRUE}} + + clang_analyzer_eval(testObj.origin.x == 1); //expected-warning{{TRUE}} + clang_analyzer_eval(testObj.origin.y == 2); //expected-warning{{TRUE}} + clang_analyzer_eval(testObj.origin.z == 3); //expected-warning{{TRUE}} +} Index: clang/test/Analysis/uninit-vals.m =================================================================== --- clang/test/Analysis/uninit-vals.m +++ clang/test/Analysis/uninit-vals.m @@ -164,15 +164,11 @@ // expected-note@-1{{TRUE}} testObj->origin = makePoint(0.0, 0.0); - if (testObj->size > 0) { ; } // expected-note{{Assuming field 'size' is <= 0}} + if (testObj->size > 0) { ; } // expected-note{{Field 'size' is <= 0}} // expected-note@-1{{Taking false branch}} - // FIXME: Assigning to 'testObj->origin' kills the default binding for the - // whole region, meaning that we've forgotten that testObj->size should also - // default to 0. Tracked by . - // This should be TRUE. - clang_analyzer_eval(testObj->size == 0); // expected-warning{{UNKNOWN}} - // expected-note@-1{{UNKNOWN}} + clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} free(testObj); } @@ -219,21 +215,17 @@ // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint(1, 2); - if (testObj->size > 0) { ; } // expected-note{{Assuming field 'size' is <= 0}} + if (testObj->size > 0) { ; } // expected-note{{Field 'size' is <= 0}} // expected-note@-1{{Taking false branch}} - // expected-note@-2{{Assuming field 'size' is <= 0}} + // expected-note@-2{{Field 'size' is <= 0}} // expected-note@-3{{Taking false branch}} - // expected-note@-4{{Assuming field 'size' is <= 0}} + // expected-note@-4{{Field 'size' is <= 0}} // expected-note@-5{{Taking false branch}} - // expected-note@-6{{Assuming field 'size' is <= 0}} + // expected-note@-6{{Field 'size' is <= 0}} // expected-note@-7{{Taking false branch}} - // FIXME: Assigning to 'testObj->origin' kills the default binding for the - // whole region, meaning that we've forgotten that testObj->size should also - // default to 0. Tracked by . - // This should be TRUE. - clang_analyzer_eval(testObj->size == 0); // expected-warning{{UNKNOWN}} - // expected-note@-1{{UNKNOWN}} + clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.x == 1); // expected-warning{{TRUE}} // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 2); // expected-warning{{TRUE}}