miscompilation: %rbx is reused across cpuid

[Quuxplusone]

Bugzilla Link	PR17907
Status	NEW
Importance	P normal
Reported by	Kostya Serebryany (kcc@google.com)
Reported on	2013-11-13 02:29:24 -0800
Last modified on	2019-03-16 07:08:08 -0700
Version	trunk
Hardware	PC Linux
CC	craig.topper@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, rafael@espindo.la, rnk@google.com, spatel+llvm@rotateright.com, twoh@fb.com
Fixed by commit(s)
Attachments	bug.ll (19722 bytes, application/octet-stream) cpuid.cc (982 bytes, text/x-c++src) cpuid.cpp (443 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 11531
bug.ll

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?

[Quuxplusone]

Attached bug.ll (19722 bytes, application/octet-stream): bug.ll

[Quuxplusone]

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.

[Quuxplusone]

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.

[Quuxplusone]

(In reply to comment #1)
> 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?

[Quuxplusone]

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

[Quuxplusone]

(In reply to comment #5)
> 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

[Quuxplusone]

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

[Quuxplusone]

I think this is a dup of http://llvm.org/PR16830. 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.

[Quuxplusone]

Attached cpuid.cc (982 bytes, text/x-c++src): cpuid.cc

[Quuxplusone]

Attached cpuid.cpp (443 bytes, text/plain): Further reduced test case

[Quuxplusone]

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

[Quuxplusone]

No. It's a long standing issue.