[cfe-commits] r123923 - in /cfe/trunk: lib/Analysis/UninitializedValuesV2.cpp test/Sema/uninit-variables.c
Ted Kremenek
kremenek at apple.com
Thu Jan 20 09:37:17 PST 2011
Author: kremenek
Date: Thu Jan 20 11:37:17 2011
New Revision: 123923
URL: http://llvm.org/viewvc/llvm-project?rev=123923&view=rev
Log:
Add rudimentary path-sensitivity to UnintializedValuesV2
analysis for short-circuited operations. For branch written like "if (x && y)",
we maintain two sets of dataflow values for the outgoing
branches. This suppresses some common false positives
for -Wuninitialized-experimental.
This change introduces some assertion failures
when running on the LLVM codebase. WIP.
Modified:
cfe/trunk/lib/Analysis/UninitializedValuesV2.cpp
cfe/trunk/test/Sema/uninit-variables.c
Modified: cfe/trunk/lib/Analysis/UninitializedValuesV2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/UninitializedValuesV2.cpp?rev=123923&r1=123922&r2=123923&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/UninitializedValuesV2.cpp (original)
+++ cfe/trunk/lib/Analysis/UninitializedValuesV2.cpp Thu Jan 20 11:37:17 2011
@@ -11,6 +11,7 @@
//
//===----------------------------------------------------------------------===//
+#include <utility>
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/BitVector.h"
@@ -71,26 +72,39 @@
// CFGBlockValues: dataflow values for CFG blocks.
//====------------------------------------------------------------------------//
+typedef std::pair<llvm::BitVector *, llvm::BitVector *> BVPair;
+
namespace {
class CFGBlockValues {
const CFG &cfg;
- llvm::BitVector **vals;
+ BVPair *vals;
llvm::BitVector scratch;
DeclToBit declToBit;
+
+ llvm::BitVector &lazyCreate(llvm::BitVector *&bv);
public:
CFGBlockValues(const CFG &cfg);
~CFGBlockValues();
void computeSetOfDeclarations(const DeclContext &dc);
- llvm::BitVector &getBitVector(const CFGBlock *block);
+ llvm::BitVector &getBitVector(const CFGBlock *block,
+ const CFGBlock *dstBlock);
+
+ BVPair &getBitVectors(const CFGBlock *block);
+
+ BVPair getPredBitVectors(const CFGBlock *block);
+
void mergeIntoScratch(llvm::BitVector const &source, bool isFirst);
bool updateBitVectorWithScratch(const CFGBlock *block);
+ bool updateBitVectors(const CFGBlock *block, const BVPair &newVals);
bool hasNoDeclarations() const {
return declToBit.size() == 0;
}
void resetScratch();
+ llvm::BitVector &getScratch() { return scratch; }
+
llvm::BitVector::reference operator[](const VarDecl *vd);
};
}
@@ -99,7 +113,7 @@
unsigned n = cfg.getNumBlockIDs();
if (!n)
return;
- vals = new llvm::BitVector*[n];
+ vals = new std::pair<llvm::BitVector*, llvm::BitVector*>[n];
memset(vals, 0, sizeof(*vals) * n);
}
@@ -107,8 +121,10 @@
unsigned n = cfg.getNumBlockIDs();
if (n == 0)
return;
- for (unsigned i = 0; i < n; ++i)
- delete vals[i];
+ for (unsigned i = 0; i < n; ++i) {
+ delete vals[i].first;
+ delete vals[i].second;
+ }
delete [] vals;
}
@@ -117,16 +133,71 @@
scratch.resize(declToBit.size());
}
-llvm::BitVector &CFGBlockValues::getBitVector(const CFGBlock *block) {
- unsigned idx = block->getBlockID();
- llvm::BitVector *bv = vals[idx];
- if (!bv) {
+llvm::BitVector &CFGBlockValues::lazyCreate(llvm::BitVector *&bv) {
+ if (!bv)
bv = new llvm::BitVector(declToBit.size());
- vals[idx] = bv;
- }
return *bv;
}
+/// This function pattern matches for a '&&' or '||' that appears at
+/// the beginning of a CFGBlock that also (1) has a terminator and
+/// (2) has no other elements. If such an expression is found, it is returned.
+static BinaryOperator *getLogicalOperatorInChain(const CFGBlock *block) {
+ if (block->empty())
+ return 0;
+
+ CFGStmt cstmt = block->front().getAs<CFGStmt>();
+ BinaryOperator *b = llvm::dyn_cast_or_null<BinaryOperator>(cstmt.getStmt());
+ if (!b || !b->isLogicalOp() || block->getTerminatorCondition() != b)
+ return 0;
+ return b;
+}
+
+llvm::BitVector &CFGBlockValues::getBitVector(const CFGBlock *block,
+ const CFGBlock *dstBlock) {
+ unsigned idx = block->getBlockID();
+ if (dstBlock && block->succ_size() == 2) {
+ assert(block->getTerminator());
+ if (getLogicalOperatorInChain(block)) {
+ if (*block->succ_begin() == dstBlock)
+ return lazyCreate(vals[idx].first);
+ assert(*(block->succ_begin()+1) == dstBlock);
+ return lazyCreate(vals[idx].second);
+ }
+ }
+
+ assert(vals[idx].second == 0);
+ return lazyCreate(vals[idx].first);
+}
+
+BVPair &CFGBlockValues::getBitVectors(const clang::CFGBlock *block) {
+ unsigned idx = block->getBlockID();
+ lazyCreate(vals[idx].first);
+ lazyCreate(vals[idx].second);
+ return vals[idx];
+}
+
+BVPair CFGBlockValues::getPredBitVectors(const clang::CFGBlock *block) {
+ assert(block->pred_size() == 2);
+ CFGBlock::const_pred_iterator itr = block->pred_begin();
+ llvm::BitVector &bvA = getBitVector(*itr, block);
+ ++itr;
+ return BVPair(&bvA, &getBitVector(*itr, block));
+}
+
+
+static void printVector(const CFGBlock *block, llvm::BitVector &bv,
+ unsigned num) {
+
+ #if 0
+ llvm::errs() << block->getBlockID() << " :";
+ for (unsigned i = 0; i < bv.size(); ++i) {
+ llvm::errs() << ' ' << bv[i];
+ }
+ llvm::errs() << " : " << num << '\n';
+ #endif
+}
+
void CFGBlockValues::mergeIntoScratch(llvm::BitVector const &source,
bool isFirst) {
if (isFirst)
@@ -136,10 +207,24 @@
}
bool CFGBlockValues::updateBitVectorWithScratch(const CFGBlock *block) {
- llvm::BitVector &dst = getBitVector(block);
+ llvm::BitVector &dst = getBitVector(block, 0);
bool changed = (dst != scratch);
if (changed)
dst = scratch;
+
+ printVector(block, scratch, 0);
+ return changed;
+}
+
+bool CFGBlockValues::updateBitVectors(const CFGBlock *block,
+ const BVPair &newVals) {
+ BVPair &vals = getBitVectors(block);
+ bool changed = *newVals.first != *vals.first ||
+ *newVals.second != *vals.second;
+ *vals.first = *newVals.first;
+ *vals.second = *newVals.second;
+ printVector(block, *vals.first, 1);
+ printVector(block, *vals.second, 2);
return changed;
}
@@ -370,15 +455,33 @@
// High-level "driver" logic for uninitialized values analysis.
//====------------------------------------------------------------------------//
-static void runOnBlock(const CFGBlock *block, const CFG &cfg,
+static bool runOnBlock(const CFGBlock *block, const CFG &cfg,
CFGBlockValues &vals,
UninitVariablesHandler *handler = 0) {
- // Merge in values of predecessor blocks.
+
+ if (const BinaryOperator *b = getLogicalOperatorInChain(block)) {
+ assert(block->pred_size() == 2);
+ assert(block->succ_size() == 2);
+ assert(block->getTerminatorCondition() == b);
+
+ BVPair valsAB = vals.getPredBitVectors(block);
+ vals.mergeIntoScratch(*valsAB.first, true);
+ vals.mergeIntoScratch(*valsAB.second, false);
+ valsAB.second = &vals.getScratch();
+ if (b->getOpcode() == BO_LOr) {
+ // Ensure the invariant that 'first' corresponds to the true
+ // branch and 'second' to the false.
+ std::swap(valsAB.first, valsAB.second);
+ }
+ return vals.updateBitVectors(block, valsAB);
+ }
+
+ // Default behavior: merge in values of predecessor blocks.
vals.resetScratch();
bool isFirst = true;
for (CFGBlock::const_pred_iterator I = block->pred_begin(),
E = block->pred_end(); I != E; ++I) {
- vals.mergeIntoScratch(vals.getBitVector(*I), isFirst);
+ vals.mergeIntoScratch(vals.getBitVector(*I, block), isFirst);
isFirst = false;
}
// Apply the transfer function.
@@ -389,6 +492,7 @@
tf.BlockStmt_Visit(cs->getStmt());
}
}
+ return vals.updateBitVectorWithScratch(block);
}
void clang::runUninitializedVariablesAnalysis(const DeclContext &dc,
@@ -404,9 +508,8 @@
worklist.enqueueSuccessors(&cfg.getEntry());
while (const CFGBlock *block = worklist.dequeue()) {
- runOnBlock(block, cfg, vals);
// Did the block change?
- bool changed = vals.updateBitVectorWithScratch(block);
+ bool changed = runOnBlock(block, cfg, vals);
if (changed || !previouslyVisited[block->getBlockID()])
worklist.enqueueSuccessors(block);
previouslyVisited[block->getBlockID()] = true;
Modified: cfe/trunk/test/Sema/uninit-variables.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/uninit-variables.c?rev=123923&r1=123922&r2=123923&view=diff
==============================================================================
--- cfe/trunk/test/Sema/uninit-variables.c (original)
+++ cfe/trunk/test/Sema/uninit-variables.c Thu Jan 20 11:37:17 2011
@@ -110,4 +110,63 @@
*x = 1; // expected-warning{{use of uninitialized variable 'x'}}
*x = 1; // no-warning
}
-
+
+int test18(int x, int y) {
+ int z;
+ if (x && y && (z = 1)) {
+ return z; // no-warning
+ }
+ return 0;
+}
+
+int test19_aux1();
+int test19_aux2();
+int test19_aux3(int *x);
+int test19() {
+ int z;
+ if (test19_aux1() + test19_aux2() && test19_aux1() && test19_aux3(&z))
+ return z; // no-warning
+ return 0;
+}
+
+int test20() {
+ int z;
+ if ((test19_aux1() + test19_aux2() && test19_aux1()) || test19_aux3(&z))
+ return z; // expected-warning{{use of uninitialized variable 'z'}}
+ return 0;
+}
+
+int test21(int x, int y) {
+ int z;
+ if ((x && y) || test19_aux3(&z) || test19_aux2())
+ return z; // expected-warning{{use of uninitialized variable 'z'}}
+ return 0;
+}
+
+int test22() {
+ int z;
+ while (test19_aux1() + test19_aux2() && test19_aux1() && test19_aux3(&z))
+ return z; // no-warning
+ return 0;
+}
+
+int test23() {
+ int z;
+ for ( ; test19_aux1() + test19_aux2() && test19_aux1() && test19_aux3(&z) ; )
+ return z; // no-warning
+ return 0;
+}
+
+// The basic uninitialized value analysis doesn't have enough path-sensitivity
+// to catch initializations relying on control-dependencies spanning multiple
+// conditionals. This possibly can be handled by making the CFG itself
+// represent such control-dependencies, but it is a niche case.
+int test24(int flag) {
+ unsigned val;
+ if (flag)
+ val = 1;
+ if (!flag)
+ val = 1;
+ return val; // expected-warning{{use of uninitialized variable 'val'}}
+}
+
More information about the cfe-commits
mailing list