[cfe-commits] [PATCH] Fix support for naked C/C++ functions writing to unexpected memory regions in debug builds

[Jeffrey Lim]

On 10 Nov, 2012, at 6:39 AM, Eli Friedman <eli.friedman at gmail.com> wrote:

> 2012/11/8 Jeffrey Lim <jeff at lim.com.au>:
>> 
>> 
>> Hi Eli (or others),
>> 
>> If you could kindly have a look at this updated proposed patch to correct
>> the errors.
>> 
>> The current svn version (r167575) has an additional problem with naked
>> functions than the version released with Xcode due to r165914 which inserts
>> llvm.trap and unreachable annotations in debug builds in naked functions
>> that need to return a value.

<snip>

> 
> Please split the change to factor out
> GenerateSubprogramDebugDescriptor and GenerateStartFunctionBlocks into
> a separate patch.

Will do. I'll discuss the change I needed to make in Verifier.cpp before submitting a newer patch to this list.

> I'm assuming you're leaving in the commented-out lines for review
> purposes?  We wouldn't want to commit something like that.

Exactly. For review only.

<snip>

> -  unsigned N = RI.getNumOperands();
> -  if (F->getReturnType()->isVoidTy())
> -    Assert2(N == 0,
> -            "Found return instr that returns non-void in Function of void "
> -            "return type!", &RI, F->getReturnType());
> -  else
> -    Assert2(N == 1 && F->getReturnType() == RI.getOperand(0)->getType(),
> -            "Function return type does not match operand "
> -            "type of return inst!", &RI, F->getReturnType());
> +  if(!F->isNaked())
> +  {
> +    unsigned N = RI.getNumOperands();
> +    if (F->getReturnType()->isVoidTy())
> +      Assert2(N == 0,
> +              "Found return instr that returns non-void in Function of void "
> +              "return type!", &RI, F->getReturnType());
> +    else
> +      Assert2(N == 1 && F->getReturnType() == RI.getOperand(0)->getType(),
> +              "Function return type does not match operand "
> +              "type of return inst!", &RI, F->getReturnType());
> +  }
> 
> I don't think this is acceptable; this is likely to confuse the
> optimizers.  This really needs some design discussion; please bring it
> up on llvmdev.


I'll post to llvmdev. Thank you so much for your feedback. 

Jeff