[PATCH] Fix cpuid register clobbering for gcc/x86_64

Hi,

When investigating a bug on Fedora[1], I’ve found an issue on the way
CPU_getCPUIDFeatures() handles saving of the %rbx register on x86_64.

The following patch fixes the issue. To avoid adding more #ifdefs to
make the code use %rx instead of %ex, I’ve changed it to use gcc
constraint rules to make it use the right registers depending on the
architecture. The instructions to explictly save %ebx to %edi were also
removed, leaving the responsibility of saving the %rbx register to gcc.

Please CC me on replies, as I am not subscribed to the SDL mailing list.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=487720---
Index: SDL-1.2.13/src/cpuinfo/SDL_cpuinfo.c

— SDL-1.2.13.orig/src/cpuinfo/SDL_cpuinfo.c
+++ SDL-1.2.13/src/cpuinfo/SDL_cpuinfo.c
@@ -147,8 +147,8 @@ static inline int CPU_getCPUIDFeatur
{
int features = 0;
#if defined(GNUC) && ( defined(i386) || defined(x86_64) )

  • int rax, rbx, rcx, rdx;
    asm (
    -" movl %%ebx,%%edi\n"
    " xorl %%eax,%%eax # Set up for CPUID instruction \n"
    " cpuid # Get and save vendor ID \n"
    " cmpl $1,%%eax # Make sure 1 is valid input for CPUID\n"
    @@ -158,10 +158,11 @@ static inline int CPU_getCPUIDFeatur
    " cpuid # Get family/model/stepping/features\n"
    " movl %%edx,%0 \n"
    “1: \n”
    -" movl %%edi,%%ebx\n"
  • : “=m” (features)
  • :
  • : “%eax”, “%ecx”, “%edx”, “%edi”
  • : “=m” (features),
  • "=a" (rax),
    
  • "=b" (rbx),
    
  • "=c" (rcx),
    
  • "=d" (rdx)
    
    );
    #elif (defined(_MSC_VER) && defined(_M_IX86)) || defined(WATCOMC)
    __asm {
    @@ -200,8 +201,8 @@ static inline int CPU_getCPUIDFeatur
    {
    int features = 0;
    #if defined(GNUC) && (defined(i386) || defined (x86_64) )
  • int rax, rbx, rcx, rdx;
    asm (
    -" movl %%ebx,%%edi\n"
    " movl $0x80000000,%%eax # Query for extended functions \n"
    " cpuid # Get extended function limit \n"
    " cmpl $0x80000001,%%eax \n"
    @@ -210,10 +211,11 @@ static inline int CPU_getCPUIDFeatur
    " cpuid # and get the information \n"
    " movl %%edx,%0 \n"
    “1: \n”
    -" movl %%edi,%%ebx\n"
  • : “=m” (features)
  • :
  • : “%eax”, “%ecx”, “%edx”, “%edi”
  • : “=m” (features),
  • "=a" (rax),
    
  • "=b" (rbx),
    
  • "=c" (rcx),
    
  • "=d" (rdx)
    
    );
    #elif (defined(_MSC_VER) && defined(_M_IX86)) || defined(WATCOMC)
    __asm {


Eduardo

Hi,

When investigating a bug on Fedora[1], I’ve found an issue on the way
CPU_getCPUIDFeatures() handles saving of the %rbx register on x86_64.

The following patch fixes the issue. To avoid adding more #ifdefs to
make the code use %rx instead of %ex, I’ve changed it to use gcc
constraint rules to make it use the right registers depending on the
architecture. The instructions to explictly save %ebx to %edi were also
removed, leaving the responsibility of saving the %rbx register to gcc.

Please CC me on replies, as I am not subscribed to the SDL mailing list.

Hey Sam, did you use GCC to build the DLL you posted? This might
be part of the issue with the SDL_FillRect bug I reported. I’m running
an x86_64 CPU on a 64-bit OS.>----- Original Message ----

From: Eduardo Habkost
Subject: [SDL] [PATCH] Fix cpuid register clobbering for gcc/x86_64

Hi,

When investigating a bug on Fedora[1], I’ve found an issue on the way
CPU_getCPUIDFeatures() handles saving of the %rbx register on x86_64.

I think this is already fixed in SDL 1.3. Can you double check that the
fix works and see whether it’s better than the one that you proposed?
http://www.libsdl.org/tmp/SDL-1.3.tar.gz

See ya,
-Sam Lantinga, Founder and President, Galaxy Gameworks LLC

Hey Sam, did you use GCC to build the DLL you posted? This might
be part of the issue with the SDL_FillRect bug I reported. I’m running
an x86_64 CPU on a 64-bit OS.

I don’t remember… probably. :slight_smile:

What OS are you running?

See ya,
-Sam Lantinga, Founder and President, Galaxy Gameworks LLC

The fix on trunk works too. Not duplicating code and letting gcc take
care of saving/restoring %rbx only where needed would be nice, but it’s
not critical.On Tue, Mar 03, 2009 at 07:57:00AM -0800, Sam Lantinga wrote:

Hi,

When investigating a bug on Fedora[1], I’ve found an issue on the way
CPU_getCPUIDFeatures() handles saving of the %rbx register on x86_64.

I think this is already fixed in SDL 1.3. Can you double check that the
fix works and see whether it’s better than the one that you proposed?
http://www.libsdl.org/tmp/SDL-1.3.tar.gz


Eduardo

Hey Sam, did you use GCC to build the DLL you posted? This might
be part of the issue with the SDL_FillRect bug I reported. I’m running
an x86_64 CPU on a 64-bit OS.

I don’t remember… probably. :slight_smile:

What OS are you running?

VIsta Home Premium, SP1, 64-bit version.>----- Original Message ----

From: Sam Lantinga
Subject: Re: [SDL] [PATCH] Fix cpuid register clobbering for gcc/x86_64