[PATCH v3] Bug 14526 - Make malloc() static analysis M_ZERO-aware

[Meyer, Conrad]

On Fri, Mar 21, 2014 at 1:11 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> On Mar 20, 2014, at 6:08 , Meyer, Conrad <conrad.meyer at isilon.com> wrote:
>
>> +  mutable Optional<uint64_t> Val_M_ZERO;
>
> Let's rename this something generic, like KernelZeroFlagVal;
>

Easy to do.

>
>>
>>   void initIdentifierInfo(ASTContext &C) const;
>>
>>   /// \brief Determine family of a deallocation expression.
>>   AllocationFamily getAllocationFamily(CheckerContext &C, const Stmt *S) const;
>> @@ -251,10 +255,15 @@ private:
>>   static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
>>                                      SVal SizeEx, SVal Init,
>>                                      ProgramStateRef State,
>>                                      AllocationFamily Family = AF_Malloc);
>>
>> +  // Check if this malloc() uses M_ZERO or __GFP_ZERO (treat like calloc).
>> +  llvm::Optional<ProgramStateRef>
>> +  MallocZero(const CallExpr *CE, CheckerContext &C,
>> +             const ProgramStateRef &State) const;
>> +
>
> How about something like "performKernelMalloc"? The special thing here is that it handles the two- and three-argument forms if it knows how. We may want to add support for the other flags some day.

Sure.

>
> Are there ever going to be systems that have both kmalloc and malloc-with-flags?

BSD has a Linux kernel compatibility layer for some dual-licensed drivers ("OFED," I think, and possibly others). And I wouldn't be surprised to see the opposite direction either. So while I don't *know* that such a system exists, it wouldn't be super surprising either.

>>
>> +llvm::Optional<ProgramStateRef> MallocChecker::MallocZero(const CallExpr *CE,
>> +  CheckerContext &C, const ProgramStateRef &State) const {
>> +  // 3-argument malloc(), as commonly used in {Free,Net,Open}BSD Kernels:
>> +  //
>> +  // void *malloc(unsigned long size, struct malloc_type *mtp, int flags);
>> +  //
>> +  // One of the possible flags is M_ZERO, which means 'give me back an
>> +  // allocation which is already zeroed', like calloc.
>> +
>> +  // 2-argument kmalloc(), as used in the Linux kernel:
>> +  //
>> +  // void *kmalloc(size_t size, gfp_t flags);
>> +  //
>> +  // Has the similar flag value __GFP_ZERO.
>> +
>> +  // This logic is largely cloned from O_CREAT in UnixAPIChecker, maybe some
>> +  // code could be shared.
>> +
>> +  llvm::Triple::OSType os = C.getASTContext().getTargetInfo().getTriple().getOS();
>> +  llvm::Optional<ProgramStateRef> RetVal;
>> +
>> +  if (!Val_M_ZERO.hasValue()) {
>> +    if (os == llvm::Triple::FreeBSD)
>> +      Val_M_ZERO = 0x0100;
>> +    else if (os == llvm::Triple::NetBSD)
>> +      Val_M_ZERO = 0x0002;
>> +    else if (os == llvm::Triple::OpenBSD)
>> +      Val_M_ZERO = 0x0008;
>> +    else if (os == llvm::Triple::Linux)
>> +      // __GFP_ZERO
>> +      Val_M_ZERO = 0x8000;
>> +    else
>> +      // FIXME: We need a more general way of getting the M_ZERO value.
>> +      // See also: O_CREAT in UnixAPIChecker.cpp.
>> +
>> +      // Fall back to normal malloc behavior on platforms where we don't
>> +      // know M_ZERO.
>> +      return RetVal;
>
> This can be written "return None;". "None" implicitly converts to an empty Optional.

Is None a special global or #define in LLVM/Clang? I haven't heard of it in C++ before. But that's easy enough to change.

>
> If you use this everywhere, you don't need to predeclare RetVal.

Sure.

>
> Other than that this looks pretty good!
>
> Jordan

Cool. I will make the suggested changes and re-send.

Thanks,
Conrad