[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

uabelho added a comment. In D108695#2999677 , @Szelethus wrote: > rGfb4d590a622f4031900516360c07ee6ace01c5e6 > should > sort this out! Yes now it works. Thank you! Repository:

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus added a comment. rGfb4d590a622f4031900516360c07ee6ace01c5e6 should sort this out! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108695/new/

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

uabelho added a comment. In D108695#2999432 , @steakhal wrote: > In D108695#2999378 , @uabelho wrote: > >> Hi @Szelethus >> A couple of tests fail for me on trunk with this patch > > Uh, that's my unittest :D >

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

steakhal added a comment. In D108695#2999378 , @uabelho wrote: > Hi @Szelethus > A couple of tests fail for me on trunk with this patch Uh, that's my unittest :D I suspect you are running some sort of CI where you set up Z3. I suspect we should

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus added a comment. I'll attend to this ASAP. Thanks for the heads up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108695/new/ https://reviews.llvm.org/D108695 ___ cfe-commits mailing list

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

uabelho added a comment. Hi @Szelethus A couple of tests fail for me on trunk with this patch: Failed Tests (3): Clang-Unit :: StaticAnalyzer/./StaticAnalysisTests/FalsePositiveRefutationBRVisitorTestBase.UnSatAtErrorNodeDueToRefinedConstraintNoReport Clang-Unit ::

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

This revision was automatically updated to reflect the committed changes. Closed by commit rG0213d7ec0c50: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D108695?vs=370549=372217#toc

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

paquette added a comment. I just reverted this again because of a bot failure here: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/23380/consoleFull Please fix and recommit! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

This revision was automatically updated to reflect the committed changes. Closed by commit rGa375bfb5b729: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D108695?vs=370274=370549#toc

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

thakis added a comment. This seems to break tests: http://45.33.8.238/linux/54991/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108695/new/

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7d0e62bfb773: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire… (authored by Szelethus). Changed prior to commit:

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks! My concerns are addressed, looks good to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108695/new/ https://reviews.llvm.org/D108695

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:683-685 + /// between \p CallEnterN and \p CallExitEndN. Mind that the stack frame + /// retrieved from a CallEnter is the *caller's* stack frame! The

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:361 + while (!SCtx->inTopFrame()) { +auto p = FramesModifying.insert(SCtx); +if (!p.second) martong wrote: >

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus updated this revision to Diff 369935. Szelethus marked 5 inline comments as done. Szelethus added a comment. Herald added a subscriber: mgorny. - Make sure that the matching CallExitEnd is retrieved. - Add a unit test to neatly demonstrate how the visitor is intended to be used. - Fix

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:675 + /// retrieved from a CallEnter is the *caller's* stack frame! The inlined + /// function's stack should be retrieved from \p CallExitBeginN. + virtual

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

martong added a comment. Nice work! Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:361 + while (!SCtx->inTopFrame()) { +auto p = FramesModifying.insert(SCtx); +if (!p.second) Why don't we add the stack frame here to

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

NoQ added a comment. In D108695#2967885 , @Szelethus wrote: > It should! But you have a point, I don't have the code to prove it right > away. Maybe if I factored out the symbol part into > NoStateChangeToSymbolVisitor, as teased in D105553#2864318 >

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus added a comment. In D108695#2967540 , @NoQ wrote: > Would this work correctly when the property is changed but then reverted to > its original state? This probably can't happen to MallocChecker (what has > been freed cannot be unfreed) but it

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

NoQ added a comment. Would this work correctly when the property is changed but then reverted to its original state? This probably can't happen to MallocChecker (what has been freed cannot be unfreed) but it may happen to eg. PthreadLockChecker or to, well, Stores. In such cases it would be

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus added a comment. I also added new dump methods -- I had a trouble before realizing that `CallEnter`'s stack frame is actually the *caller's*, not the *callee's* stack frame. Changes in `findModifyingFrames` aim to make the function more readable, and make sure that the correct stack

[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

Szelethus created this revision. Szelethus added reviewers: NoQ, martong, balazske, vsavchenko, steakhal, ASDenysPetrov. Szelethus added a project: clang. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun,