[cfe-dev] [analyzer] What's up with c++-allocator-inlining?

[Artem Dergachev via cfe-dev]

Yes, we should remove the old code for c++-allocator-inlining=false. The 
worry we've had back then was that in the new mode we've disabled 
aggressive behavior of MallocChecker in which it reacted to some 
overloaded operator new invocations but i think this was the right thing 
to do and also nobody complained; also nothing prevents us from bringing 
back the old behavior in a much less confusing way.

Using a CXXAllocatorCall sounds wonderful as well. I'm afraid it might 
be technically difficult to do so because our Environment is screwed due 
to CXXNewExpr serving two different purposes, so there's a wrong SVal 
attached to it and CallEvent might be unable to retrieve the right SVal, 
so we're passing it separately. If this issue is solved, we might as 
well provide these callbacks as part of checkPreCall/checkPostCall and 
then abandon the check::NewAllocator callback entirely. More discussion 
on this in http://lists.llvm.org/pipermail/cfe-dev/2017-December/056314.html

I think at this point we might actually do a good job sorting out this 
check::NewAllocator issue because we have a "separate" "Environment" to 
hold the other SVal, which is "objects under construction"! - so we 
should probably simply teach CXXAllocatorCall to extract the value from 
the objects-under-construction trait of the program state and we're good.

On 2/26/20 5:02 PM, Kristóf Umann via cfe-dev wrote:
> Hey!
>
> This is short and sweet. MallocChecker uses both check::newAllocator 
> and check::postStmt<CXXNewExpr> to model aspects of operator new. Mind 
> that these two are redundant not only with each other, but with the 
> already used check::postCall. The reason I can see is handling all 
> values of the analyzer config c++-allocator-inlining.
>
> So, this flag has been true by default for a long-long time, and I 
> personally never changed had the need to change it. Is there a need to 
> keep tiptoeing around it? Here is a patch that tackles the issue, but 
> it quite dated and I'm not too sure about the current state of things.
>
> [analyzer] Add a new checker callback, check::NewAllocator.
> https://reviews.llvm.org/D41406
>
> Also, shouldn't we make check::NewAllocator provide a CXXAllocatorCall 
> rather then a CXXNewExpr and a related under-construction SVal?
>
> Cheers,
> Husi
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev