[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker
Balázs Benics via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 15 01:58:59 PDT 2019
steakhal added a comment.
I think this patch is ok.
Although there are remarks:
- I think the current implementation of the taint filtering functions does not follow the expected semantics. Now the modelling would remove taint before calling the function (//pre statement//). One might expect that the modelling would remove taint only on function return (//post statement//). This way we would not catch this:
int myFilter(int* p) { // this function is stated as a filter function in the config file
int lookupTable[32] = {};
int value = lookupTable[*p]; // using a tainted value for indexing
escape(p);
return value;
}
- I agree with @NoQ about the `testConfigurationFilter`, which now does not test the implementation but the behavior of the static analyzer if it conservatively invalidates in case of an unknown function call.
- I also think that we should have testcases for testing the filtering functionality of the config file. Eg. using the `myFilter1` could do the job here in the tests.
All in all, I still say that it seems to be a good patch.
================
Comment at: clang/test/Analysis/taint-generic.c:59
+#define bool int
+
----------------
Have you considered using `_Bool` instead?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59516/new/
https://reviews.llvm.org/D59516
More information about the cfe-commits
mailing list