llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.36k stars 11.71k forks source link

miscompilation: %rbx is reused across cpuid #18281

Open kcc opened 10 years ago

kcc commented 10 years ago
Bugzilla Link 17907
Version trunk
OS Linux
Attachments bug.ll
CC @topperc,@RKSimon,@rnk,@rotateright

Extended Description

I have a cpuid function implemented like this:

void __cpuid(int cpu_info[4], int info_type) {                                   
  __asm__ volatile("cpuid \n\t"                                                  
                   : "=a"(cpu_info[0]), "=b"(cpu_info[1]), "=c"(cpu_info[2]),    
                     "=d"(cpu_info[3])                                           
                   : "a"(info_type));                                            
}

The asm constraints look correct -- they say that the instruction clobbers %rbx However in some circumstances, the codegen doesn't seem to honor this. This happens when I build a .cc files with locally modified AddressSanitizer. I don't have a proper .cc example, only a .ll example:

% llc bug.ll 
        pushq   %rbx
...
        movq    %rsp, %rbx
...
        cpuid
        movq    128(%rbx), %rsi

Here %rbx is used right after cpuid which clobbers %rbx

So far I was not able to reproduce this bug w/o my local modification in AddressSanitizer.

Any suggestion?

rnk commented 6 years ago

No. It's a long standing issue.

llvmbot commented 6 years ago

I wonder if there's any update on this? I still observe the same issue from ToT. Thanks!

rnk commented 10 years ago

Further reduced test case I believe this is a further reduction of the miscompile for x86_64 Linux:

$ cat cpuid.cpp
void __attribute__((noinline)) use_local(int *l) { *l = 0; }

int main(int argc, char **argv) {
  __attribute__((aligned(32))) int *cpu_info;
  int local;
  if (argv)
    cpu_info = (int *)__builtin_alloca(sizeof(int) * 4);
  __asm__ volatile("cpuid"
                   : "=a"(cpu_info[0]), "=b"(cpu_info[1]), "=c"(cpu_info[2]),
                     "=d"(cpu_info[3])
                   : "a"(0));
  use_local(&local);
  return cpu_info[0];
}

$ ./bin/clang cpuid.cpp -o cpuid -O1 -mstackrealign && ./cpuid
Segmentation fault (core dumped)
rnk commented 10 years ago

I think this is a dup of llvm/llvm-project#17204 . ASan + coverage + -mstackrealign (I'm assuming you are using that) was creating a dynamic alloca, which forced us to use 3 registers: rbp, rbx (esi for 32-bit), and rsp to address the incoming args, local vars, and outgoing args respectively.

I can't investigate conclusively at the moment, though.

kcc commented 10 years ago

r196973 seems to have fixed this miscompile. Reid, do you think this was the actual fix, or just a coincidence?

kcc commented 10 years ago

Well the comment in Host.cpp implies that gcc is also doing the same thing. I haven't verified myself though.

The comment is 4 years old (http://llvm.org/viewvc/llvm-project?view=revision&revision=88768), I would not be surprised if it's not relevant any more.

The recent gcc understands the "=b" constraint very well:

% g++ -O2 -c  cpuid.cc && objdump -d cpuid.o | grep cpuid -A 2 
 120:   0f a2                   cpuid  
 122:   48 8d 7c 24 74          lea    0x74(%rsp),%rdi
 127:   89 5c 24 74             mov    %ebx,0x74(%rsp)

Same for clang:

% clang++ -O2 -c  cpuid.cc && objdump -d cpuid.o | grep cpuid -A 2
 137:   0f a2                   cpuid  
 139:   89 44 24 40             mov    %eax,0x40(%rsp)
 13d:   89 5c 24 44             mov    %ebx,0x44(%rsp)

But with the extra register pressure caused by asan something somewhere breaks badly.

% clang++ -fsanitize=address -mllvm -asan-coverage   -O2 -c  cpuid.cc && objdump 
-d cpuid.o | grep cpuid -2 
 341:   0f a2                   cpuid  
 343:   48 8b 73 78             mov    0x78(%rbx),%rsi
topperc commented 10 years ago

Well the comment in Host.cpp implies that gcc is also doing the same thing. I haven't verified myself though.

kcc commented 10 years ago

I believe this is because EBX/RBX is reserved for the global offset table for position independent code. gcc should exhibit the same behavior.

Which is why the calls to cpuid inside of llvm manually juggle ebx/rbx. See the code in lib/Support/Host.cpp.

Not sure I understood this comment. Do you mean this is not a bug? Or do you suggest that the bug is in lib/Support/Host.cpp?

Any other suggestion?

kcc commented 10 years ago

cpuid.cc

kcc commented 10 years ago

With today's trunk (r195222) one can reproduce the failure like this (x86_64 linux):

% clang -fsanitize=address -mllvm -asan-coverage=1 -O2 cpuid.cc  && ./a.out 
ASAN:SIGSEGV
=================================================================
==8194==ERROR: AddressSanitizer: SEGV on unknown address 0x0000756e65c7 (pc 0x000000465246 sp 0x7fff68152340 bp 0x7fff681524b0 T0)
    #0 0x465245 in main (/home/kcc/tmp/a.out+0x465245)

% gdb ./a.out 
Reading symbols from /home/kcc/tmp/./a.out...done.
(gdb) r
...
Program received signal SIGSEGV, Segmentation fault.
0x0000000000465246 in main ()
(gdb) disassemble 
   0x0000000000465244 <+580>:   cpuid  
=> 0x0000000000465246 <+582>:   mov    0x80(%rbx),%rsi

This is clearly wrong.

topperc commented 10 years ago

I believe this is because EBX/RBX is reserved for the global offset table for position independent code. gcc should exhibit the same behavior.

Which is why the calls to cpuid inside of llvm manually juggle ebx/rbx. See the code in lib/Support/Host.cpp.