franchiotta added a comment. In http://reviews.llvm.org/D11700#216472, @krememek wrote:
> I don't remember why the 'tainted' methods were added to ProgramState in the > first place, but it doesn't seem quite right. Taint can easily be modeled as > a set of APIs that modify and produce new ProgramStates, but I don't see why > it should be part of ProgramState itself. Yes, I agree with you Ted. We can leave as it is just for now, and then I can work on that patch. May I assign you as subscriber when submitting it? ================ Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:338-339 @@ +337,4 @@ + /// Create a new state in which the symbol is marked as non-tainted. + ProgramStateRef removeTaint(SymbolRef S, + TaintTagType Kind = TaintTagGeneric) const; + ---------------- krememek wrote: > The parameters from the two lines should line up. Alright ================ Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:687-721 @@ -686,1 +686,37 @@ +ProgramStateRef ProgramState::removeTaint(const Stmt *S, + const LocationContext *LCtx, + TaintTagType Kind) const { + if (const Expr *E = dyn_cast_or_null<Expr>(S)) + S = E->IgnoreParens(); + + SymbolRef Sym = getSVal(S, LCtx).getAsSymbol(); + if (Sym) + return removeTaint(Sym, Kind); + + const MemRegion *R = getSVal(S, LCtx).getAsRegion(); + 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<SymbolicRegion>(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<SymbolCast>(Sym)) + Sym = SC->getOperand(); + + ProgramStateRef NewState = remove<TaintMap>(Sym); + assert(NewState); + return NewState; +} + ---------------- krememek wrote: > This seems reasonable enough. It mostly mirrors the other taint operations > we have, and there is noticeably a fair amount of copy-paste here that I > wonder could be better factored. `removeTaint` looks pretty much identical > to `addTaint` except that it uses the `remove` function on the TaintMap. I'm > particularly concerned about the copy-paste for the first `removeTaint`, as > that is a non-trivial amount of logic. Could this case possibly be > refactored a bit with `addTaint`? Yes, sure! Let me upload a new patch for this. http://reviews.llvm.org/D11700 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits