[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
zaks.anna added a comment. Update: Adding support for top frame to the CallEvent is difficult, so let's just use CallEvent API in the checkers for all frames but the top one and have custom top frame handling in the Recursion checker itself. The top frame handling does not need to be complete (handle all call types) since in the worst case we will be missing bugs (no false positives due to this). https://reviews.llvm.org/D27091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
NoQ added a comment.
Chatted about this a bit more. Because `CallEvent` is an API designed mostly
for convenience, and `ProgramState` is a core API that needs to stay as clean
and separate from other entities and easy to understand as possible, it would
be the best to
1. construct a `CallEvent` in the checker (which is cheap) and query that,
2. and also allow constructing `CallEvent`s for the top frame (with
`StackFrameContext::inTopFrame()`, with null call site ("origin") expression),
and
3. add a branch to the `CallEvent::getArgSVal()` method (and similar methods
for obtaining implicit arguments like C++ `this`, ObjC `self`, ObjC `super`) to
handle the top frame case.
If this sounds hard, handling this completely on the checker side is fine,
because few checker are using this at the moment.
https://reviews.llvm.org/D27091
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
zaks.anna added a comment. Artem just pointed out that I have "Smalls" instead of "SVals" in my first comment. https://reviews.llvm.org/D27091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
zaks.anna added a comment. > Hmm, i'm thinking of just the opposite - refactor the CallEvent methods to > use the respective new ProgramState methods. > > This way we'd avoid touching the CallEvent allocator for a simple Environment > lookup, while still avoiding code duplication. I see what you are suggesting, but I still have the concerns I've mentioned in my first comment. **The whole purpose of CallEvent is to allow an easy and lightweight way of dealing with all the different types of calls**, which is exactly what we are doing in this checker. CallEvent is used for this exact purpose elsewhere throughout the analyzer. What do you mean by "touching the CallEvent allocator" and why would you want to discourage that? CallEventManager::getCaller contains logic to dispatch to different CallEvent types depending on what call we are dealing with. The argument lookup will depend on type of the call. The method added in this PR does not deal with all call types, but only with CallExpr, so you'd need to duplicate the code from CallEvent construction to here. As I've mentioned before, I do not think that Call dispatch logic and API for looking up arguments SVals should belong to ProgramState as it is a higher abstraction and should not reason about LocationContexts or Calls (which are part of Program Location and AST respectively). In addition, using CallEven to look up argument values is much simpler to understand for API users. https://reviews.llvm.org/D27091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
NoQ added a comment. Hmm, i'm thinking of just the opposite - refactor the CallEvent methods to use the respective new ProgramState methods. This way we'd avoid touching the CallEvent allocator for a simple Environment lookup, while still avoiding code duplication. https://reviews.llvm.org/D27091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
zaks.anna added a comment.
Hi!
Looks like this this is used by the Infinite recursion checker. Specifically,
the checker not only needs to get Smalls for arguments of the current
CallEvent, but it also looks for arguments of other calls on the stack. The
checker walks the LocationContext and uses this new API to look up the
arguments passed to the calls on the stack.
The two concerns I have with this patch is that it is extending the overloaded
ProgramState API and also that this does not fully cover all the call types we
have in C/ObjC/C++. The good news is that the CallEvent API was created
specifically to encapsulate this complexity and it already contains the needed
API to look up the SVal of the argument. Have you considered creating CallEvent
for the calls on the stack and using that existing API to find the Smalls of
the arguments.
Here is some documentation from CallEvent.h:
`
/// CallEvents are created through the factory methods of CallEventManager.
///
/// CallEvents should always be cheap to create and destroy. In order for
/// CallEventManager to be able to re-use CallEvent-sized memory blocks,
/// subclasses of CallEvent may not add any data members to the base class.
/// Use the "Data" and "Location" fields instead.
`
It looks like this code from BugReporterVisitors.cpp is doing something similar:
` // Don't automatically suppress a report if one of the arguments is
// known to be a null pointer. Instead, start tracking /that/ null
// value back to its origin.
ProgramStateManager = BRC.getStateManager();
CallEventManager = StateMgr.getCallEventManager();
ProgramStateRef State = N->getState();
CallEventRef<> Call = CallMgr.getCaller(StackFrame, State);
for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) {
Optional ArgV = Call->getArgSVal(I).getAs();
if (!ArgV)
continue;
`
I hope this approach works and you could just add that code to your checker.
(If not, we can investigate other options.)
https://reviews.llvm.org/D27091
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx
k-wisniewski created this revision.
k-wisniewski added reviewers: NoQ, zaks.anna, dcoughlin, a.sidorin.
k-wisniewski added subscribers: cfe-commits, NoQ.
his patch adds getArgsSVal method to ProgramState that allows the user to
obtain SVals of argumetns used in a call that created the given StackFrameCtx.
The problem with arguments SVals being overwritten has been fixed according to
@NoQ 's suggestion from the previous revision of this patch
(https://reviews.llvm.org/D26760)
https://reviews.llvm.org/D27091
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -294,6 +294,10 @@
/// Get the lvalue for an array index.
SVal getLValue(QualType ElementType, SVal Idx, SVal Base) const;
+ /// Get the symbolic value of arguments used in a call
+ /// that created the given stack frame
+ SVal getArgSVal(const StackFrameContext *SFC, const unsigned ArgIdx) const;
+
/// Returns the SVal bound to the statement 'S' in the state's environment.
SVal getSVal(const Stmt *S, const LocationContext *LCtx) const;
@@ -725,6 +729,36 @@
return UnknownVal();
}
+inline SVal ProgramState::getArgSVal(const StackFrameContext *SFC,
+ const unsigned ArgIdx) const {
+ const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();
+ const ObjCMethodDecl *MethodDecl =
+ dyn_cast_or_null(SFC->getDecl());
+ unsigned NumArgs = FunctionDecl
+ ? FunctionDecl->getNumParams()
+ : MethodDecl->getSelector().getNumArgs();
+ assert(ArgIdx < NumArgs && "Arg access out of range!");
+
+ if (SFC->inTopFrame()) {
+// if we are in the top frame we don't have any arguments bound in
+// the environment because the call wasn't modeled in the first place.
+const VarDecl *ArgDecl = FunctionDecl
+ ? FunctionDecl->parameters()[ArgIdx]
+ : MethodDecl->parameters()[ArgIdx];
+const Loc ArgLoc = getLValue(ArgDecl, SFC);
+StoreManager = stateMgr->getStoreManager();
+Store initialStore = storeMgr.getInitialStore(SFC).getStore();
+return storeMgr.getBinding(initialStore, ArgLoc);
+ } else {
+// in this case we need to ask the environment as the arguments' memory
+// region may have been purged as no longer needed.
+const Stmt *callSite = SFC->getCallSite();
+const CallExpr *callSiteExpr = dyn_cast(callSite);
+const Expr *argExpr = callSiteExpr->getArg(ArgIdx);
+return getSVal(argExpr, SFC->getParent());
+ }
+}
+
inline SVal ProgramState::getSVal(const Stmt *Ex,
const LocationContext *LCtx) const{
return Env.getSVal(EnvironmentEntry(Ex, LCtx),
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -294,6 +294,10 @@
/// Get the lvalue for an array index.
SVal getLValue(QualType ElementType, SVal Idx, SVal Base) const;
+ /// Get the symbolic value of arguments used in a call
+ /// that created the given stack frame
+ SVal getArgSVal(const StackFrameContext *SFC, const unsigned ArgIdx) const;
+
/// Returns the SVal bound to the statement 'S' in the state's environment.
SVal getSVal(const Stmt *S, const LocationContext *LCtx) const;
@@ -725,6 +729,36 @@
return UnknownVal();
}
+inline SVal ProgramState::getArgSVal(const StackFrameContext *SFC,
+ const unsigned ArgIdx) const {
+ const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();
+ const ObjCMethodDecl *MethodDecl =
+ dyn_cast_or_null(SFC->getDecl());
+ unsigned NumArgs = FunctionDecl
+ ? FunctionDecl->getNumParams()
+ : MethodDecl->getSelector().getNumArgs();
+ assert(ArgIdx < NumArgs && "Arg access out of range!");
+
+ if (SFC->inTopFrame()) {
+// if we are in the top frame we don't have any arguments bound in
+// the environment because the call wasn't modeled in the first place.
+const VarDecl *ArgDecl = FunctionDecl
+ ? FunctionDecl->parameters()[ArgIdx]
+ : MethodDecl->parameters()[ArgIdx];
+const Loc ArgLoc = getLValue(ArgDecl, SFC);
+StoreManager = stateMgr->getStoreManager();
+Store initialStore = storeMgr.getInitialStore(SFC).getStore();
+return storeMgr.getBinding(initialStore, ArgLoc);
+ } else {
+// in this case we need to ask the environment as the arguments' memory
+// region may have been purged as no longer
