[Mesa-dev] [Mesa-stable] [PATCH] mapi: avoid text relocation in x86 tsd stubs

[Jonathan Gray]

On Fri, Nov 09, 2018 at 10:30:42PM +1100, Jonathan Gray wrote:
> On Thu, Nov 08, 2018 at 03:54:20PM +0000, Emil Velikov wrote:
> > On Fri, 2 Nov 2018 at 00:02, Jonathan Gray <jsg at jsg.id.au> wrote:
> > >
> > > On Thu, Nov 01, 2018 at 12:26:34PM -0700, Ian Romanick wrote:
> > > > On 10/31/2018 09:08 PM, Jonathan Gray wrote:
> > > > > Make similiar changes to libglvnd to avoid a text relocation in
> > > > > x86 tsd stubs fixing the build with lld.
> > > > >
> > > > > Signed-off-by: Jonathan Gray <jsg at jsg.id.au>
> > > > > Cc: mesa-stable at lists.freedesktop.org
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108541
> > > > > ---
> > > > >  src/mapi/entry_x86_tsd.h | 14 +++++++++-----
> > > > >  1 file changed, 9 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/src/mapi/entry_x86_tsd.h b/src/mapi/entry_x86_tsd.h
> > > > > index 0c28c8ff068..e08a02f3db2 100644
> > > > > --- a/src/mapi/entry_x86_tsd.h
> > > > > +++ b/src/mapi/entry_x86_tsd.h
> > > > > @@ -31,7 +31,7 @@
> > > > >  #define HIDDEN
> > > > >  #endif
> > > > >
> > > > > -#define X86_ENTRY_SIZE 32
> > > > > +#define X86_ENTRY_SIZE 64
> > > > >
> > > > >  __asm__(".text\n"
> > > > >          ".balign 32\n"
> > > > > @@ -44,12 +44,16 @@ __asm__(".text\n"
> > > > >     func ":"
> > > > >
> > > > >  #define STUB_ASM_CODE(slot)         \
> > > > > -   "movl " ENTRY_CURRENT_TABLE ", %eax\n\t" \
> > > > > +   "call 1f\n\t"                    \
> > > > > +   "1:\n\t"                         \
> > > > > +   "popl %eax\n\t"                  \
> > > > > +   "addl $_GLOBAL_OFFSET_TABLE_+[.-1b], %eax\n\t" \
> > > > > +   "movl " ENTRY_CURRENT_TABLE "@GOT(%eax), %eax\n\t" \
> > > > > +   "mov (%eax), %eax\n\t"           \
> > > > >     "testl %eax, %eax\n\t"           \
> > > > > -   "je 1f\n\t"                      \
> > > > > -   "jmp *(4 * " slot ")(%eax)\n"    \
> > > > > +   "jne 1f\n\t"                     \
> > > > > +   "call " ENTRY_CURRENT_TABLE_GET "@PLT\n\t" \
> > > > >     "1:\n\t"                         \
> > > > > -   "call " ENTRY_CURRENT_TABLE_GET "\n\t" \
> > > > >     "jmp *(4 * " slot ")(%eax)"
> > > >
> > > > After this change, the code is:
> > > >
> > > > #define STUB_ASM_CODE(slot)         \
> > > >    "call 1f\n\t"                    \
> > > >    "1:\n\t"                         \
> > > >    "popl %eax\n\t"                  \
> > > >    "addl $_GLOBAL_OFFSET_TABLE_+[.-1b], %eax\n\t" \
> > > >    "movl " ENTRY_CURRENT_TABLE "@GOT(%eax), %eax\n\t" \
> > > >    "mov (%eax), %eax\n\t"           \
> > > >    "testl %eax, %eax\n\t"           \
> > > >    "jne 1f\n\t"                     \
> > > >    "call " ENTRY_CURRENT_TABLE_GET "@PLT\n\t" \
> > > >    "1:\n\t"                         \
> > > >    "jmp *(4 * " slot ")(%eax)"
> > > >
> > > > So there's going to be two labels "1:".  Does that even assemble?
> > >
> > > Yes, the call/jmp is always forward as it is '1f'.
> > > This also runs glxinfo, glxgears etc on a pentium m running OpenBSD/i386.
> > >
> > > https://github.com/NVIDIA/libglvnd/blob/master/src/GLdispatch/vnd-glapi/entry_x86_tsd.c#L58
> > >
> > > libglvnd has two labels like this as well, the ebx use there isn't needed.
> > 
> > Hi all, did this get stuck or it's superseded/obsolete?
> 
> This is still the latest version of the patch.

The original patch should be ignored as while it worked for dynamically
linked libGL users like glxinfo and glxgears a program using SDL2 which
dlopens libGL segfaulted.  The ebx portion is required.

Index: entry_x86_tsd.h
===================================================================
RCS file: /cvs/xenocara/lib/mesa/src/mapi/entry_x86_tsd.h,v
retrieving revision 1.3
diff -u -p -r1.3 entry_x86_tsd.h
--- entry_x86_tsd.h	10 Nov 2018 08:11:16 -0000	1.3
+++ entry_x86_tsd.h	10 Nov 2018 08:12:09 -0000
@@ -31,7 +31,7 @@
 #define HIDDEN
 #endif
 
-#define X86_ENTRY_SIZE 32
+#define X86_ENTRY_SIZE 64
 
 __asm__(".text\n"
         ".balign 32\n"
@@ -44,12 +44,18 @@ __asm__(".text\n"
    func ":"
 
 #define STUB_ASM_CODE(slot)         \
-   "movl " ENTRY_CURRENT_TABLE ", %eax\n\t" \
+   "push %ebx\n\t"                  \
+   "call 1f\n\t"                    \
+   "1:\n\t"                         \
+   "popl %ebx\n\t"                  \
+   "addl $_GLOBAL_OFFSET_TABLE_+[.-1b], %ebx\n\t" \
+   "movl " ENTRY_CURRENT_TABLE "@GOT(%ebx), %eax\n\t" \
+   "mov (%eax), %eax\n\t"           \
    "testl %eax, %eax\n\t"           \
-   "je 1f\n\t"                      \
-   "jmp *(4 * " slot ")(%eax)\n"    \
+   "jne 1f\n\t"                     \
+   "call " ENTRY_CURRENT_TABLE_GET "@PLT\n\t" \
    "1:\n\t"                         \
-   "call " ENTRY_CURRENT_TABLE_GET "\n\t" \
+   "pop %ebx\n\t"                   \
    "jmp *(4 * " slot ")(%eax)"
 
 #define MAPI_TMP_STUB_ASM_GCC