[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Starting to look real good!



================
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:807
+                  "",
+                  Released>,
+  ]>,
----------------
We mark options that are not yet ready for used with `InAlpha`, rather then `Released`. I think it would be more fitting in this case!

Mind that if you'd like to list this option after that, you'd have to use
```lang=bash
clang -cc1 -analyzer-checker-option-help-alpha
```


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:58
+
+  /// The ``TaintConfiguration`` is used to parse configuration file.
+  struct TaintConfiguration {
----------------
Just simply `Used to parse the configuration file`.
https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:302-303
+      Result.push_back(InvalidArgIndex);
+      llvm::errs() << "Invalid arg number for propagation rules: " << Arg
+                   << '\n';
+    } else
----------------
Do we emit an error for this case?


================
Comment at: lib/StaticAnalyzer/Checkers/Yaml.h:1
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/ADT/APInt.h"
----------------
Header blurb (licence stuff etc)!


================
Comment at: test/Analysis/taint-generic.c:1-2
-// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
-// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
 
----------------
Could you please use line breaks here? I know it isn't your making, but if we're touching it anyways :^)

```
// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
// RUN:   -DFILE_IS_STRUCT \
// RUN:   -analyzer-checker=alpha.security.taint \
// RUN:   -analyzer-checker=core \
// RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
// RUN:   -analyzer-config \
// RUN:     alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
```

And also a testcase for an incorrect checker option:
```

// RUN: not %clang_analyze_cc1 -verify %s \
// RUN:   -analyzer-checker=alpha.security.taint \
// RUN:   -analyzer-config \
// RUN:     alpha.security.taint.TaintPropagation:Config=justguessit \
// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FILE

// CHECK-INVALID-FILE: (frontend): invalid input for checker option
// CHECK-INVALID-FILE-SAME:        'alpha.security.taint.TaintPropagation:Config',
// CHECK-INVALID-FILE-SAME:        that expects a valid yaml file
```
something like that.


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

https://reviews.llvm.org/D59555