Bug 33440 - _gl_DispatchTSD in glapi_x86_64.S has incorrect relocation type
Summary: _gl_DispatchTSD in glapi_x86_64.S has incorrect relocation type
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: x86-64 (AMD64) All
: medium normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-24 15:18 UTC by Dimitry Andric
Modified: 2011-01-27 17:05 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Change relocation type of _gl_DispatchTSD to GOTPCREL (927 bytes, application/octet-stream)
2011-01-24 15:18 UTC, Dimitry Andric
Details
Change relocation type of _gl_DispatchTSD to GOTPCREL, take 2 (979 bytes, patch)
2011-01-27 13:07 UTC, Dimitry Andric
Details | Splinter Review

Description Dimitry Andric 2011-01-24 15:18:16 UTC
Created attachment 42416 [details]
Change relocation type of _gl_DispatchTSD to GOTPCREL

While building Mesa on FreeBSD, with recent binutils (>2.17), libGL.so.1
will fail to link with a misleading error:

...
mklib: Making FreeBSD shared library:  libGL.so.1
/usr/bin/ld: ../../../src/mesa/x86-64/glapi_x86-64.o: relocation R_X86_64_PC32 against `_gl_DispatchTSD' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value

But of course, the glapi_x86-64.o file is already compiled with -fPIC,
the actual command line used is:

cc -c -I. -I../../../include -I../../../include/GL/internal -I../../../src/mesa -I../../../src/mesa/glapi -I/usr/local/include -I/usr/local/include/drm -I/usr/local/include -D_THREAD_SAFE -I/usr/local/include -I/usr/local/include -O2 -pipe -fno-strict-aliasing -Wall -Wmissing-prototypes -std=c99 -ffast-math -fno-strict-aliasing -fPIC -DUSE_X86_64_ASM -DHAVE_POSIX_MEMALIGN -DUSE_XCB -DPTHREADS -DUSE_EXTERNAL_DXTN_LIB=1 -DIN_DRI_DRIVER -DHAVE_ALIAS -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DXF86VIDMODE -D_REENTRANT -UIN_DRI_DRIVER -DDEFAULT_DRIVER_DIR=\"/usr/local/lib/dri\" ../../../src/mesa/x86-64/glapi_x86-64.S -o ../../../src/mesa/x86-64/glapi_x86-64.o

The problem in glapi_x86-64.S is that if PTHREADS is defined,
_gl_DispatchTSD is declared as .extern, but is then accessed directly
with _gl_DispatchTSD(%rip), which is only applicable to locally defined
objects:

#elif defined(PTHREADS)

        .extern _glapi_Dispatch
        .extern _gl_DispatchTSD			# If this is extern...
        .extern pthread_getspecific

        .p2align        4,,15
_x86_64_get_dispatch:
        movq    _gl_DispatchTSD(%rip), %rdi	# ...this is incorrect
        jmp     pthread_getspecific@PLT

Instead, this access to _gl_DispatchTSD needs to be qualified with
@GOTPCREL, to indicate the symbol is relative to the Global Offset
Table, e.g.:

        movq    _gl_DispatchTSD@GOTPCREL(%rip), %rdi

(This was also found out by José Fonseca in one of his side branches,
commit b09d1ef60dae3b3c44f1370fd2f726c7044bc17)

As an example, consider the following small piece of C code:

extern int foo;

int bar(void)
{
        return foo + 42;
}

If you compile this on x86-64 with "gcc -O2 -fPIC -S", so for use in a
shared library, it will result in (approximately);

.globl bar
        .type   bar, @function
bar:
.LFB2:
        movq    foo@GOTPCREL(%rip), %rax
        movl    (%rax), %eax
        addl    $42, %eax
        ret

Again, since the glapi_x86-64.S file is generated from gl_x86-64_asm.py,
I changed both.
Comment 1 Brian Paul 2011-01-25 08:25:35 UTC
Thanks.  Committed as 731ec60da3ccb92f5bfb4d6f1bc3c8e712751376
Comment 2 Alex Deucher 2011-01-25 11:03:39 UTC
This patch causes a segfault in all 3D apps on Linux (64-bit on AMD Phenom 9350e).
Comment 3 Brian Paul 2011-01-25 11:14:05 UTC
I've reverted the patch.  Dimitry, can you look into this again?
Comment 4 Dimitry Andric 2011-01-27 13:07:18 UTC
Created attachment 42610 [details] [review]
Change relocation type of _gl_DispatchTSD to GOTPCREL, take 2

Okay, I think I have now determined the proper way to solve this issue.
I should have examined my own gcc example more closely... :)

When Mesa is using pthreads, _gl_DispatchTSD is actually an instance of
the following struct:

struct u_tsd {
   pthread_key_t key;
   int initMagic;
};

The purpose of _x86_64_get_dispatch is to get the pthread-specific data
associated with the key (normally a struct mapi_table pointer).  In C
this would be:

void *_x86_64_get_dispatch(void)
{
   return pthread_getspecific(_gl_DispatchTSD.key);
}

which, using -fPIC, gets compiled by gcc to:

_x86_64_get_dispatch:
	movq	_gl_DispatchTSD@GOTPCREL(%rip), %rax
	movl	(%rax), %edi
	jmp	pthread_getspecific@PLT

E.g. the _gl_DispatchTSD@GOTPCREL(%rip) reference gives a pointer to the
start of the struct, which gets stored in %rax.  Then, the pointer is
dereferenced to get the first member of the struct: an unsigned int on
Linux, even on x86-64 (on FreeBSD, it's a regular int).  It is stored in
%edi, to serve as first parameter to pthread_getspecific(), which is
tail-called.

So please try this revised patch, which I have actually tested to run
properly on Linux.
Comment 5 Alex Deucher 2011-01-27 13:56:18 UTC
Seem to work fine here on 64-bit Linux.
Comment 6 Brian Paul 2011-01-27 17:05:19 UTC
Works for me too.  Committed (cfb9aae3ec4e42bd9be8445039dc52b8d6c71f9c).


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.