NoQ created this revision. NoQ added reviewers: zaks.anna, krememek. NoQ added subscribers: cfe-commits, dergachev.a.
In Clang Static Analyzer, when the symbol is referenced by an index value of an element region, it does not prevent garbage collection (reaping) of that symbol. Hence, if the element index value is the only place where the symbol is stored, the symbol gets reaped, and range information is removed from the constraint manager. It seems that both the store and the environment need to mark the element indices of their regions as live. http://reviews.llvm.org/D12726 Files: lib/StaticAnalyzer/Core/Environment.cpp lib/StaticAnalyzer/Core/RegionStore.cpp test/Analysis/return-ptr-range.cpp Index: test/Analysis/return-ptr-range.cpp =================================================================== --- test/Analysis/return-ptr-range.cpp +++ test/Analysis/return-ptr-range.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.ReturnPtrRange -verify %s + +int arr[10]; +int *ptr; + +int conjure_index(); + +int *test_element_index_lifetime() { + do { + int x = conjure_index(); + ptr = arr + x; + if (x != 20) + return arr; // no-warning + } while (0); + return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}} +} + +int *test_element_index_lifetime_with_local_ptr() { + int *local_ptr; + do { + int x = conjure_index(); + local_ptr = arr + x; + if (x != 20) + return arr; // no-warning + } while (0); + return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}} +} Index: lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- lib/StaticAnalyzer/Core/RegionStore.cpp +++ lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2260,6 +2260,16 @@ if (const MemRegion *R = V.getAsRegion()) { AddToWorkList(R); + // Element index of an element region is live. + if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) { + SVal Idx = ER->getIndex(); + for (SymExpr::symbol_iterator SI = Idx.symbol_begin(), + SE = Idx.symbol_end(); + SI != SE; ++SI) { + SymReaper.markLive(*SI); + } + } + // All regions captured by a block are also live. if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) { BlockDataRegion::referenced_vars_iterator I = BR->referenced_vars_begin(), Index: lib/StaticAnalyzer/Core/Environment.cpp =================================================================== --- lib/StaticAnalyzer/Core/Environment.cpp +++ lib/StaticAnalyzer/Core/Environment.cpp @@ -171,8 +171,15 @@ EBMapRef = EBMapRef.add(BlkExpr, X); // If the block expr's value is a memory region, then mark that region. - if (Optional<loc::MemRegionVal> R = X.getAs<loc::MemRegionVal>()) - SymReaper.markLive(R->getRegion()); + if (Optional<loc::MemRegionVal> R = X.getAs<loc::MemRegionVal>()) { + const MemRegion *MR = R->getRegion(); + SymReaper.markLive(MR); + + // Mark the element index as live. + if (const ElementRegion *ER = dyn_cast<ElementRegion>(MR)) + if (SymbolRef Sym = ER->getIndex().getAsSymbol()) + SymReaper.markLive(Sym); + } // Mark all symbols in the block expr's value live. RSScaner.scan(X);
Index: test/Analysis/return-ptr-range.cpp =================================================================== --- test/Analysis/return-ptr-range.cpp +++ test/Analysis/return-ptr-range.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.ReturnPtrRange -verify %s + +int arr[10]; +int *ptr; + +int conjure_index(); + +int *test_element_index_lifetime() { + do { + int x = conjure_index(); + ptr = arr + x; + if (x != 20) + return arr; // no-warning + } while (0); + return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}} +} + +int *test_element_index_lifetime_with_local_ptr() { + int *local_ptr; + do { + int x = conjure_index(); + local_ptr = arr + x; + if (x != 20) + return arr; // no-warning + } while (0); + return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}} +} Index: lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- lib/StaticAnalyzer/Core/RegionStore.cpp +++ lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2260,6 +2260,16 @@ if (const MemRegion *R = V.getAsRegion()) { AddToWorkList(R); + // Element index of an element region is live. + if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) { + SVal Idx = ER->getIndex(); + for (SymExpr::symbol_iterator SI = Idx.symbol_begin(), + SE = Idx.symbol_end(); + SI != SE; ++SI) { + SymReaper.markLive(*SI); + } + } + // All regions captured by a block are also live. if (const BlockDataRegion *BR = dyn_cast<BlockDataRegion>(R)) { BlockDataRegion::referenced_vars_iterator I = BR->referenced_vars_begin(), Index: lib/StaticAnalyzer/Core/Environment.cpp =================================================================== --- lib/StaticAnalyzer/Core/Environment.cpp +++ lib/StaticAnalyzer/Core/Environment.cpp @@ -171,8 +171,15 @@ EBMapRef = EBMapRef.add(BlkExpr, X); // If the block expr's value is a memory region, then mark that region. - if (Optional<loc::MemRegionVal> R = X.getAs<loc::MemRegionVal>()) - SymReaper.markLive(R->getRegion()); + if (Optional<loc::MemRegionVal> R = X.getAs<loc::MemRegionVal>()) { + const MemRegion *MR = R->getRegion(); + SymReaper.markLive(MR); + + // Mark the element index as live. + if (const ElementRegion *ER = dyn_cast<ElementRegion>(MR)) + if (SymbolRef Sym = ER->getIndex().getAsSymbol()) + SymReaper.markLive(Sym); + } // Mark all symbols in the block expr's value live. RSScaner.scan(X);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits