[PATCH] D142354: [analyzer] Create a stub for an std::variant checker

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

Szelethus added a comment.

In D142354#4078450 <https://reviews.llvm.org/D142354#4078450>, @NoQ wrote:

> Interesting, what specific goals do you have here? Are you planning to find specific bugs (eg. force-unwrap to a wrong type) or just to model the semantics?

Hi!

Meant to write this comment yesterday, but here we go. My idea was aiming for both of the goals you mentioned:

1. Emit diagnostics on improper usages of `std::variant`, which mostly boils down retrieving a field through a wrong type, yes. This will be, in my view, the main showpiece of the thesis.
2. Model the semantics.

In this or in a followup patch I think we should demonstrate with a few tests what we expect the checker to be capable of.

> In the latter case, have you explored the possibility of force-inlining the class instead, like I suggested in the thread?

I have done some tests then, and figured that the analyzer can't really follow what happens in std::variant. I admit, now that I've taken a more thorough look, I was wrong! Here are some examples.

  std::variant<int, std::string> v = 0;
  int i = std::get<int>(v);
  clang_analyzer_eval(i == 0); // expected-warning{{TRUE}}
  10 / i; // FIXME: This should warn for division by zero!



  std::variant<int, std::string> v = 0;
  std::string *sptr = std::get_if<std::string>(&v);
  clang_analyzer_eval(sptr == nullptr); // expected-warning{{TRUE}}
  *sptr = "Alma!"; // FIXME: Should warn, sptr is a null pointer!



  void g(std::variant<int, std::string> v) {
    if (!std::get_if<std::string>(&v))
      return;
    int *iptr = std::get_if<int>(&v);
    clang_analyzer_eval(iptr == nullptr); // expected-warning{{TRUE}}
    *iptr = 5; // FIXME: Should warn, iptr is a null pointer!
  }

In neither of these cases was a warning emitted, but that was a result of bug report suppression, because the exploded graph indicates that the errors were found.

We will need to teach these suppression strategies in these cases, the heuristic is too conservative, and I wouldn't mind some `NoteTag`s to tell the user something along the lines of "Assuming this variant holds and std::string value".

> Have you found a reasonable amount of code that uses `std::variant`, to test your checker on?

Acid <https://github.com/EQMG/Acid> has a few, csv-parser <https://github.com/ashaduri/csv-parser> in one place, there is a couple in config-loader <https://github.com/netcan/config-loader> and cista <https://github.com/felixguendling/cista>, and a surprising amount in userver <https://github.com/userver-framework/userver>. Though its a question how painful it is to set up their dependencies.


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

https://reviews.llvm.org/D142354