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

[Eli Friedman]

2012/11/6 Jeffrey Lim <jeff at lim.com.au>:
>
> On 7 Nov, 2012, at 5:59 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
>
>> 2012/11/6 Jeffrey Lim <jeff at lim.com.au>:
>>>
>>> First time post to this list -- I think I posted to the wrong place before
>>> (cfe-dev). Hopefully I'm right this time.
>>>
>>> The problem I was noticing is that a naked function would cause very
>>> unexpected behaviour. I simplified it to this test case:
>>>
>>> test.c:
>>>
>>> __attribute__((naked)) void NakedTest(int value, int value2)
>>> {
>>> asm("");
>>> }
>>>
>>>
>>> clang -S test.cpp
>>> test.s:
>>>
>>> .section __TEXT,__text,regular,pure_instructions
>>> .globl __Z9NakedTestii
>>> .align 4, 0x90
>>> __Z9NakedTestii:                        ## @_Z9NakedTestii
>>> .cfi_startproc
>>> ## BB#0:
>>> movl %edi, -4(%rbp)
>>> movl %esi, -8(%rbp)
>>> ## InlineAsm Start
>>> ## InlineAsm End
>>> ret
>>> .cfi_endproc
>>>
>>>
>>> clang -flto -S test.cpp
>>> test.s:
>>>
>>> define void @_Z9NakedTestii(i32 %value, i32 %value2) nounwind uwtable
>>> noinline ssp naked {
>>> entry:
>>>  %value.addr = alloca i32, align 4
>>>  %value2.addr = alloca i32, align 4
>>>  store i32 %value, i32* %value.addr, align 4
>>>  store i32 %value2, i32* %value2.addr, align 4
>>>  call void asm sideeffect "", "~{dirflag},~{fpsr},~{flags}"() nounwind,
>>> !srcloc !0
>>>  ret void
>>> }
>>>
>>> The bolded instructions above are generated in debug builds, causing
>>> problems. This happens in all architectures that I've tested on (ARM, x86,
>>> x64)
>>>
>>>
>>> I've attached a patch file which I fixes the problem. I don't know if I've
>>> done it the right way (I only downloaded and looked at the source for clang
>>> for the first time 2 hours ago), but here's the newer result:
>>>
>>> clang -S test.cpp
>>> test.s:
>>> .section __TEXT,__text,regular,pure_instructions
>>> .globl __Z9NakedTestii
>>> .align 4, 0x90
>>> __Z9NakedTestii:                        ## @_Z9NakedTestii
>>> .cfi_startproc
>>> ## BB#0:                                ## %entry
>>> ## InlineAsm Start
>>> ## InlineAsm End
>>> ret
>>> .cfi_endproc
>>>
>>>
>>> clang -flto -S test.cpp
>>> test.s:
>>>
>>> define void @_Z9NakedTestii(i32, i32) nounwind uwtable noinline ssp naked {
>>> entry:
>>>  call void asm sideeffect "", "~{dirflag},~{fpsr},~{flags}"() nounwind,
>>> !srcloc !0
>>>  ret void
>>> }
>>>
>>> Any comments, or alternative fixes to this problem would be greatly
>>> appreciated!
>>
>> I would like to see a more principled approach here... scattering
>> checks for the naked attribute all over the function emission code
>> seems like a bad idea.  (clang with just your patch doesn't handle,
>> for example, the case where the return type is not void consistently,
>> the case where the function in question is a member function, etc.)
>>
>> -Eli
>
> Thanks for the feedback -- you're absolutely right about the fail cases.
>
> Obviously I could add in even more checks in the function emission code, but do you have any thoughts on how the right way to address this might look like? I'd love to fix it in the proper way with a little guidance.

I would suggest adding a special case to CodeGenFunction::GenerateCode
which does exactly what is necessary for naked functions.

-Eli