We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 734

Summary: Missing clobbered register in cpuinfo
Product: SDL Reporter: Nicholas Phillips <eudoxus>
Component: mainAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: josh+sdl, sezeroz
Version: HG 2.0   
Hardware: x86   
OS: Linux   
Attachments: Proposed patch for SDL_cpuinfo.c

Description Nicholas Phillips 2009-04-26 21:34:05 UTC
I am using x64 Linux (using Intel Core 2 DUO), and I have noticed that there is an error in SDL_cpuinfo.c, function CPU_getCPUIDFeaturesExt for my platform.

The code in question is:

#elif defined(__GNUC__) && defined (__x86_64__)
	__asm__ (
"        movq    %%rbx,%%rdi\n"
"        movl    $0x80000000,%%eax   # Query for extended functions    \n"
"        cpuid                       # Get extended function limit     \n"
"        cmpl    $0x80000001,%%eax                                     \n"
"        jl      1f                  # Nope, we dont have function 800000001h\n"
"        movl    $0x80000001,%%eax   # Setup extended function 800000001h\n"
"        cpuid                       # and get the information         \n"
"        movl    %%edx,%0                                              \n"
"1:                                                                    \n"
"        movq    %%rdi,%%rbx\n"
	: "=m" (features)
	:
	: "%rax", "%rcx", "%rdx", "%rdi"
	);

Even though %rbx is saved and restored manually, it still needs to appear on the clobbered register list, because it is modified by the cpuid instruction. If GCC chose to use %rbx to store %0 (features) it would cause big problems. For example consider the following code produced by GCC with the -S switch (code in question used out of context):

_ZN5CTest14MemberFunctionEv:
.LFB1029:
	pushq	%rbx
.LCFI1:
	movl	$10, %edx
	movl	$.LC0, %esi
	movq	%rdi, %rbx
	call	_ZNSs6assignEPKcm
#APP
# 225 "test.cpp" 1
	        movq    %rbx,%rdi
        movl    $0x80000000,%eax   # Query for extended functions    
        cpuid                       # Get extended function limit     
        cmpl    $0x80000001,%eax                                     
        jl      1f                  # Nope, we dont have function 800000001h
        movl    $0x80000001,%eax   # Setup extended function 800000001h
        cpuid                       # and get the information         
        movl    %edx,48(%rbx)                                              
1:                                                                    
        movq    %rdi,%rbx

# 0 "" 2
#NO_APP

This causes a segmentation fault for obvious reasons. 

Adding %rbx to the clobbered register list makes GCC use a different register to store %0, and eliminates the problem.

Cheers.
Comment 1 Nicholas Phillips 2009-04-26 21:46:26 UTC
Created attachment 324 [details]
Proposed patch for SDL_cpuinfo.c

I noticed that the same problem could happen in other functions and for other platforms found in SDL_cpuinfo.c. Here is a simple patch.
Comment 2 Ozkan Sezer 2009-06-25 10:56:27 UTC
Is SDL-1.2 not affected?
Comment 3 Sam Lantinga 2009-09-26 03:05:47 UTC
Thank you very much for your patch!

Do you give me permission to release your code with SDL 1.3 and future
versions of SDL under both the LGPL and a closed-source commercial
license?
Comment 4 Joshua Root 2009-09-26 09:35:19 UTC
Building on i386 OS X seem to be broken now:
./src/cpuinfo/SDL_cpuinfo.c: In function 'SDL_GetCPUFeatures':
./src/cpuinfo/SDL_cpuinfo.c:150: error: PIC register '%ebx' clobbered in 'asm'
Comment 5 Sam Lantinga 2009-09-26 14:40:57 UTC
Whoops, fixed, thanks!
Comment 6 Sam Lantinga 2009-10-29 21:46:02 UTC
Okay, I tracked this down and took care of it, thanks!