[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

https://github.com/fangyi-zhou created 
https://github.com/llvm/llvm-project/pull/128251

Closes #57270.

This PR changes the `Stmt *` field in `SymbolConjured` with 
`CFGBlock::ConstCFGElementRef`. The motivation is that, when conjuring a 
symbol, there might not always be a statement available, causing information to 
be lost for conjured symbols, whereas the CFGElementRef can always be provided 
at the callsite.

Following the idea, this PR changes callsites of functions to create conjured 
symbols, and replaces them with appropriate `CFGElementRef`s.

>From 97c4e0e39ba5e9486e893691b40e78fe3d8548b0 Mon Sep 17 00:00:00 2001
From: Fangyi Zhou 
Date: Fri, 21 Feb 2025 20:54:08 +
Subject: [PATCH 1/3] WIP: use CFGElement in conjuredSymbol

---
 .../Core/PathSensitive/SValBuilder.h  | 87 +
 .../Core/PathSensitive/SymbolManager.h| 56 +++
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  | 18 ++--
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 55 ++-
 .../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 13 +--
 .../Core/ExprEngineCallAndReturn.cpp  |  6 +-
 .../StaticAnalyzer/Core/ExprEngineObjC.cpp| 16 ++--
 clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 93 +++
 8 files changed, 200 insertions(+), 144 deletions(-)

diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 54430d426a82a..5b2887b0f9a86 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -19,6 +19,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/Type.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
@@ -171,20 +172,27 @@ class SValBuilder {
 
   // Forwarding methods to SymbolManager.
 
-  const SymbolConjured* conjureSymbol(const Stmt *stmt,
+  const SymbolConjured *conjureSymbol(const CFGBlock::CFGElementRef ElemRef,
   const LocationContext *LCtx,
-  QualType type,
-  unsigned visitCount,
+  QualType type, unsigned visitCount,
   const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
+return SymMgr.conjureSymbol(ElemRef, LCtx, type, visitCount, symbolTag);
   }
 
-  const SymbolConjured* conjureSymbol(const Expr *expr,
-  const LocationContext *LCtx,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
-  }
+  // const SymbolConjured* conjureSymbol(const Stmt *stmt,
+  // const LocationContext *LCtx,
+  // QualType type,
+  // unsigned visitCount,
+  // const void *symbolTag = nullptr) {
+  //   return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
+  // }
+
+  // const SymbolConjured* conjureSymbol(const Expr *expr,
+  // const LocationContext *LCtx,
+  // unsigned visitCount,
+  // const void *symbolTag = nullptr) {
+  //   return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
+  // }
 
   /// Construct an SVal representing '0' for the specified type.
   DefinedOrUnknownSVal makeZeroVal(QualType type);
@@ -198,33 +206,38 @@ class SValBuilder {
   /// The advantage of symbols derived/built from other symbols is that we
   /// preserve the relation between related(or even equivalent) expressions, so
   /// conjured symbols should be used sparingly.
-  DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
-const Expr *expr,
-const LocationContext *LCtx,
-unsigned count);
-  DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag, const Stmt *S,
-const LocationContext *LCtx,
-QualType type, unsigned count);
-  DefinedOrUnknownSVal conjureSymbolVal(const Stmt *stmt,
-const LocationContext *LCtx,
-QualType type,
-unsigned visitCount);
-
-  /// Conjure a symbol representing heap allocated memory region.
-  ///
-  /// Note, the expression should represent a location.
-  DefinedSVal getConjuredHea

[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -1376,8 +1379,8 @@ StoreRef RegionStoreManager::invalidateRegions(
   }
 
   RegionBindingsRef B = getRegionBindings(store);
-  InvalidateRegionsWorker W(*this, StateMgr, B, S, Count, LCtx, IS, ITraits,
-Invalidated, GlobalsFilter);
+  InvalidateRegionsWorker W(*this, StateMgr, B, Call->getCFGElementRef(), 
Count,

fangyi-zhou wrote:

Q: Here `Call` is possibly null, how should I get a ref here?

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -171,20 +172,27 @@ class SValBuilder {
 
   // Forwarding methods to SymbolManager.
 
-  const SymbolConjured* conjureSymbol(const Stmt *stmt,
-  const LocationContext *LCtx,
-  QualType type,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
+  const SymbolConjured *
+  conjureSymbol(const CFGBlock::ConstCFGElementRef ElemRef,
+const LocationContext *LCtx, QualType type, unsigned 
visitCount,
+const void *symbolTag = nullptr) {
+return SymMgr.conjureSymbol(ElemRef, LCtx, type, visitCount, symbolTag);
   }
 
-  const SymbolConjured* conjureSymbol(const Expr *expr,
-  const LocationContext *LCtx,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
-  }
+  // const SymbolConjured* conjureSymbol(const Stmt *stmt,

fangyi-zhou wrote:

Will remove these commented out lines before marking this PR as ready.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -113,19 +120,21 @@ class SymbolConjured : public SymbolData {
 
   void dumpToStream(raw_ostream &os) const override;
 
-  static void Profile(llvm::FoldingSetNodeID &profile, const Stmt *S,
+  static void Profile(llvm::FoldingSetNodeID &profile,
+  const CFGBlock::ConstCFGElementRef ElemRef,
   const LocationContext *LCtx, QualType T, unsigned Count,
   const void *SymbolTag) {
 profile.AddInteger((unsigned)SymbolConjuredKind);
-profile.AddPointer(S);
+// profile.Add(ElemRef);

fangyi-zhou wrote:

Q: Do I need to add `ElemRef` to the profile?

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -867,18 +868,18 @@ ProgramStateRef createContainerBegin(ProgramStateRef 
State,
   return setContainerData(State, Cont, CData);
 }
 
-ProgramStateRef createContainerEnd(ProgramStateRef State, const MemRegion 
*Cont,
-   const Expr *E, QualType T,
-   const LocationContext *LCtx,
+ProgramStateRef createContainerEnd(CheckerContext &C, ProgramStateRef State,
+   const MemRegion *Cont, const Expr *E,
+   QualType T, const LocationContext *LCtx,

fangyi-zhou wrote:

Fixed in new commits.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -844,7 +845,7 @@ SymbolRef getContainerEnd(ProgramStateRef State, const 
MemRegion *Cont) {
   return CDataPtr->getEnd();
 }
 
-ProgramStateRef createContainerBegin(ProgramStateRef State,
+ProgramStateRef createContainerBegin(CheckerContext &C, ProgramStateRef State,
  const MemRegion *Cont, const Expr *E,
  QualType T, const LocationContext *LCtx,
  unsigned BlockCount) {

fangyi-zhou wrote:

Fixed in new commits.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -111,8 +111,13 @@ class SValExplainer : public 
FullSValVisitor {
   }
 
   std::string VisitSymbolConjured(const SymbolConjured *S) {
-return "symbol of type '" + S->getType().getAsString() +
-   "' conjured at statement '" + printStmt(S->getStmt()) + "'";
+std::string Str;
+llvm::raw_string_ostream OS(Str);
+OS << "symbol of type '" + S->getType().getAsString() +
+  "' conjured at statement '";
+S->getCFGElementRef()->dumpToStream(OS);

fangyi-zhou wrote:

`printStmt()` delegates to `Stmt::printPretty()`, but I don't immediately see 
any equivalent for `CFGElementRef`. The slightly annoying thing about 
`dumpToStream` for `CFGElementRef` is that it has an newline character in the 
end...

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

https://github.com/fangyi-zhou ready_for_review 
https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

fangyi-zhou wrote:

I've made some more progress, the crash goes away, there are still some review 
comments that I need to address, which I'll try to complete later.

```
/home/fangyi/playground/bug.cc:21:5: warning: value derived from (symbol of 
type 'int' conjured at statement '->~S() (Implicit destructor)
') for global variable 'S::a' [debug.ExprInspection]
   21 | clang_analyzer_explain(S::a);
  | ^~~~
1 warning generated.
```

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -111,8 +111,13 @@ class SValExplainer : public 
FullSValVisitor {
   }
 
   std::string VisitSymbolConjured(const SymbolConjured *S) {
-return "symbol of type '" + S->getType().getAsString() +
-   "' conjured at statement '" + printStmt(S->getStmt()) + "'";
+std::string Str;
+llvm::raw_string_ostream OS(Str);
+OS << "symbol of type '" + S->getType().getAsString() +
+  "' conjured at statement '";
+S->getCFGElementRef()->dumpToStream(OS);

fangyi-zhou wrote:

Q: Do we have a way to pretty print a CFGElementRef instead of `dumpToStream`?

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -151,72 +151,63 @@ SValBuilder::getRegionValueSymbolVal(const 
TypedValueRegion *region) {
   return nonloc::SymbolVal(sym);
 }
 
-DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *SymbolTag,
-   const Expr *Ex,
-   const LocationContext *LCtx,
-   unsigned Count) {
-  QualType T = Ex->getType();
-
-  if (T->isNullPtrType())
-return makeZeroVal(T);
-
-  // Compute the type of the result. If the expression is not an R-value, the
-  // result should be a location.
-  QualType ExType = Ex->getType();
-  if (Ex->isGLValue())
-T = LCtx->getAnalysisDeclContext()->getASTContext().getPointerType(ExType);
-
-  return conjureSymbolVal(SymbolTag, Ex, LCtx, T, Count);
-}
+// DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *SymbolTag,

fangyi-zhou wrote:

Note to self: This should probably be added back in some shape or form since it 
has some different behaviour when handling expressions with a provided type --- 
which means it's not entirely same to calling `conjureSymbolVal(SymbolTag, Ex, 
LCtx, Ex->getType(), Count)`. Probably need to revisit all callsites and check.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

https://github.com/fangyi-zhou edited 
https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -494,7 +494,7 @@ void IteratorModeling::handleComparison(CheckerContext &C, 
const Expr *CE,
 auto &SymMgr = C.getSymbolManager();
 auto *LCtx = C.getLocationContext();
 RetVal = nonloc::SymbolVal(SymMgr.conjureSymbol(
-CE, LCtx, C.getASTContext().BoolTy, C.blockCount()));
+C.getCFGElementRef(), LCtx, C.getASTContext().BoolTy, C.blockCount()));
 State = State->BindExpr(CE, LCtx, RetVal);

fangyi-zhou wrote:

Used here.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -27,7 +27,8 @@ namespace ento {
 /// by the loop body in any iteration.
 ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
 const LocationContext *LCtx,
-unsigned BlockCount, const Stmt *LoopStmt);
+unsigned BlockCount, const Stmt *LoopStmt,

fangyi-zhou wrote:

Not any more, will remove.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -171,19 +172,11 @@ class SValBuilder {
 
   // Forwarding methods to SymbolManager.
 
-  const SymbolConjured* conjureSymbol(const Stmt *stmt,
-  const LocationContext *LCtx,
-  QualType type,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
-  }
-
-  const SymbolConjured* conjureSymbol(const Expr *expr,
-  const LocationContext *LCtx,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
+  const SymbolConjured *
+  conjureSymbol(const CFGBlock::ConstCFGElementRef ElemRef,

fangyi-zhou wrote:

We can try to downcast the `ElemRef` for a statement, and try to downcast as an 
expression to obtain the type in those cases. I guess it would work in the 
callsites in the deleted overload if the `ElemRef` are statements.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -171,19 +172,11 @@ class SValBuilder {
 
   // Forwarding methods to SymbolManager.
 
-  const SymbolConjured* conjureSymbol(const Stmt *stmt,
-  const LocationContext *LCtx,
-  QualType type,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
-  }
-
-  const SymbolConjured* conjureSymbol(const Expr *expr,
-  const LocationContext *LCtx,
-  unsigned visitCount,
-  const void *symbolTag = nullptr) {
-return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
+  const SymbolConjured *
+  conjureSymbol(const CFGBlock::ConstCFGElementRef ElemRef,

fangyi-zhou wrote:

The type needs to come from the expression, which we don't pass in as an 
argument any more.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -198,32 +191,24 @@ class SValBuilder {
   /// The advantage of symbols derived/built from other symbols is that we
   /// preserve the relation between related(or even equivalent) expressions, so
   /// conjured symbols should be used sparingly.
-  DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
-const Expr *expr,
-const LocationContext *LCtx,
-unsigned count);
-  DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag, const Stmt *S,
-const LocationContext *LCtx,
-QualType type, unsigned count);
-  DefinedOrUnknownSVal conjureSymbolVal(const Stmt *stmt,
-const LocationContext *LCtx,
-QualType type,
-unsigned visitCount);
+  DefinedOrUnknownSVal
+  conjureSymbolVal(const void *symbolTag, const Expr *expr,
+   const CFGBlock::ConstCFGElementRef elemRef,
+   const LocationContext *LCtx, unsigned count);
+  DefinedOrUnknownSVal
+  conjureSymbolVal(const void *symbolTag,
+   const CFGBlock::ConstCFGElementRef elemRef,
+   const LocationContext *LCtx, QualType type, unsigned count);
+  DefinedOrUnknownSVal
+  conjureSymbolVal(const CFGBlock::ConstCFGElementRef elemRef,
+   const LocationContext *LCtx, QualType type,
+   unsigned visitCount);
 
   /// Conjure a symbol representing heap allocated memory region.
-  ///
-  /// Note, the expression should represent a location.
-  DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
-   const LocationContext *LCtx,
-   unsigned Count);
-
-  /// Conjure a symbol representing heap allocated memory region.
-  ///
-  /// Note, now, the expression *doesn't* need to represent a location.
-  /// But the type need to!
-  DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
-   const LocationContext *LCtx,
-   QualType type, unsigned Count);
+  DefinedSVal
+  getConjuredHeapSymbolVal(const CFGBlock::ConstCFGElementRef elemRef,

fangyi-zhou wrote:

Same as above.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -533,18 +538,12 @@ class SymbolManager {
   template 
   const SymExprT *acquire(Args &&...args);
 
-  const SymbolConjured *conjureSymbol(const Stmt *E,
-  const LocationContext *LCtx, QualType T,
-  unsigned VisitCount,
-  const void *SymbolTag = nullptr) {
-return acquire(E, LCtx, T, VisitCount, SymbolTag);
-  }
+  const SymbolConjured *
+  conjureSymbol(const CFGBlock::ConstCFGElementRef ElemRef,

fangyi-zhou wrote:

Same as above.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -1515,7 +1515,8 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, 
const CallEvent &Call,
   // conjure a return value for later.
   if (lastElement.isUnknown())
 lastElement = C.getSValBuilder().conjureSymbolVal(
-nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+nullptr, Call.getOriginExpr(), C.getCFGElementRef(), LCtx,

fangyi-zhou wrote:

I don't know what's the correct one to pass and don't have a way to check 
reliably since I don't understand the static analysis code. I don't understand 
your intention in this comment, do you mean I should pass the CFGElementRef to 
the Call or not?

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -198,32 +191,24 @@ class SValBuilder {
   /// The advantage of symbols derived/built from other symbols is that we
   /// preserve the relation between related(or even equivalent) expressions, so
   /// conjured symbols should be used sparingly.
-  DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
-const Expr *expr,
-const LocationContext *LCtx,
-unsigned count);
-  DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag, const Stmt *S,
-const LocationContext *LCtx,
-QualType type, unsigned count);
-  DefinedOrUnknownSVal conjureSymbolVal(const Stmt *stmt,
-const LocationContext *LCtx,
-QualType type,
-unsigned visitCount);
+  DefinedOrUnknownSVal
+  conjureSymbolVal(const void *symbolTag, const Expr *expr,
+   const CFGBlock::ConstCFGElementRef elemRef,

fangyi-zhou wrote:

If you look at the implementation code, there's some special treatment wrt the 
type of the expression

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -1376,8 +1379,8 @@ StoreRef RegionStoreManager::invalidateRegions(
   }
 
   RegionBindingsRef B = getRegionBindings(store);
-  InvalidateRegionsWorker W(*this, StateMgr, B, S, Count, LCtx, IS, ITraits,
-Invalidated, GlobalsFilter);
+  InvalidateRegionsWorker W(*this, StateMgr, B, Call->getCFGElementRef(), 
Count,

fangyi-zhou wrote:

Are you referring to this instance that I should pass the refernece to the 
statement instead of the call? I think I misunderstood this comment so I 
changed the references to not use the one from the call in all places.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -151,10 +151,10 @@ SValBuilder::getRegionValueSymbolVal(const 
TypedValueRegion *region) {
   return nonloc::SymbolVal(sym);
 }
 
-DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *SymbolTag,

fangyi-zhou wrote:

This overload would have been removed, but its behaviour is not entirely the 
same as obtaining the type from the expression directly. (See the check for 
`isGLValue`). Without complicating the refactor in this PR I decided to leave 
it as is, and add a new parameter for the CFG Element Ref

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -166,57 +166,47 @@ DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const 
void *SymbolTag,
   if (Ex->isGLValue())
 T = LCtx->getAnalysisDeclContext()->getASTContext().getPointerType(ExType);
 
-  return conjureSymbolVal(SymbolTag, Ex, LCtx, T, Count);
+  return conjureSymbolVal(SymbolTag, elemRef, LCtx, T, Count);
 }
 
-DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *symbolTag,
-   const Stmt *St,
-   const LocationContext *LCtx,
-   QualType type,
-   unsigned count) {
+DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(
+const void *symbolTag, const CFGBlock::ConstCFGElementRef elemRef,
+const LocationContext *LCtx, QualType type, unsigned count) {
   if (type->isNullPtrType())
 return makeZeroVal(type);
 
   if (!SymbolManager::canSymbolicate(type))
 return UnknownVal();
 
-  SymbolRef sym = SymMgr.conjureSymbol(St, LCtx, type, count, symbolTag);
+  SymbolRef sym = SymMgr.conjureSymbol(elemRef, LCtx, type, count, symbolTag);
 
   if (Loc::isLocType(type))
 return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
 
   return nonloc::SymbolVal(sym);
 }
 
-DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const Stmt *stmt,
-   const LocationContext *LCtx,
-   QualType type,
-   unsigned visitCount) {
+DefinedOrUnknownSVal
+SValBuilder::conjureSymbolVal(const CFGBlock::ConstCFGElementRef elemRef,
+  const LocationContext *LCtx, QualType type,
+  unsigned visitCount) {
   if (type->isNullPtrType())
 return makeZeroVal(type);
 
   if (!SymbolManager::canSymbolicate(type))
 return UnknownVal();
 
-  SymbolRef sym = SymMgr.conjureSymbol(stmt, LCtx, type, visitCount);
+  SymbolRef sym = SymMgr.conjureSymbol(elemRef, LCtx, type, visitCount);
 
   if (Loc::isLocType(type))
 return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
 
   return nonloc::SymbolVal(sym);
 }
 
-DefinedSVal SValBuilder::getConjuredHeapSymbolVal(const Expr *E,

fangyi-zhou wrote:

This overload is removed since it's not possible to retrieve the type from the 
CFGElementRef. If there's a way to obtain it please let me know so I can add it 
back.

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

@@ -695,6 +695,21 @@ class CFGBlock {
 void dump() const {
   dumpToStream(llvm::errs());
 }
+
+void Profile(llvm::FoldingSetNodeID &ID) const {
+  ID.AddPointer(Parent);
+  ID.AddInteger(Index);
+}
+
+int64_t getID() const {

fangyi-zhou wrote:

Any suggestion for replacement?

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

fangyi-zhou wrote:

May I get a re-review for the changes please?

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)

[Fangyi Zhou via cfe-commits]

fangyi-zhou wrote:

Sorry I've been a bit busy with other things, just had some time to address the 
review comments. Please let me know if anything else needs changing

https://github.com/llvm/llvm-project/pull/128251
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits