[clang] b8ae323 - Revert "[analyzer] Add failing test case demonstrating buggy taint propagation"
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 14 09:46:00 PST 2022
Author: Balazs Benics
Date: 2022-02-14T18:45:46+01:00
New Revision: b8ae323cca61dc1edcd36e9ae18c7e4c3d76d52e
URL: https://github.com/llvm/llvm-project/commit/b8ae323cca61dc1edcd36e9ae18c7e4c3d76d52e
DIFF: https://github.com/llvm/llvm-project/commit/b8ae323cca61dc1edcd36e9ae18c7e4c3d76d52e.diff
LOG: Revert "[analyzer] Add failing test case demonstrating buggy taint propagation"
This reverts commit 744745ae195f0997e5bfd5aa2de47b9ea156b6a6.
I'm reverting this since this patch caused a build breakage.
https://lab.llvm.org/buildbot/#/builders/91/builds/3818
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
Removed:
clang/test/Analysis/taint-checker-callback-order-has-definition.c
clang/test/Analysis/taint-checker-callback-order-without-definition.c
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 428778e6cfaa6..e2209e3debfde 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -32,8 +32,6 @@
#include <memory>
#include <utility>
-#define DEBUG_TYPE "taint-checker"
-
using namespace clang;
using namespace ento;
using namespace taint;
@@ -693,13 +691,6 @@ void GenericTaintChecker::checkPostCall(const CallEvent &Call,
if (TaintArgs.isEmpty())
return;
- LLVM_DEBUG(for (ArgIdxTy I
- : TaintArgs) {
- llvm::dbgs() << "PostCall<";
- Call.dump(llvm::dbgs());
- llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
- });
-
for (ArgIdxTy ArgNum : TaintArgs) {
// Special handling for the tainted return value.
if (ArgNum == ReturnValueIndex) {
@@ -777,25 +768,15 @@ void GenericTaintRule::process(const GenericTaintChecker &Checker,
/// Propagate taint where it is necessary.
ForEachCallArg(
- [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';);
+ [this, &State, WouldEscape](ArgIdxTy I, const Expr *E, SVal V) {
+ if (PropDstArgs.contains(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 (!State->contains<TaintArgsOnPostVisit>(I)) {
- llvm::dbgs() << "PreCall<";
- Call.dump(llvm::dbgs());
- llvm::dbgs() << "> prepares tainting arg index: " << I << '\n';
- });
+ if (WouldEscape(V, E->getType()))
State = State->add<TaintArgsOnPostVisit>(I);
- }
});
C.addTransition(State);
diff --git a/clang/test/Analysis/taint-checker-callback-order-has-definition.c b/clang/test/Analysis/taint-checker-callback-order-has-definition.c
deleted file mode 100644
index 295f95c2bf452..0000000000000
--- a/clang/test/Analysis/taint-checker-callback-order-has-definition.c
+++ /dev/null
@@ -1,42 +0,0 @@
-// RUN: %clang_analyze_cc1 %s \
-// RUN: -analyzer-checker=core,alpha.security.taint \
-// 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);
-
-void nested_call(void) {}
-
-char *fgets(char *s, int n, FILE *fp) {
- nested_call(); // no-crash: we should not try adding taint to a non-existent argument.
- return (char *)0;
-}
-
-void top(const char *fname, char *buf) {
- FILE *fp = fopen(fname, "r");
- // CHECK: PreCall<fopen(fname, "r")> prepares tainting arg index: -1
- // CHECK-NEXT: PostCall<fopen(fname, "r")> actually wants to taint arg index: -1
-
- if (!fp)
- return;
-
- (void)fgets(buf, 42, fp); // Trigger taint propagation.
- // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1
- // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0
- // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 1
- // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting 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
-}
diff --git a/clang/test/Analysis/taint-checker-callback-order-without-definition.c b/clang/test/Analysis/taint-checker-callback-order-without-definition.c
deleted file mode 100644
index 962e8b259298e..0000000000000
--- a/clang/test/Analysis/taint-checker-callback-order-without-definition.c
+++ /dev/null
@@ -1,34 +0,0 @@
-// RUN: %clang_analyze_cc1 %s \
-// RUN: -analyzer-checker=core,alpha.security.taint \
-// RUN: -mllvm -debug-only=taint-checker \
-// RUN: 2>&1 | FileCheck %s
-
-struct _IO_FILE;
-typedef struct _IO_FILE FILE;
-FILE *fopen(const char *fname, const char *mode);
-
-char *fgets(char *s, int n, FILE *fp); // no-definition
-
-void top(const char *fname, char *buf) {
- FILE *fp = fopen(fname, "r"); // Introduce taint.
- // CHECK: PreCall<fopen(fname, "r")> prepares tainting arg index: -1
- // CHECK-NEXT: PostCall<fopen(fname, "r")> actually wants to taint arg index: -1
-
- if (!fp)
- return;
-
- (void)fgets(buf, 42, fp); // Trigger taint propagation.
-
- // FIXME: Why is the arg index 1 prepared for taint?
- // Before the call it wasn't tainted, and it also shouldn't be tainted after the call.
-
- // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1
- // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0
- // 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
-}
More information about the cfe-commits
mailing list