[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

Balázs Benics via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 5 05:04:15 PDT 2019


steakhal added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:191
     static TaintPropagationRule
-    getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+    getTaintPropagationRule(const GenericTaintChecker *Checker,
+                            const FunctionDecl *FDecl, StringRef Name,
----------------
Szelethus wrote:
> How about only passing `CustomPropagations`?
I would even consider to move this function out of the whole class. (Not only this function, but the others as well. Like isStdin, etc.)
I think pure, free-functions (in an anonymous namespace) are easier to reason about.


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:844
   if (Config)
-    Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
+    Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue()));
 }
----------------
Szelethus wrote:
> Wasn't this commited before?
Yes it was (https://github.com/llvm/llvm-project/blob/master/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp#L814).
I would kindly request a rebase.


================
Comment at: test/Analysis/taint-generic.c:377
+  int x = mySource1();
+  mySink(x, 1, 2); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+  mySink(1, x, 2); // no-warning
----------------
We could use this syntacs to achieve shorter lines. Note that `@-1`. Same for all the other lines.
```
mySink(x, 1, 2);
// expected-warning at -1 {{Untrusted data is passed to a user-defined sink}}
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59637/new/

https://reviews.llvm.org/D59637





More information about the cfe-commits mailing list