[cfe-commits] [PATCH] VisitInitListExpr

[Ted Kremenek]

On Oct 29, 2008, at 6:35 PM, Zhongxing Xu wrote:

> This patch implements GRExprEngine::VisitInitListExpr().  
> CompoundValsData is added to BasicValue classes. I found it can  
> represent recursive struct compound value, so we do not need to  
> flatten struct init list values. (not implemented in this patch).
>
> A full visit is done on each initializer bacause initializer can  
> contain arbitrary expressions. But no state splitting is allowed.  
> Reasons are: state splitting is rare in initializers, and  
> implementing state splitting is not easy when visiting a unknown  
> length sequence of expressions.
>
> I post this patch for review since it involves some heavy design.  
> Thanks.

This is a big patch, so I think we should divide it into one patch  
that adds nonloc:: CompoundVal (and CompundSValData) and the other  
that modifies GRExprEngine.  I'll also review these pieces in separate  
emails.  First for nonloc::CompoundVal:

Index: include/clang/Analysis/PathSensitive/BasicValueFactory.h
===================================================================
--- include/clang/Analysis/PathSensitive/BasicValueFactory.h	（版本  
58411）
+++ include/clang/Analysis/PathSensitive/BasicValueFactory.h	（工作 
副本）
@@ -28,7 +28,25 @@
  namespace clang {

  class SVal;
-
+
+class CompoundSValData : public llvm::FoldingSetNode {
+  QualType T;
+  unsigned NumSVals;
+  SVal* SVals;
+
+public:
+  CompoundSValData(QualType t, unsigned n, const SVal* Vals);
+
+  ~CompoundSValData();
+
+  static void Profile(llvm::FoldingSetNodeID& ID, QualType T,  
unsigned N,
+                      const SVal* Vals);
+
+  void Profile(llvm::FoldingSetNodeID& ID) {
+    Profile(ID, T, NumSVals, SVals);
+  }
+};

Looks good.  This should probably be named "CompoundValData" instead  
of CompoundSValData just to be consistent with nonloc:: CompoundVal.   
Also, we'll probably need accessories or iterators for iterating over  
the values of CompundSValData, but that should come in a second patch.

+
  class BasicValueFactory {
    typedef llvm::FoldingSet<llvm::FoldingSetNodeWrapper<llvm::APSInt> >
            APSIntSetTy;
@@ -45,6 +63,8 @@
    void*         PersistentSVals;
    void*         PersistentSValPairs;

+  llvm::FoldingSet<CompoundSValData> CompoundSValDataSet;
+
  public:
    BasicValueFactory(ASTContext& ctx, llvm::BumpPtrAllocator& Alloc)
    : Ctx(ctx), BPAlloc(Alloc), PersistentSVals(0),  
PersistentSValPairs(0) {}
@@ -68,6 +88,9 @@
    const SymIntConstraint& getConstraint(SymbolID sym,  
BinaryOperator::Opcode Op,
                                          const llvm::APSInt& V);

+  const CompoundSValData* getCompoundSValData(QualType T, unsigned  
NumSVals,
+                                              const SVal* SVals);
+

Looks good.


Index: include/clang/Analysis/PathSensitive/SVals.h
===================================================================
--- include/clang/Analysis/PathSensitive/SVals.h	（版本 58411）
+++ include/clang/Analysis/PathSensitive/SVals.h	（工作副本）
@@ -45,6 +45,7 @@
      : Data(D), Kind(k) {}

nLoc MakeIntTruthVal(BasicValueFactory& BasicVals, bool b);
-
+
+  static NonLoc MakeCompoundVal(QualType T, unsigned NumSVals, SVal*  
BegV,
+                                BasicValueFactory& BasicVals);
+

Factory methods like this are great, and should be the *only* way we  
create CompoundVals.  I would switch BegV and NumSVals around, since I  
think the tendency is to provide the buffer first and the size second.


+class CompoundVal : public NonLoc {
+public:
+
+  CompoundVal(const CompoundSValData* D) : NonLoc(CompoundValKind, D)  
{}

I'd make the constructor private, and add friendship to NonLoc so that  
MakeCompoundVal is the only way to construct these objects.

+
+  const CompoundSValData* getValue() {
+    return static_cast<CompoundSValData*>(Data);
+  }
+
+  static bool classof(const SVal* V) {
+    return V->getBaseKind() == NonLocKind && V->getSubKind() ==  
CompoundValKind;
+  }
+
+  static bool classof(const NonLoc* V) {
+    return V->getSubKind() == CompoundValKind;
+  }
+};

Looks good.  Later we'll add an iterator interface for iterating over  
the values.


Index: lib/Analysis/SVals.cpp
===================================================================
--- lib/Analysis/SVals.cpp	（版本 58411）
+++ lib/Analysis/SVals.cpp	（工作副本）
@@ -245,6 +245,11 @@
    return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
  }

+NonLoc NonLoc::MakeCompoundVal(QualType T, unsigned NumSVals, SVal*  
BegV,
+                               BasicValueFactory& BasicVals) {
+  return nonloc::CompoundVal(BasicVals.getCompoundSValData(T,  
NumSVals, BegV));
+}
+
  SVal SVal::GetSymbolValue(SymbolManager& SymMgr, VarDecl* D) {

    QualType T = D->getType();
Index: lib/Analysis/BasicValueFactory.cpp
===================================================================
--- lib/Analysis/BasicValueFactory.cpp	（版本 58411）
+++ lib/Analysis/BasicValueFactory.cpp	（工作副本）
@@ -18,6 +18,25 @@

  using namespace clang;

+CompoundSValData::CompoundSValData(QualType t, unsigned n, const  
SVal* Vals)
+  : T(t), NumSVals(n) {
+  SVals = new SVal[n];
+  for (unsigned i = 0; i < n; ++i)
+    SVals[i] = Vals[i];
+}

Since BasicValueFactory creates and manages CompoundValData objects,  
let's allocate the array of SVals using the BumpPtrAllocator (i.e.,  
use a placement new).  Right now when the FoldingSets in BasicVals are  
destroyed we don't actually call delete on of its constituent objects  
(since they are all allocated from the BumpPtrAllocator).  Just pass  
the BumpPtrAllocator in the constructor.

+
+CompoundSValData::~CompoundSValData() {
+  delete[] SVals;
+}

No need for this destructor if we allocate using the BumpPtrAllocator.

+
+void CompoundSValData::Profile(llvm::FoldingSetNodeID& ID, QualType T,
+                               unsigned N, const SVal* Vals) {
+  T.Profile(ID);
+  ID.AddInteger(N);
+  for (unsigned i = 0; i < N; ++i)
+    Vals[i].Profile(ID);
+}

You need the QualType in the profile as well, otherwise you might get  
a collision.


+const CompoundSValData*
+BasicValueFactory::getCompoundSValData(QualType T, unsigned NumSVals,
+                                       const SVal* SVals) {
+  llvm::FoldingSetNodeID ID;
+  CompoundSValData::Profile(ID, T, NumSVals, SVals);
+  void* InsertPos;
+
+  CompoundSValData* D = CompoundSValDataSet.FindNodeOrInsertPos(ID,  
InsertPos);
+
+  if (!D) {
+    D = (CompoundSValData*) BPAlloc.Allocate<CompoundSValData>();
+    new (D) CompoundSValData(T, NumSVals, SVals);
+    CompoundSValDataSet.InsertNode(D, InsertPos);
+  }

Looks great.  Just pass BPAlloc down to the ctor for CompoundSValData.