[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

[Balázs Benics via Phabricator via cfe-commits]

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