The issues highlighted here have been partially (one could argue mostly) solved by D105821. That patch hasn't been merged because it needs more tests and also needs to be split.

In D108912#2982201, @Szelethus wrote:

In D108912#2982169, @RedDocMD wrote:

I suppose that the SmartPtrModelling patches from GSoC this year shouldn't be added since they only involve an alpha checker.

Actually, you also patched the enabled-by-default modeling part of smart pointers, haven't you? Those definitely deserve their mention!

I suppose that the SmartPtrModelling patches from GSoC this year shouldn't be added since they only involve an alpha checker.

Connecting to MallocChecker

Further pointer escape

Never gonna give you up.

Bug fix in modelling

I have incorporated the bug-fixes suggested last meeting (except the pointer escape one). And it seems to have had dramatic results - now the only extra errors being reported are the pointer escape ones (5 of them, from 3 different projects). Some projects are actually reporting that bug reports have been removed due to this patch.

Better modelling, bug fixes

Well some of them are exactly the same type as the Lame class example above. Like: simbody/report-TestArray.cpp-testMoveConstructionAndAssignment-27-1.html#EndPath. (So the incomplete modelling of the destructor is at least one cause. The other reason that you suggested might as well be true).
Btw, the destructor1.txt file from my previous comment should be used to drill down the newly added reports. (Sorry for the inconvenience, sshfs is really slow and so it was more convenient to tar the whole folder and scp it).

Invalidating using inner pointer destructor call

In D106739#2907647, @ASDenysPetrov wrote:

@RedDocMD Could you, please, explain the motivation and add at least one test to verify it works.

On running this patch on the projects directory, a bunch of projects emit false-positives: mostly of the form Potential memory leak. This points to the fact that without calling the destructor of the pointee type, we are going to have a lot of false positives (408 for one project is the worst I have seen). I have attached the result file.

Put in a TODO

Bug fixes, some cleanup

Ah I see.
As a side note, without the "redundant" invalidation that is being done, the analyzer crashes on shared_ptr. (Because the State essentially remains the same and that's what causes the crash).

Invalidating via the CallEvent

Removed a fatal bug

@vsavchenko is this okay?

In D105821#2897006, @NoQ wrote:

the following code doesn't emit any warnings

This code doesn't seem to have any unique_ptrs in it? It's not like you're modeling this custom class as well? Can you try the same with the actual unique_ptr?

It turns out that there is already an option called extra-analyzer-config to pass analyzer configs, like setting ModelSmartPtrDereference to true

But before we go there we should decide whether we want to actually go for inlining (or otherwise default-evaluating) these destructors. If we do, we should probably not spend too much time on improving invalidation in the checker, because default evaluation would do that properly for us anyway (well, it doesn't really dodge any problems such as the absence of the necessary AST so we'll probably have to solve all these problems anyway, just in a different setting). So it's great that we've fixed evalCall for destructors, this could definitely land as a separate patch (tested via debug.AnalysisOrder), but we really need to think what to do next here. So I recommend gathering some data to see if proper destructor evaluation is actually a real problem.

This is a minimal model of destructors in smart-ptr.
Other than the need to probably model the destructor of the pointee, is there anything else to do?

Minimal modelling of destructor

Reformat

Retrieving patch

Removed unnecessary white space

More refactor

Refactored out check

Would this test do?

Added a simple test

Reformatted patch

For some reason this revision did not get automatically closed after commit, manually merging it.

Marked test with FIXME notes

Fixed up tests

Post-rebase cleanup

Post rebase cleanup

Post rebase cleanup

Removed one bug, many more to go

Cleanup, still doesn't work

Is the syntax of specifying expected notes and warnings documented somewhere? I could not find the note-specific syntax.

Fixing up tests

Removed stupid mistakes

Little refactors, one more test

Tests implemented

I will be figuring out some tests tomorrow morning.

Little refactors

Invalidating regions

Major bug fix

Little refactors

Simplify SVal on state split, other refactors

Performing state split on normal comparision ops

Refactored out common block

Fixed bug in enum conversion

Sorry for not updating. Was down with fever.
This patch does *not* work now. operationKindFromOverloadedOperator is broken because the maps don't get populated. I am not entirely sure why this is happening.
Will try to fix tomorrow. @NoQ, @vsavchenko, @xazax.hun, @teemperor do you have a hunch as to why this may be happening?

First try at implementing conversion function from OverloadedOperatorKind to BinaryOperatorKind

Removed dump statement

Refactored code, removed duplications, fixed tests, added some more

We have a failing test here (test at line 473).
Which makes me wonder if the handleComparision function is at all called. This is something I need to check.

Removed re-invention, added tests

In D104616#2835061, @xazax.hun wrote:

In D104616#2835030, @RedDocMD wrote:

Looks like I have wasted a good deal of effort. :(

Sorry about that! :( If we learned anything new in the process it was not wasted effort though.

In D104616#2834770, @xazax.hun wrote:

In D104616#2834714, @NoQ wrote:

Why not simply delegate this job to assume(evalBinOp(...)) over raw pointer values, which already has all this logic written down nicely?

This is what I had in mind, I just did not want to spoil it :)

Logic for handling special cases, when both are unique_ptr

The only method that I think can be realistically modelled is == (and thus !=). If both the operands refer to the same unique_ptr, we know == returns true. If they are not the same, the only way == can return true if the two smart pointers were initialized from the same raw pointer. This is of course a fatal bug in itself. So perhaps we can ignore this case and only consider the first case.
The ordering operators I guess can't be handled because there is no way to statically tell in general the address of some value. We have the following deductions, nevertheless, mathematically:
Let ptr1 and ptr2 be two std::unique_ptr objects.
If (ptr1 == ptr2) is true:

• ptr1 < ptr2 is false
• ptr1 > ptr2 is false
• ptr1 <= ptr2 is true
• ptr1 >= ptr2 is true

If (ptr1 == ptr2) is false, we can't say anything really.