[PATCH] D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I've just one thing to add.



================
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:149
+    C.addTransition(State);
     return;
+  }
----------------
xazax.hun wrote:
> Nit: This return is redundant.
Because of how easy it is to accidentally split the state, i'm on a brink of declaring `return` after `addTransition` a good practice.


================
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:207
 
-  if (mayInvalidateBuffer(Call)) {
-    if (const PtrSet *PS = State->get<RawPtrMap>(ObjRegion)) {
-      // Mark all pointer symbols associated with the deleted object released.
-      const Expr *Origin = Call.getOriginExpr();
-      for (const auto Symbol : *PS) {
-        // NOTE: `Origin` may be null, and will be stored so in the symbol's
-        // `RefState` in MallocChecker's `RegionState` program state map.
-        State = allocation_state::markReleased(State, Symbol, Origin);
-      }
-      State = State->remove<RawPtrMap>(ObjRegion);
-      C.addTransition(State);
-      return;
+void InnerPointerChecker::checkPreCall(const CallEvent &Call,
+                                       CheckerContext &C) const {
----------------
I believe that this should also go into `PostCall`. Symbols aren't released until some point //within// the call.


================
Comment at: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp:212
+  // Check [string.require] / first point.
+  if (const auto *FC = dyn_cast<AnyFunctionCall>(&Call)) {
+    const FunctionDecl *FD = FC->getDecl();
----------------
xazax.hun wrote:
> Shouldn't we also check if the function is a standard library function? Or do we assume that user functions also invalidate the strings?
That's right, it's an important thing to check.


Repository:
  rC Clang

https://reviews.llvm.org/D49656