[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I don't think you actually need active support for invoking exit handlers path-sensitively at the end of `main()` in order to implement your checker. You can still find out in a path-insensitive manner whether any given function acts as an exit handler, like you already do. Then during path-sensitive analysis of that function you can warn at any invocation of exit().

I do believe that you want to implement this check as a path-sensitive check as long as you want any sort of interprocedural analysis (i.e., warn when an exit handler calls a function that calls a function ... that calls a function that calls `exit()` - and you do already have such tests). This is because of the following potential situation:

  void foo(bool already_exiting) {
    if (!already_exiting)
      exit();
  }
  
  void bar() {
    foo(true);
  }
  
  void baz() {
    foo(false);
  }
  
  int main() {
    atexit(bar);
    return 0;
  }

In this case `bar()` is an exit handler that calls `foo()` that calls `exit()`. However the code is correct and no warning should be emitted, because `foo()` would never call `exit()` //when called from// `bar()` //specifically//. Precise reasoning about such problems requires path-sensitive analysis. Of course it's up to you to decide what to do with these false positives - whether you'll be ok with having them, or choose to suppress with an imprecise heuristic - but that's one of the possible reasons to consider reimplementing the checker via path sensitive analysis that the static analyzer provides.

We've had a similar problem with the checker that warns on calling pure virtual functions in constructors/destructors (which is undefined behavior). Such checker had to be path sensitive in order to be interprocedural for the exact same unobvious reason. We've decided to re-implement it with path-sensitive analysis and now we're pretty happy about that decision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717