Author: Balazs Benics Date: 2022-02-14T18:45:46+01:00 New Revision: d16c5f4192c30d53468a472c6820163a81192825
URL: https://github.com/llvm/llvm-project/commit/d16c5f4192c30d53468a472c6820163a81192825 DIFF: https://github.com/llvm/llvm-project/commit/d16c5f4192c30d53468a472c6820163a81192825.diff LOG: Revert "[analyzer] Fix taint propagation by remembering to the location context" This reverts commit b099e1e562555fbc67e2e0abbc15074e14a85ff1. I'm reverting this since the head of the patch stack caused a build breakage. https://lab.llvm.org/buildbot/#/builders/91/builds/3818 Added: Modified: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp clang/test/Analysis/taint-checker-callback-order-has-definition.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 66143f78932c..428778e6cfaa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -38,8 +38,6 @@ using namespace clang; using namespace ento; using namespace taint; -using llvm::ImmutableSet; - namespace { class GenericTaintChecker; @@ -436,9 +434,7 @@ template <> struct ScalarEnumerationTraits<TaintConfiguration::VariadicType> { /// to the call post-visit. The values are signed integers, which are either /// ReturnValueIndex, or indexes of the pointer/reference argument, which /// points to data, which should be tainted on return. -REGISTER_MAP_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, const LocationContext *, - ImmutableSet<ArgIdxTy>) -REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ArgIdxFactory, ArgIdxTy) +REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, ArgIdxTy) void GenericTaintRuleParser::validateArgVector(const std::string &Option, const ArgVecTy &Args) const { @@ -689,26 +685,22 @@ void GenericTaintChecker::checkPostCall(const CallEvent &Call, // Set the marked values as tainted. The return value only accessible from // checkPostStmt. ProgramStateRef State = C.getState(); - const StackFrameContext *CurrentFrame = C.getStackFrame(); // Depending on what was tainted at pre-visit, we determined a set of // arguments which should be tainted after the function returns. These are // stored in the state as TaintArgsOnPostVisit set. - TaintArgsOnPostVisitTy TaintArgsMap = State->get<TaintArgsOnPostVisit>(); - - const ImmutableSet<ArgIdxTy> *TaintArgs = TaintArgsMap.lookup(CurrentFrame); - if (!TaintArgs) + TaintArgsOnPostVisitTy TaintArgs = State->get<TaintArgsOnPostVisit>(); + if (TaintArgs.isEmpty()) return; - assert(!TaintArgs->isEmpty()); LLVM_DEBUG(for (ArgIdxTy I - : *TaintArgs) { + : TaintArgs) { llvm::dbgs() << "PostCall<"; Call.dump(llvm::dbgs()); llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n'; }); - for (ArgIdxTy ArgNum : *TaintArgs) { + for (ArgIdxTy ArgNum : TaintArgs) { // Special handling for the tainted return value. if (ArgNum == ReturnValueIndex) { State = addTaint(State, Call.getReturnValue()); @@ -722,7 +714,7 @@ void GenericTaintChecker::checkPostCall(const CallEvent &Call, } // Clear up the taint info from the state. - State = State->remove<TaintArgsOnPostVisit>(CurrentFrame); + State = State->remove<TaintArgsOnPostVisit>(); C.addTransition(State); } @@ -784,33 +776,28 @@ void GenericTaintRule::process(const GenericTaintChecker &Checker, }; /// Propagate taint where it is necessary. - auto &F = State->getStateManager().get_context<ArgIdxFactory>(); - ImmutableSet<ArgIdxTy> Result = F.getEmptySet(); ForEachCallArg( - [this, WouldEscape, &Call, &Result, &F](ArgIdxTy I, const Expr *E, - SVal V) { + [this, &State, WouldEscape, &Call](ArgIdxTy I, const Expr *E, SVal V) { if (PropDstArgs.contains(I)) { LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs()); llvm::dbgs() << "> prepares tainting arg index: " << I << '\n';); - Result = F.add(Result, I); + State = State->add<TaintArgsOnPostVisit>(I); } // TODO: We should traverse all reachable memory regions via the // escaping parameter. Instead of doing that we simply mark only the // referred memory region as tainted. if (WouldEscape(V, E->getType())) { - LLVM_DEBUG(if (!Result.contains(I)) { + LLVM_DEBUG(if (!State->contains<TaintArgsOnPostVisit>(I)) { llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs()); llvm::dbgs() << "> prepares tainting arg index: " << I << '\n'; }); - Result = F.add(Result, I); + State = State->add<TaintArgsOnPostVisit>(I); } }); - if (!Result.isEmpty()) - State = State->set<TaintArgsOnPostVisit>(C.getStackFrame(), Result); C.addTransition(State); } @@ -901,11 +888,7 @@ void GenericTaintChecker::taintUnsafeSocketProtocol(const CallEvent &Call, if (SafeProtocol) return; - ProgramStateRef State = C.getState(); - auto &F = State->getStateManager().get_context<ArgIdxFactory>(); - ImmutableSet<ArgIdxTy> Result = F.add(F.getEmptySet(), ReturnValueIndex); - State = State->set<TaintArgsOnPostVisit>(C.getStackFrame(), Result); - C.addTransition(State); + C.addTransition(C.getState()->add<TaintArgsOnPostVisit>(ReturnValueIndex)); } /// Checker registration diff --git a/clang/test/Analysis/taint-checker-callback-order-has-definition.c b/clang/test/Analysis/taint-checker-callback-order-has-definition.c index a070077004fa..295f95c2bf45 100644 --- a/clang/test/Analysis/taint-checker-callback-order-has-definition.c +++ b/clang/test/Analysis/taint-checker-callback-order-has-definition.c @@ -3,6 +3,9 @@ // RUN: -mllvm -debug-only=taint-checker \ // RUN: 2>&1 | FileCheck %s +// FIXME: We should not crash. +// XFAIL: * + struct _IO_FILE; typedef struct _IO_FILE FILE; FILE *fopen(const char *fname, const char *mode); @@ -28,8 +31,12 @@ void top(const char *fname, char *buf) { // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2 - // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1 - // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0 - // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 1 - // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2 + // FIXME: We should propagate taint from PreCall<fgets> -> PostCall<fgets>. + // CHECK-NEXT: PostCall<nested_call()> actually wants to taint arg index: -1 + // CHECK-NEXT: PostCall<nested_call()> actually wants to taint arg index: 0 + // CHECK-NEXT: PostCall<nested_call()> actually wants to taint arg index: 1 + // CHECK-NEXT: PostCall<nested_call()> actually wants to taint arg index: 2 + + // FIXME: We should not crash. + // CHECK: PLEASE submit a bug report } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits