[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

Umann Kristóf via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 25 09:55:49 PST 2018


Szelethus added a comment.

I think the reason why the printed message was either along the lines of "this is 0" and "this is non-0", is that we don't necessarily know what constraint solver we're using, and adding more non-general code `BugReporter` is most probably a bad approach. How about we tinker with `ContraintManager` a little more, maybe add an `explainValueOfVarDecl`, that would return the value of a variable in a given `ProgramState`? We could implement it for `RangeConstraintSolver` (essentially move the code you already wrote there), and leave a `TODO` with a super generic implementation for Z3.

Although, I can't quite write an essay on top of my head about golden rules of constraint solving, so take my advice with a grain of salt.



================
Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:164
 class ConditionBRVisitor final : public BugReporterVisitor {
+  bool IsAssuming;
+
----------------
We only get to know what this field is for while reading the actual implementation, please add comments.


================
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1892-1920
+  const auto CurrentCR = N->getState()->get<ConstraintRange>();
+  const auto PredCR = PredState->get<ConstraintRange>();
+  const auto PredPredCR = PredPredState->get<ConstraintRange>();
+
+  // We only want to check BlockEdges once so we have to determine if the change
+  // of the range information is not happened because of dead symbols.
+  //
----------------
`ConstraintRange`, as far as I know, is the data  `RangedConstraintManager` stores in the GDM. What if we're using a different constraint solver? I think it'd be better to take the more abstract approach, extend `ConstraintManager`'s interface with an `isEqual` or `operator==` pure virtual method, and implement it within it's descendants.


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

https://reviews.llvm.org/D53076





More information about the cfe-commits mailing list