Re: [PATCH] D11700: Added remove taint support to ProgramState.
franchiotta added inline comments.
Comment at:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:444-448
@@ +443,7 @@
+
+ SymbolRef
+ getSymbolFromStmt(const Stmt *S, const LocationContext *LCtx) const;
+
+ const MemRegion*
+ getRegionFromStmt(const Stmt *S, const LocationContext *LCtx) const;
+
krememek wrote:
> krememek wrote:
> > Can we add documentation comments for these? Seems like generally useful
> > utility methods. We could also probably just make these public.
> Actually, I'm wondering if we really need to add these at all. They are just
> one liners that easily could be written where they are used.
Right. Removing these methods, and adding the one-liners directly where they
are used.
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:653-654
@@ -654,3 +652,4 @@
+ *LCtx) const {
if (const Expr *E = dyn_cast_or_null(S))
S = E->IgnoreParens();
krememek wrote:
> Is this even needed? I think Environment::getSVal() already handles
> parenthesis and other ignored expressions. This looks like dead code.
>
> This can probably just be an inline method in ProgramState.h, that just
> forwards to getSVal(S, LCtx).getAsSymbol().
>
> Alternatively, if this is only called once, we don't need to add a method at
> all, since it is just a one liner.
Yes, you are right. It is not needed since it is handle by
ignoreTransparentExprs method in Environment module. I will not add this method
at all.
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:660-663
@@ +659,6 @@
+
+const MemRegion* ProgramState::getRegionFromStmt(const Stmt *S, const
LocationContext
+ *LCtx) const {
+ return getSVal(S, LCtx).getAsRegion();
+}
+
krememek wrote:
> This is just a one-liner. Do we really need this method at all? It is only
> called once.
We don't. I will add the one-liner directly where it is used.
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:672-676
@@ -660,7 +671,7 @@
- const MemRegion *R = getSVal(S, LCtx).getAsRegion();
+ const MemRegion *R = getRegionFromStmt(S, LCtx);
addTaint(R, Kind);
// Cannot add taint, so just return the state.
return this;
}
krememek wrote:
> This looks fishy. 'addTaint' returns a new state, but then the return value
> is ignored, and 'this' is returned.
Yes, it does.. I will return at the time the last addTaint is invoked.
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:704-708
@@ +703,7 @@
+
+ const MemRegion *R = getRegionFromStmt(S, LCtx);
+ removeTaint(R, Kind);
+
+ // Cannot remove taint, so just return the state.
+ return this;
+}
krememek wrote:
> This looks fishy. 'removeTaint' returns a new state, but then the return
> value is ignored. 'ProgramState' values are immutable, so this method
> appears to do nothing.
Yes, you are right. I will return at the time the last addTaint is invoked.
http://reviews.llvm.org/D11700
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11700: Added remove taint support to ProgramState.
franchiotta updated the summary for this revision.
franchiotta updated this revision to Diff 36431.
franchiotta added a comment.
Some changes made based on the received suggestions.
http://reviews.llvm.org/D11700
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
lib/StaticAnalyzer/Core/ProgramState.cpp
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -648,32 +648,24 @@
return true;
}
-ProgramStateRef ProgramState::addTaint(const Stmt *S,
- const LocationContext *LCtx,
- TaintTagType Kind) const {
- if (const Expr *E = dyn_cast_or_null(S))
-S = E->IgnoreParens();
-
+ProgramStateRef ProgramState::addTaint(const Stmt *S, const LocationContext
+ *LCtx, TaintTagType Kind) const {
SymbolRef Sym = getSVal(S, LCtx).getAsSymbol();
if (Sym)
return addTaint(Sym, Kind);
const MemRegion *R = getSVal(S, LCtx).getAsRegion();
- addTaint(R, Kind);
-
- // Cannot add taint, so just return the state.
- return this;
+ return addTaint(R, Kind);
}
ProgramStateRef ProgramState::addTaint(const MemRegion *R,
- TaintTagType Kind) const {
+ TaintTagType Kind) const {
if (const SymbolicRegion *SR = dyn_cast_or_null(R))
return addTaint(SR->getSymbol(), Kind);
return this;
}
-ProgramStateRef ProgramState::addTaint(SymbolRef Sym,
- TaintTagType Kind) const {
+ProgramStateRef ProgramState::addTaint(SymbolRef Sym, TaintTagType Kind) const {
// If this is a symbol cast, remove the cast before adding the taint. Taint
// is cast agnostic.
while (const SymbolCast *SC = dyn_cast(Sym))
@@ -684,6 +676,35 @@
return NewState;
}
+ProgramStateRef ProgramState::removeTaint(const Stmt *S, const LocationContext
+ *LCtx, TaintTagType Kind) const {
+ SymbolRef Sym = getSVal(S, LCtx).getAsSymbol();
+ if (Sym)
+return removeTaint(Sym, Kind);
+
+ const MemRegion *R = getSVal(S, LCtx).getAsRegion();
+ return removeTaint(R, Kind);
+}
+
+ProgramStateRef ProgramState::removeTaint(const MemRegion *R,
+ TaintTagType Kind) const {
+ if (const SymbolicRegion *SR = dyn_cast_or_null(R))
+return removeTaint(SR->getSymbol(), Kind);
+ return this;
+}
+
+ProgramStateRef ProgramState::removeTaint(SymbolRef Sym,
+ TaintTagType Kind) const {
+ // If this is a symbol cast, remove the cast before removing the taint. Taint
+ // is cast agnostic.
+ while (const SymbolCast *SC = dyn_cast(Sym))
+Sym = SC->getOperand();
+
+ ProgramStateRef NewState = remove(Sym);
+ assert(NewState);
+ return NewState;
+}
+
bool ProgramState::isTainted(const Stmt *S, const LocationContext *LCtx,
TaintTagType Kind) const {
if (const Expr *E = dyn_cast_or_null(S))
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -341,15 +341,27 @@
/// Create a new state in which the statement is marked as tainted.
ProgramStateRef addTaint(const Stmt *S, const LocationContext *LCtx,
- TaintTagType Kind = TaintTagGeneric) const;
+ TaintTagType Kind = TaintTagGeneric) const;
/// Create a new state in which the symbol is marked as tainted.
ProgramStateRef addTaint(SymbolRef S,
- TaintTagType Kind = TaintTagGeneric) const;
+ TaintTagType Kind = TaintTagGeneric) const;
/// Create a new state in which the region symbol is marked as tainted.
ProgramStateRef addTaint(const MemRegion *R,
- TaintTagType Kind = TaintTagGeneric) const;
+ TaintTagType Kind = TaintTagGeneric) const;
+
+ /// Create a new state in which the statement is marked as non-tainted.
+ ProgramStateRef removeTaint(const Stmt *S, const LocationContext *LCtx,
+ TaintTagType Kind = TaintTagGeneric) const;
+
+ /// Create a new state in which the symbol is marked as non-tainted.
+ ProgramStateRef removeTaint(SymbolRef S,
+ TaintTagType Kind = TaintTagGeneric) const;
+
+ /// Create a new state in which the region symbol is marked as non-tainted.
+ ProgramStateRef removeTaint(const MemRegion *R,
+ TaintTagType Kind = TaintTagGeneric) const;
/// Check if the statement is tainted in the current sta
Re: [PATCH] D11700: Added remove taint support to ProgramState.
franchiotta updated this revision to Diff 31756.
franchiotta added a comment.
Updated refactoring.
http://reviews.llvm.org/D11700
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
lib/StaticAnalyzer/Core/ProgramState.cpp
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -648,17 +648,28 @@
return true;
}
-ProgramStateRef ProgramState::addTaint(const Stmt *S,
- const LocationContext *LCtx,
- TaintTagType Kind) const {
+SymbolRef ProgramState::getSymbolFromStmt(const Stmt *S, const LocationContext
+ *LCtx) const {
if (const Expr *E = dyn_cast_or_null(S))
S = E->IgnoreParens();
- SymbolRef Sym = getSVal(S, LCtx).getAsSymbol();
+ return getSVal(S, LCtx).getAsSymbol();
+
+}
+
+const MemRegion* ProgramState::getRegionFromStmt(const Stmt *S, const LocationContext
+ *LCtx) const {
+ return getSVal(S, LCtx).getAsRegion();
+}
+
+ProgramStateRef ProgramState::addTaint(const Stmt *S,
+ const LocationContext *LCtx,
+ TaintTagType Kind) const {
+ SymbolRef Sym = getSymbolFromStmt(S, LCtx);
if (Sym)
return addTaint(Sym, Kind);
- const MemRegion *R = getSVal(S, LCtx).getAsRegion();
+ const MemRegion *R = getRegionFromStmt(S, LCtx);
addTaint(R, Kind);
// Cannot add taint, so just return the state.
@@ -684,6 +695,38 @@
return NewState;
}
+ProgramStateRef ProgramState::removeTaint(const Stmt *S, const LocationContext
+ *LCtx, TaintTagType Kind) const {
+ SymbolRef Sym = getSymbolFromStmt(S, LCtx);
+ if (Sym)
+return removeTaint(Sym, Kind);
+
+ const MemRegion *R = getRegionFromStmt(S, LCtx);
+ removeTaint(R, Kind);
+
+ // Cannot remove taint, so just return the state.
+ return this;
+}
+
+ProgramStateRef ProgramState::removeTaint(const MemRegion *R, TaintTagType Kind)
+ const {
+ if (const SymbolicRegion *SR = dyn_cast_or_null(R))
+return removeTaint(SR->getSymbol(), Kind);
+ return this;
+}
+
+ProgramStateRef ProgramState::removeTaint(SymbolRef Sym, TaintTagType Kind)
+ const {
+ // If this is a symbol cast, remove the cast before removing the taint. Taint
+ // is cast agnostic.
+ while (const SymbolCast *SC = dyn_cast(Sym))
+Sym = SC->getOperand();
+
+ ProgramStateRef NewState = remove(Sym);
+ assert(NewState);
+ return NewState;
+}
+
bool ProgramState::isTainted(const Stmt *S, const LocationContext *LCtx,
TaintTagType Kind) const {
if (const Expr *E = dyn_cast_or_null(S))
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -330,6 +330,18 @@
ProgramStateRef addTaint(const MemRegion *R,
TaintTagType Kind = TaintTagGeneric) const;
+ /// Create a new state in which the statement is marked as non-tainted.
+ ProgramStateRef removeTaint(const Stmt *S, const LocationContext *LCtx,
+ TaintTagType Kind = TaintTagGeneric) const;
+
+ /// Create a new state in which the symbol is marked as non-tainted.
+ ProgramStateRef removeTaint(SymbolRef S, TaintTagType Kind = TaintTagGeneric)
+ const;
+
+ /// Create a new state in which the region symbol is marked as non-tainted.
+ ProgramStateRef removeTaint(const MemRegion *R, TaintTagType Kind =
+ TaintTagGeneric) const;
+
/// Check if the statement is tainted in the current state.
bool isTainted(const Stmt *S, const LocationContext *LCtx,
TaintTagType Kind = TaintTagGeneric) const;
@@ -428,6 +440,13 @@
InvalidatedSymbols *IS,
RegionAndSymbolInvalidationTraits *HTraits,
const CallEvent *Call) const;
+
+ SymbolRef
+ getSymbolFromStmt(const Stmt *S, const LocationContext *LCtx) const;
+
+ const MemRegion*
+ getRegionFromStmt(const Stmt *S, const LocationContext *LCtx) const;
+
};
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
