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
