[llvm] r340807 - Fix in getAllocationDataForFunction

[Matthew Parkinson via llvm-commits]

So we were seeing issues with requesting something to be 
   MallocOrCallocLike

So if you query 
   getAllocationDataForFunction
with 
   MallocOrCallocLike

Then 
  `if ((FnData->AllocTy & AllocTy) == 0) return None`
The current fix, will return None, for anything that doesn't have one of the bits set. So matches lots of stuff.

The original test 
  `if ((FnData->AllocTy & AllocTy) != AllocTy) return None`
Will return None for everything. Nothing has all those bits set.

Andy, I think the issue for your code is that you want a 

enum AllocType : uint8_t {
  OpNewLike          = 1<<0, // allocates; never returns null
  MallocLike           = 1<<1,
  MallocOrNewLike         = MallocLike  | OpNewLike,
  ...
};

Sorry if I have misunderstood something.

Matt

-----Original Message-----
From: David Chisnall <dc552 at hermes.cam.ac.uk> On Behalf Of David Chisnall
Sent: 28 August 2018 18:05
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: 
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.o
> rg%2Fviewvc%2Fllvm-project%3Frev%3D340807%26view%3Drev&data=02%7C0
> 1%7Cmattpark%40microsoft.com%7C022b5d243e5d40b8517d08d60d089ae4%7C72f9
> 88bf86f141af91ab2d7cd011db47%7C1%7C0%7C636710727960849332&sdata=wS
> QOx0KWrMSL0MHwpCoASEVC2keN5TajZJpeCe%2FC95U%3D&reserved=0
> 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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Frevie
> ws.llvm.org%2FD50959&data=02%7C01%7Cmattpark%40microsoft.com%7C022
> b5d243e5d40b8517d08d60d089ae4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7
> C0%7C636710727960849332&sdata=%2FpJHJlER6z%2FvPxSWyE9jE%2BvOEpZ5Mh
> DKK2rp0p4QwwY%3D&reserved=0
> 
> Modified:
>    llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
> 
> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
> URL: 
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.o
> rg%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FAnalysis%2FMemoryBui
> ltins.cpp%3Frev%3D340807%26r1%3D340806%26r2%3D340807%26view%3Ddiff&amp
> ;data=02%7C01%7Cmattpark%40microsoft.com%7C022b5d243e5d40b8517d08d60d0
> 89ae4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636710727960849332&
> amp;sdata=yF5eflZbKra5ZjjhlB0XTFWUXQbq3hoUyEToqIgpq7E%3D&reserved=
> 0 
> ======================================================================
> ========
> --- 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
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.
> llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits&data=02%7C0
> 1%7Cmattpark%40microsoft.com%7C022b5d243e5d40b8517d08d60d089ae4%7C72f9
> 88bf86f141af91ab2d7cd011db47%7C1%7C0%7C636710727960849332&sdata=wg
> %2BKrs7BPB4arlrM8J0MQnoRBoi7mvI7bT6kd71i8gw%3D&reserved=0
>