[llvm] r340807 - Fix in getAllocationDataForFunction

David Chisnall via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 28 10:05:25 PDT 2018


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
> 



More information about the llvm-commits mailing list