Re: [Mesa-dev] [Mesa-stable] [PATCH] mapi: avoid text relocation in x86 tsd stubs
On Fri, Nov 09, 2018 at 10:30:42PM +1100, Jonathan Gray wrote:
> On Thu, Nov 08, 2018 at 03:54:20PM +, Emil Velikov wrote:
> > On Fri, 2 Nov 2018 at 00:02, Jonathan Gray 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
> > > > > Cc: mesa-sta...@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 - 1.3
+++ entry_x86_tsd.h 10 Nov 2018 08:12:09 -
@@ -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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mapi: avoid text relocation in x86 tsd stubs
On Thu, Nov 08, 2018 at 03:54:20PM +, Emil Velikov wrote:
> On Fri, 2 Nov 2018 at 00:02, Jonathan Gray 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
> > > > Cc: mesa-sta...@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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mapi: avoid text relocation in x86 tsd stubs
On Fri, 2 Nov 2018 at 00:02, Jonathan Gray 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
> > > Cc: mesa-sta...@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?
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mapi: avoid text relocation in x86 tsd stubs
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
> > Cc: mesa-sta...@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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mapi: avoid text relocation in x86 tsd stubs
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
> Cc: mesa-sta...@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?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
