Re: [PATCH] D11700: Added remove taint support to ProgramState.

2015-10-03 Thread Francisco via cfe-commits
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.

2015-10-03 Thread Francisco via cfe-commits
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.

2015-08-10 Thread Francisco via cfe-commits
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