Opened 7 years ago

Closed 7 years ago

Last modified 3 months ago

#2675 closed defect (fixed)

[PATCH] x86_x64.cpp fails to build with -fpie on i386

Reported by: pstumpf Owned by: historic_bruno
Priority: Should Have Milestone: Alpha 18
Component: Core engine Keywords: patch
Cc: Patch:

Description (last modified by historic_bruno)

The cpuid assembler stuff does not compile when using PIE on i386 due to register constraints. One must save and restore the PIC register (%ebx) before and after the call. See also cpuid.h as provided by GCC or Clang.

In all other cases (amd64 and !PIE i386), use the regular code.

Compile and runtime tested on OpenBSD/i386, which uses -fpie by default.

  • x86_x64.cpp

     
    4949#if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 150030729
    5050// VC10+ and VC9 SP1: __cpuidex is already available
    5151#elif GCC_VERSION
     52# if defined(__i386__) && defined(__PIC__)
    5253# define __cpuidex(regsArray, level, index)\
     54       __asm__ __volatile__ ("pushl    %%ebx\n" \
     55       " cpuid\n"\
     56       "mov %%ebx,%1\n" \
     57       "popl %%ebx" \
     58       : "=a" ((regsArray)[0]), "=r" ((regsArray)[1]), "=c" ((regsArray)[2]), "=d" ((regsArray)[3])\
     59       : "0" (level), "2" (index));
     60# else
     61# define __cpuidex(regsArray, level, index)\
    5362       __asm__ __volatile__ ("cpuid"\
    5463       : "=a" ((regsArray)[0]), "=b" ((regsArray)[1]), "=c" ((regsArray)[2]), "=d" ((regsArray)[3])\
    5564       : "0" (level), "2" (index));
     65# endif
    5666#else
    5767# error "compiler not supported"
    5868#endif

Attachments (1)

diff (889 bytes ) - added by pstumpf 7 years ago.

Download all attachments as: .zip

Change History (9)

by pstumpf, 7 years ago

Attachment: diff added

comment:1 by stanislas69, 7 years ago

Keywords: unanswered added

comment:2 by historic_bruno, 7 years ago

Description: modified (diff)
Keywords: patch review added; unanswered removed
Milestone: BacklogAlpha 18
Summary: x86_x64.cpp fails to build with -fpie on i386[PATCH] x86_x64.cpp fails to build with -fpie on i386

Hi, please follow the instructions when submitting patches: SubmittingPatches

comment:3 by historic_bruno, 7 years ago

Owner: set to historic_bruno
Status: newassigned

comment:4 by ben, 7 years ago

Resolution: fixed
Status: assignedclosed

In 15886:

Fixes cpuid to work with i386 CPUs and PIE/PIC, based on old GCC cpuid.h source and patch by pstumpf, fixes #2675

comment:5 by historic_bruno, 7 years ago

Keywords: review removed

Thanks! I used your patch and this ticket as inspiration, and combined with an old cpuid.h from GCC (one that is GPLv2, the newer ones are GPLv3). It also fixes the problem I had with 32-bit builds on OS X 10.6, where PIC is used by default.

comment:6 by ben, 7 years ago

In 15890:

Reverts r15886 due to licensing issues (source/lib is MIT licensed) and replaces with compatible FreeBSD/clang-based solution, patch by pstumpf. Refs #2675

comment:7 by historic_bruno, 7 years ago

Does this work with cross compiling? I'm getting errors when targeting i386 (running on iPhone simulator on OS X) though the system and CPU are x86-64. The previous code worked fine.

x86_x64.cpp(203): Function call failed: return value was -130002 (Unknown error (-130002, 0xFFFFFFFFFFFE042E))
Function call failed: return value was -130002 (Unknown error (-130002, 0xFFFFFFFFFFFE042E))
Location: x86_x64.cpp:203 (InitVendor)

vendorString contains some garbage, like "Lg{ineIntel".

comment:8 by wraitii, 3 months ago

In 25405:

Fix cpuid call with -fPIC on i386 architectures.

GCC < 5 used to reserve the ebx register for PIC (position-independent code) metadata. This meant that we needed to save the state of ebx before calling cpuid (fixed in #2675)
However, the original patch from r15886 did not force a particular register to store this value in. Following the GCC 5 upgrade, GCC stopped reserving ebx, and that register silently got used instead. The code became non-sensical, and our ASM cpuidex started returning random garbage in edx.

Since we now only support GCC7 and above, the PIC-specific branch is no longer necessary and is removed.

Fixes #6028. The assertion was a result of random data in ebx.
Refs #2675 / reverts rP15890 and reverts rP15886.

Patch by: nwtour

Comments by: vladislavbelov

Differential Revision: https://code.wildfiregames.com/D3575

Note: See TracTickets for help on using tickets.