[llvm-commits] PATCH: review request: X86: Use StackRegister instead of FrameRegister in getFrameIndexReference if stack needs realignment

[Eric Christopher]

On Apr 27, 2012, at 6:41 AM, Alexey Samsonov <samsonov at google.com> wrote:

> Hi, llvm-commits!
> 
> It looks like debug info generated for variables is incorrect in case the stack needs realignment. Consider:
> $ cat test.cc 
> #include <stdio.h>
> void run() {
>   int x __attribute__((aligned(32))); // enforce alignment
>   printf("%p\n", &x);
> }
> $ clang++ -g -c test.cc
> $ objdump -d test.o
> <...>
>    0:   55                      push   %rbp
>    1:   48 89 e5                mov    %rsp,%rbp
>    4:   48 81 e4 e0 ff ff ff    and    $0xffffffffffffffe0,%rsp   <----- stack gets aligned here
>    b:   48 83 ec 40             sub    $0x40,%rsp
>    f:   48 8d 3c 25 00 00 00    lea    0x0,%rdi
>   16:   00 
>   17:   48 8d 74 24 20          lea    0x20(%rsp),%rsi
>   1c:   b0 00                   mov    $0x0,%al
>   1e:   e8 00 00 00 00          callq  23 <_Z3runv+0x23>
> <...>
> $ readelf -wi test.o
> <...>
>  <3><55>: Abbrev Number: 4 (DW_TAG_variable)
>     <56>   DW_AT_name        : (indirect string, offset: 0xa4): x       
>     <5a>   DW_AT_decl_file   : 1        
>     <5b>   DW_AT_decl_line   : 3        
>     <5c>   DW_AT_type        : <0x65>   
>     <60>   DW_AT_location    : 2 byte block: 91 20      (DW_OP_fbreg: 32)
> 
> That is, dwarf debug info says that "x" is located at offset 0x20 from frame register (%rbp), which is wrong:
> 1) the offset from %rbp should clearly be negative
> 2) we can't calculate it that easy, as stack is aligned (%rsp is changed) after old rsp value is assigned to rbp.
> 3) 0x20 is actually the offset from *%rsp*, not %rbp
> 
> I may be wrong, but X86FrameLowering::getFrameIndexOffset in case the stack needs realignment (http://llvm.org/docs/doxygen/html/X86FrameLowering_8cpp_source.html#l01144)
> seems to be calculating the offset from %rsp. If this is true, we should enforce the usage of rsp when calculating variable locations, even if we have frame pointers. This can be fixed by applying the patch.
> 
> Codereview link: http://codereview.appspot.com/6127054/

Looks reasonable, please apply.

Thanks!

-eric