[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:842-849
+      if (Optional<CallExitEnd> CEE = Node->getLocationAs<CallExitEnd>())
         if (CEE->getCalleeContext()->getCallSite() == S)
           break;
-      if (auto SP = Node->getLocationAs<StmtPoint>())
-        if (SP->getStmt() == S)
-          break;
+
+      if (!IsBypass)
+        if (Optional<StmtPoint> SP = Node->getLocationAs<StmtPoint>())
+          if (SP->getStmt() == S)
----------------
Charusso wrote:
> NoQ wrote:
> > Comparing statements is usually insufficient because the same statement may appear multiple times due to recursion. When recursion occurs, you may reach the same statement in a different location context. You should think in terms of (statement, location context) pairs to avoid these problems. Your aim here is to find the `CallExitEnd` node that corresponds to returning from an inlined operator new to the current location context. You should stop searching when you find an unrelated statement in the current location context or when you exit the current location context entirely.
> I have made a little test when we have a 25-second long Static Analyzer run with predefined names and checker. The loop ran 500 times in summary and we have some serious performance impacts at other places.
> 
> We exit the current context to see inlined calls, so that could not work sadly. If you remove that nonsense second condition we run at the same time, so if we have not got any problem since 7 years ago I think it is good to go.
You should break the loop as soon as we go past the new-expression that we've started with in the stack frame that we've started with. That is, you should at most go through the constructor within the new-expression, and then break. You shouldn't explore the whole graph to the root every time the operator-new call isn't inlined.

This might still be slow in some rare cases, but it should be much faster.


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

https://reviews.llvm.org/D62926