[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-08-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:431
+return ExprEngine::getObjectUnderConstruction(
+getState(), {getOriginExpr(), Index}, 
getCalleeStackFrame()).hasValue();
+  }

Whoops, this is wrong: should say `getLocationContext()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D49715



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-07-31 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338474: [analyzer] CallEvent: Add helper methods for 
obtaining the callee stack frame. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49715?vs=157550=158446#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49715

Files:
  cfe/trunk/include/clang/Analysis/ConstructionContext.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -581,6 +581,14 @@
 return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);
   }
 
+  /// By looking at a certain item that may be potentially part of an object's
+  /// ConstructionContext, retrieve such object's location. A particular
+  /// statement can be transparently passed as \p Item in most cases.
+  static Optional
+  getObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem ,
+ const LocationContext *LC);
+
 protected:
   /// evalBind - Handle the semantics of binding a value to a specific location.
   ///  This method is used by evalStore, VisitDeclStmt, and others.
@@ -773,13 +781,6 @@
const ConstructionContextItem ,
const LocationContext *LC);
 
-  /// If the given statement corresponds to an object under construction,
-  /// being part of its construciton context, retrieve that object's location.
-  static Optional
-  getObjectUnderConstruction(ProgramStateRef State,
- const ConstructionContextItem ,
- const LocationContext *LC);
-
   /// If the given expression corresponds to a temporary that was used for
   /// passing into an elidable copy/move constructor and that constructor
   /// was actually elided, track that we also need to elide the destructor.
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -29,6 +29,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
@@ -404,6 +405,46 @@
   /// \p D must not be null.
   static bool isVariadic(const Decl *D);
 
+  /// Returns AnalysisDeclContext for the callee stack frame.
+  /// Currently may fail; returns null on failure.
+  AnalysisDeclContext *getCalleeAnalysisDeclContext() const;
+
+  /// Returns the callee stack frame. That stack frame will only be entered
+  /// during analysis if the call is inlined, but it may still be useful
+  /// in intermediate calculations even if the call isn't inlined.
+  /// May fail; returns null on failure.
+  const StackFrameContext *getCalleeStackFrame() const;
+
+  /// Returns memory location for a parameter variable within the callee stack
+  /// frame. May fail; returns null on failure.
+  const VarRegion *getParameterLocation(unsigned Index) const;
+
+  /// Returns true if on the current path, the argument was constructed by
+  /// calling a C++ constructor over it. This is an internal detail of the
+  /// analysis which doesn't necessarily represent the program semantics:
+  /// if we are supposed to construct an argument directly, we may still
+  /// not do that because we don't know how (i.e., construction context is
+  /// unavailable in the CFG or not supported by the analyzer).
+  bool isArgumentConstructedDirectly(unsigned Index) const {
+// This assumes that the object was not yet removed from the state.
+return ExprEngine::getObjectUnderConstruction(
+getState(), {getOriginExpr(), Index}, getCalleeStackFrame()).hasValue();
+  }
+
+  /// Some calls have parameter numbering mismatched from argument numbering.
+  /// This function converts an argument index to the corresponding
+  /// parameter index. Returns None is the argument doesn't correspond
+  /// to any parameter variable.
+  Optional getAdjustedParameterIndex(unsigned ArgumentIndex) const {
+if (dyn_cast_or_null(getOriginExpr()) &&
+dyn_cast_or_null(getDecl())) {
+  // For member operator calls argument 0 on the 

[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-07-26 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 157550.
NoQ added a comment.

Add one more handy method, `getAdjustedParameterIndex()`, which helps using 
`getParameterLocation()`.


https://reviews.llvm.org/D49715

Files:
  include/clang/Analysis/ConstructionContext.h
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/CallEvent.cpp

Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -166,6 +167,68 @@
   return CheckerContext::isCLibraryFunction(FD, FunctionName);
 }
 
+AnalysisDeclContext *CallEvent::getCalleeAnalysisDeclContext() const {
+  const Decl *D = getDecl();
+
+  // If the callee is completely unknown, we cannot construct the stack frame.
+  if (!D)
+return nullptr;
+
+  // FIXME: Skip virtual functions for now. There's no easy procedure to foresee
+  // the exact decl that should be used, especially when it's not a definition.
+  if (const Decl *RD = getRuntimeDefinition().getDecl())
+if (RD != D)
+  return nullptr;
+
+  return LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
+}
+
+const StackFrameContext *CallEvent::getCalleeStackFrame() const {
+  AnalysisDeclContext *ADC = getCalleeAnalysisDeclContext();
+  if (!ADC)
+return nullptr;
+
+  const Expr *E = getOriginExpr();
+  if (!E)
+return nullptr;
+
+  // Recover CFG block via reverse lookup.
+  // TODO: If we were to keep CFG element information as part of the CallEvent
+  // instead of doing this reverse lookup, we would be able to build the stack
+  // frame for non-expression-based calls, and also we wouldn't need the reverse
+  // lookup.
+  CFGStmtMap *Map = LCtx->getAnalysisDeclContext()->getCFGStmtMap();
+  const CFGBlock *B = Map->getBlock(E);
+  assert(B);
+
+  // Also recover CFG index by scanning the CFG block.
+  unsigned Idx = 0, Sz = B->size();
+  for (; Idx < Sz; ++Idx)
+if (auto StmtElem = (*B)[Idx].getAs())
+  if (StmtElem->getStmt() == E)
+break;
+  assert(Idx < Sz);
+
+  return ADC->getManager()->getStackFrame(ADC, LCtx, E, B, Idx);
+}
+
+const VarRegion *CallEvent::getParameterLocation(unsigned Index) const {
+  const StackFrameContext *SFC = getCalleeStackFrame();
+  // We cannot construct a VarRegion without a stack frame.
+  if (!SFC)
+return nullptr;
+
+  const ParmVarDecl *PVD = parameters()[Index];
+  const VarRegion *VR =
+  State->getStateManager().getRegionManager().getVarRegion(PVD, SFC);
+
+  // This sanity check would fail if our parameter declaration doesn't
+  // correspond to the stack frame's function declaration.
+  assert(VR->getStackFrame() == SFC);
+
+  return VR;
+}
+
 /// Returns true if a type is a pointer-to-const or reference-to-const
 /// with no further indirection.
 static bool isPointerToConst(QualType Ty) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -580,7 +580,15 @@
  SVal LHS, SVal RHS, QualType T) {
 return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);
   }
-  
+
+  /// By looking at a certain item that may be potentially part of an object's
+  /// ConstructionContext, retrieve such object's location. A particular
+  /// statement can be transparently passed as \p Item in most cases.
+  static Optional
+  getObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem ,
+ const LocationContext *LC);
+
 protected:
   /// evalBind - Handle the semantics of binding a value to a specific location.
   ///  This method is used by evalStore, VisitDeclStmt, and others.
@@ -773,13 +781,6 @@
const ConstructionContextItem ,
const LocationContext *LC);
 
-  /// If the given statement corresponds to an object under construction,
-  /// being part of its construciton context, retrieve that object's location.
-  static Optional
-  getObjectUnderConstruction(ProgramStateRef State,
- const ConstructionContextItem ,
- const LocationContext *LC);
-
   /// If the given expression corresponds to a temporary that was used for
   /// passing into an elidable copy/move constructor and that constructor
   /// was actually elided, track that we also need to elide the destructor.
Index: 

[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-07-26 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:197
+
+  // Recover CFG block via reverse lookup. Maybe it should just be a part of 
the
+  // CallEvent object? That would have been convenient.

george.karpenkov wrote:
> Can we remove "maybe" / "that would have been" comments?
Changed them into a TODO because they're rather important to keep. Like, that's 
a flaw in this code.


https://reviews.llvm.org/D49715



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-07-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 157416.
NoQ marked 4 inline comments as done.
NoQ added a comment.

Address comments by editing comments.


https://reviews.llvm.org/D49715

Files:
  include/clang/Analysis/ConstructionContext.h
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/CallEvent.cpp

Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -166,6 +167,68 @@
   return CheckerContext::isCLibraryFunction(FD, FunctionName);
 }
 
+AnalysisDeclContext *CallEvent::getCalleeAnalysisDeclContext() const {
+  const Decl *D = getDecl();
+
+  // If the callee is completely unknown, we cannot construct the stack frame.
+  if (!D)
+return nullptr;
+
+  // FIXME: Skip virtual functions for now. There's no easy procedure to foresee
+  // the exact decl that should be used, especially when it's not a definition.
+  if (const Decl *RD = getRuntimeDefinition().getDecl())
+if (RD != D)
+  return nullptr;
+
+  return LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
+}
+
+const StackFrameContext *CallEvent::getCalleeStackFrame() const {
+  AnalysisDeclContext *ADC = getCalleeAnalysisDeclContext();
+  if (!ADC)
+return nullptr;
+
+  const Expr *E = getOriginExpr();
+  if (!E)
+return nullptr;
+
+  // Recover CFG block via reverse lookup.
+  // TODO: If we were to keep CFG element information as part of the CallEvent
+  // instead of doing this reverse lookup, we would be able to build the stack
+  // frame for non-expression-based calls, and also we wouldn't need the reverse
+  // lookup.
+  CFGStmtMap *Map = LCtx->getAnalysisDeclContext()->getCFGStmtMap();
+  const CFGBlock *B = Map->getBlock(E);
+  assert(B);
+
+  // Also recover CFG index by scanning the CFG block.
+  unsigned Idx = 0, Sz = B->size();
+  for (; Idx < Sz; ++Idx)
+if (auto StmtElem = (*B)[Idx].getAs())
+  if (StmtElem->getStmt() == E)
+break;
+  assert(Idx < Sz);
+
+  return ADC->getManager()->getStackFrame(ADC, LCtx, E, B, Idx);
+}
+
+const VarRegion *CallEvent::getParameterLocation(unsigned Index) const {
+  const StackFrameContext *SFC = getCalleeStackFrame();
+  // We cannot construct a VarRegion without a stack frame.
+  if (!SFC)
+return nullptr;
+
+  const ParmVarDecl *PVD = parameters()[Index];
+  const VarRegion *VR =
+  State->getStateManager().getRegionManager().getVarRegion(PVD, SFC);
+
+  // This sanity check would fail if our parameter declaration doesn't
+  // correspond to the stack frame's function declaration.
+  assert(VR->getStackFrame() == SFC);
+
+  return VR;
+}
+
 /// Returns true if a type is a pointer-to-const or reference-to-const
 /// with no further indirection.
 static bool isPointerToConst(QualType Ty) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -580,7 +580,15 @@
  SVal LHS, SVal RHS, QualType T) {
 return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);
   }
-  
+
+  /// By looking at a certain item that may be potentially part of an object's
+  /// ConstructionContext, retrieve such object's location. A particular
+  /// statement can be transparently passed as \p Item in most cases.
+  static Optional
+  getObjectUnderConstruction(ProgramStateRef State,
+ const ConstructionContextItem ,
+ const LocationContext *LC);
+
 protected:
   /// evalBind - Handle the semantics of binding a value to a specific location.
   ///  This method is used by evalStore, VisitDeclStmt, and others.
@@ -773,13 +781,6 @@
const ConstructionContextItem ,
const LocationContext *LC);
 
-  /// If the given statement corresponds to an object under construction,
-  /// being part of its construciton context, retrieve that object's location.
-  static Optional
-  getObjectUnderConstruction(ProgramStateRef State,
- const ConstructionContextItem ,
- const LocationContext *LC);
-
   /// If the given expression corresponds to a temporary that was used for
   /// passing into an elidable copy/move constructor and that constructor
   /// was actually elided, track that we also need to elide the destructor.
Index: 

[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-07-24 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.

Minor nits mostly.




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:411
+  /// for bad reasons, returning null, even in post-call of an inlined 
function.
+  AnalysisDeclContext *getCalleeAnalysisDeclContext() const;
+

Could we be more succinct here?
e.g. \return Best-effort getter for AnalysisDeclContext, returns null on 
failure.

Same for all the methods below.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:776
 
+public:
   /// If the given statement corresponds to an object under construction,

Could we group it with other public methods at the top?



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:193
+  const Expr *E = getOriginExpr();
+  // We cannot lookup a CFG element without an origin-expression.
+  if (!E)

"we cannot have" comments seem redundant, it's usually implied by the code.
This one is up to you though.



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:197
+
+  // Recover CFG block via reverse lookup. Maybe it should just be a part of 
the
+  // CallEvent object? That would have been convenient.

Can we remove "maybe" / "that would have been" comments?


Repository:
  rC Clang

https://reviews.llvm.org/D49715



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49715: [analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.

2018-07-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.

These methods allow reasoning about the stack frame that will be (or was) used 
when the function will be (or was) inlined, even if it will not be (or was not) 
inlined.

They are implemented in a fairly ugly manner, and while i plan to address some 
of the issues in follow-up patches (at least, do something about virtual 
calls), i don't think it'll work perfectly soon. A proper implementation would 
require a large amount of refactoring for `Call::getDecl()` and 
`Call::getRuntimeDefinition()` and the logic in `ExprEngine`'s call modeling 
and i didn't accomplish much in a couple days of working on it, so i'm putting 
a proper implementation on hold for now, safely returning `nullptr` when the 
proper `Decl` cannot be reliably obtained.

This was ultimately necessary to obtain the parameter regions on the future 
stack frame for the purposes of https://reviews.llvm.org/D49443. We don't need 
this function to be perfect because a lot of other things are imperfect and 
we're fine with falling back to a conservative behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D49715

Files:
  include/clang/Analysis/ConstructionContext.h
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/CallEvent.cpp

Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -166,6 +167,67 @@
   return CheckerContext::isCLibraryFunction(FD, FunctionName);
 }
 
+AnalysisDeclContext *CallEvent::getCalleeAnalysisDeclContext() const {
+  const Decl *D = getDecl();
+
+  // If the callee is completely unknown, we cannot construct the stack frame.
+  if (!D)
+return nullptr;
+
+  // FIXME: Skip virtual functions for now. There's no easy procedure to foresee
+  // the exact decl that should be used, especially when it's not a definition.
+  if (const Decl *RD = getRuntimeDefinition().getDecl())
+if (RD != D)
+  return nullptr;
+
+  return LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
+}
+
+const StackFrameContext *CallEvent::getCalleeStackFrame() const {
+  AnalysisDeclContext *ADC = getCalleeAnalysisDeclContext();
+  // We cannot construct a stack frame without an ADC.
+  if (!ADC)
+return nullptr;
+
+  const Expr *E = getOriginExpr();
+  // We cannot lookup a CFG element without an origin-expression.
+  if (!E)
+return nullptr;
+
+  // Recover CFG block via reverse lookup. Maybe it should just be a part of the
+  // CallEvent object? That would have been convenient.
+  CFGStmtMap *Map = LCtx->getAnalysisDeclContext()->getCFGStmtMap();
+  const CFGBlock *B = Map->getBlock(E);
+  assert(B);
+
+  // Also recover CFG index by scanning the CFG block.
+  unsigned Idx = 0, Sz = B->size();
+  for (; Idx < Sz; ++Idx)
+if (auto StmtElem = (*B)[Idx].getAs())
+  if (StmtElem->getStmt() == E)
+break;
+  assert(Idx < Sz);
+
+  return ADC->getManager()->getStackFrame(ADC, LCtx, E, B, Idx);
+}
+
+const VarRegion *CallEvent::getParameterLocation(unsigned Index) const {
+  const StackFrameContext *SFC = getCalleeStackFrame();
+  // We cannot construct a VarRegion without a stack frame.
+  if (!SFC)
+return nullptr;
+
+  const ParmVarDecl *PVD = parameters()[Index];
+  const VarRegion *VR =
+  State->getStateManager().getRegionManager().getVarRegion(PVD, SFC);
+
+  // This sanity check would fail if our parameter declaration doesn't
+  // correspond to the stack frame's function declaration.
+  assert(VR->getStackFrame() == SFC);
+
+  return VR;
+}
+
 /// Returns true if a type is a pointer-to-const or reference-to-const
 /// with no further indirection.
 static bool isPointerToConst(QualType Ty) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -773,13 +773,15 @@
const ConstructionContextItem ,
const LocationContext *LC);
 
+public:
   /// If the given statement corresponds to an object under construction,
   /// being part of its construciton context, retrieve that object's location.
   static Optional
   getObjectUnderConstruction(ProgramStateRef State,
  const