[LLVMdev] Verifier should not make any assumptions about calls to "malloc"

[Victor Hernandez]

Duncan,

Thanks for brining the Ada issue to my attention.

On Sep 22, 2009, at 6:11 AM, Duncan Sands wrote:

> Hi Victor, this code from the verifier broke the Ada front-end build:
>
>  const Module* M = CI.getParent()->getParent()->getParent();
>  Constant *MallocFunc = M->getFunction("malloc");
>
>  if (CI.getOperand(0) == MallocFunc) {
>    const PointerType *PTy =
> PointerType::getUnqual(Type::getInt8Ty(CI.getParent()->getContext()));
>    Assert1(CI.getType() == PTy, "Malloc call must return i8*", &CI);
>  }
>
> I think it is completely wrong for the verifier to be checking  
> anything
> about calls to functions that happen to be called "malloc".  What if I
> have my own runtime in which "malloc" is completely different to the
> usual one?  From my reading of the gcc docs, malloc is not provided
> in a freestanding environment and thus cannot be assumed to be the
> normal malloc.

What does the Ada front-end declare malloc as?

>
> I think this code should be removed from the verifier.  Instead,
> isMalloc should also check the number of parameters and their types,
> as well as the return value.

The verifier code is needed because when a malloc return value is used  
directly, not via a bitcast, the type of the malloc is determined to  
be i8*.  But that could be updated to use the declared return type of  
malloc.  I need to understand more about how this is breaking Ada to  
determine how to resolve this.  Removing this check from the verifier  
could end up being the resolution.

>
> Actually isMalloc also seems bogus.  In a freestanding environment
> there is no reason that a function that happens to be called "malloc"
> should have anything to do with memory allocation.  Do you have a
> plan to handle this?  Shouldn't all malloc manipulations be done from
> SimplifyLibcalls?

The free-standing environment with a non-memory-allocating malloc  
function will be handled via -fno-builtins.  I don't believe that  
isMalloc is bogus, just that llvm does not support the ability to turn  
it off.  Here is some previous discussion about this topic:

On Sep 11, 2009, at 11:30 AM, Chris Lattner wrote:

> On Sep 11, 2009, at 10:22 AM, Dan Gohman wrote:
>
>> If MallocHelper is going to replace MallocInst, how will it support
>> -fno-builtins?  With MallocInst, one at least had the option of
>> omitting the RaiseAllocations pass (though it seems llvm-gcc and
>> clang don't actually do this, which is a bug). With MallocHelper,
>> how can -fno-builtins be implemented?
>
> In order to support -fno-builtin-foo, we'd want to introduce a  
> function attribute saying "not a builtin" and stick it on foo.  This  
> doesn't exist today, but shouldn't be particularly hard.


Victor

>
> Ciao,
>
> Duncan.