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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
