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

[Artem Dergachev via cfe-dev]

On 2/26/20 6:45 PM, Artem Dergachev wrote:
> 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.

Nvm, i'm wrong. CXXAllocatorCall should have the value before the cast 
but "objects under construction" and check::NewAllocator should have the 
value after the cast. So there's still no easy way to fix CXXAllocatorCall.

>
> 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
>