[llvm] r340807 - Fix in getAllocationDataForFunction

[Kaylor, Andrew via llvm-commits]

Thanks, David.

Can you share an example of the false negatives you were seeing?

-Andy

-----Original Message-----
From: David Chisnall [mailto:dc552 at hermes.cam.ac.uk] On Behalf Of David Chisnall
Sent: Tuesday, August 28, 2018 10:05 AM
To: Kaylor, Andrew <andrew.kaylor at intel.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [llvm] r340807 - Fix in getAllocationDataForFunction

Please revert it if it’s causing problems for you, and reopen the review.  We’re seeing false negatives with the version from before the change, which is breaking memory safety analyses.  It may be that we need to add a second API to handle this correctly.

David

> On 28 Aug 2018, at 17:46, Kaylor, Andrew <andrew.kaylor at intel.com> wrote:
> 
> I believe this change is incorrect. It causes us to report that malloc is a new-like function.
> 
> The way that the allocation kinds are defined, each allocation kind has a unique bit except for malloc-like which shares a bit with new-like. With the old implementation, this allowed us to recognize new-like functions as also being malloc-like, but not the other way around. The other combination masks in the AllocType enum allow for matching any of multiple types, but no function will be defined as being, for instance, MallocOrCallocLike.
> 
> So the old logic in getAllocationDataForFunction was applying the incoming AllocTy as a mask, and if the FnData->AllocTy was unchanged after applying that mask, then it matched.  That is, if all of the bits in the FnData->AllocTy were in the callers mask, then it matched. Otherwise, it did not. I think that's the behavior we want.
> 
> Can this change be reverted?
> 
> -Andy
> 
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf Of David Chisnall via llvm-commits
> Sent: Tuesday, August 28, 2018 1:59 AM
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r340807 - Fix in getAllocationDataForFunction
> 
> Author: theraven
> Date: Tue Aug 28 01:59:06 2018
> New Revision: 340807
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=340807&view=rev
> Log:
> Fix in getAllocationDataForFunction
> 
> Summary:
> Correct to use set like behaviour of AllocType.  Should check for subset, not precise value.
> 
> Reviewers: theraven
> 
> Reviewed By: theraven
> 
> Subscribers: hiraditya, llvm-commits
> 
> Differential Revision: https://reviews.llvm.org/D50959
> 
> Modified:
>    llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
> 
> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=340807&r1=340806&r2=340807&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Tue Aug 28 01:59:06 2018
> @@ -150,7 +150,7 @@ getAllocationDataForFunction(const Funct
>     return None;
> 
>   const AllocFnsTy *FnData = &Iter->second;
> -  if ((FnData->AllocTy & AllocTy) != FnData->AllocTy)
> +  if ((FnData->AllocTy & AllocTy) == 0)
>     return None;
> 
>   // Check function prototype.
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>