<div dir="ltr"><div dir="ltr"><div dir="ltr">I think that sounds great, and I have thought about adding this later down the line to your tracking interface. The issue with the specific, and rather poor memory leak report I'm investigating is that its not a (lack of a) value change to a region that I want to track down, but rather a change of some arbitrary property on a symbol. Take for instance the following code snippet:<div><br></div><div><font face="monospace">void sink(int *P) {} // no notes<br><br>void f() {<br>  sink(new int(5)); // note: Memory is allocated</font></div><div><font face="monospace">                    // Well hold on, sink() was supposed to deal with</font></div><div><font face="monospace">                    //</font><span style="font-family:monospace"> </span><span style="font-family:monospace">that,</span><span style="font-family:monospace"> this must be a false positive...</span></div><div><font face="monospace">} // warning: Potential memory leak [cplusplus.NewDeleteLeaks]</font><br></div><div><font face="monospace"><br></font></div><div><font face="arial, sans-serif">The fact that the allocated memory leaks is the property of the symbol being dead</font><span style="font-family:arial,sans-serif"> </span><span style="font-family:arial,sans-serif">(SymbolReaper::isDead)</span><span style="font-family:arial,sans-serif"> despite still being marked as allocated after the call to sink(). The fact that sink() didn't help to prevent this error is a property of:</span></div><div><font face="arial, sans-serif"><br>* No region with a lifetime longer then the call to f holds the value of this symbol</font></div><div><font face="arial, sans-serif">* The RefState hasn't changed (from Kind::Allocated or Kind::</font>AllocatedOfSizeZero<span style="font-family:arial,sans-serif">)</span></div><div><span style="font-family:arial,sans-serif"><br></span></div><div><font face="arial, sans-serif">...and to me, it seems like both of these should be checked at bugreporting, not analysis time.</font></div><div><span style="font-family:arial,sans-serif"><br></span></div><div><font face="arial, sans-serif">As these properties could only be checked during tracking, I think a handler the way you describe it (as I understand) would be insufficient. While this really is a MallocChecker-specific problem, I think generalizing NoStoreFuncVisitor to have an overridable "did anything change about this entity during this function call?" would be beneficial for many other similar checkers. That might as well end up as an extension to the existing StoreHandler interface.</font></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 30 Jun 2021 at 15:17, Valeriy Savchenko <<a href="mailto:vsavchenko@apple.com">vsavchenko@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Kristóf,<br>
<br>
In my opinion, this is the checker’s logic and it shouldn’t live inside of `NoStoreFuncVisitor`.  However, I think that `NoStateChangeFuncVisitor` can require an extra step for the users.<br>
<br>
During my big discussion with Artem in the patchset, where I introduced the `Tracker` interface, I suggested to extract different bits of what I called “events” from different parts of `trackExpressionValue`, so that checkers can react on those events.  At first, we make it part of the tracker mechanism, so we detect them during this stage, and later we can put special tags on the nodes.<br>
<br>
Long story short, I think that `NoStore` is event and with that it is similar to `Store`.  We can introduce `NoStoreHandler` (probably we can come up with a better name than that), that produces the note, just like `StoreHandler` does.  And the user can plug in their own handler to produce customized note.<br>
<br>
What do you think about this?<br>
<br>
Valeriy<br>
<br>
> On 30 Jun 2021, at 15:57, Kristóf Umann <<a href="mailto:dkszelethus@gmail.com" target="_blank">dkszelethus@gmail.com</a>> wrote:<br>
> <br>
> Hi!<br>
> <br>
> Just a quick one -- I want to generalize NoStoreFuncVisitor to be able to construct messages such as "Returning without deallocating or changing the ownership of allocated memory". The grand idea is to create a NoStateChangeFuncVisitor base class that can be specialized for what a (lack of a) state change is.<br>
> <br>
> Are there any similar ongoing efforts or shall I proceed?<br>
> <br>
> Cheers,<br>
> Kristóf<br>
<br>
</blockquote></div>