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

[Eric Engestrom]

On Wednesday, 2019-11-27 10:20:04 -0800, Lepton Wu wrote:
> OK, I didn't know there is a patch in mail list.  BTW, it seems there is no
> meaningful difference
> with the generated code:  when it's trying to align to 32 bytes border,
> since the actual code size
> is between 32 and 64, actually the entry size is 64.   So in this case,
> balign to 32 or 64 has same result
> 
> I heard that mesa will change to use libglvnd, if that will happen soon,
> then we don't worry about code here
> any more.

Mesa has had support for glvnd for a few years already, but I don't
think there's any plan to drop support for building our own libs, so
this code is not going anywhere.

> 
> On Wed, Nov 27, 2019 at 5:57 AM Jonathan Gray <jsg at jsg.id.au> wrote:
> 
> > On Sun, Nov 11, 2018 at 03:47:35PM +1100, Jonathan Gray wrote:
> > > Make similiar changes to libglvnd to avoid a text relocation in
> > > x86 tsd stubs fixing the build with lld.
> > >
> > > v2:
> > >   - store the address of the GOT in ebx required before calling PLT stub
> > >   - change .balign values to match X86_ENTRY_SIZE
> >
> > When a different version of this patch was committed/pushed in
> > 45206d7673adb1484cbdb3eadaf82e0849c9cdcf
> > (with author rewritten to imply I wrote the commit message)
> > it did not include the .balign changes to match the entry size change.
> >
> > Was this purposefully skipped or overlooked as the bugzilla patch was
> > used?
> >
> > The diff to what was committed and this patch is
> >
> > diff --git a/src/mapi/entry_x86_tsd.h b/src/mapi/entry_x86_tsd.h
> > index bd8db7b19f9..1dec3ed86c4 100644
> > --- a/src/mapi/entry_x86_tsd.h
> > +++ b/src/mapi/entry_x86_tsd.h
> > @@ -34,18 +34,18 @@
> >  #define X86_ENTRY_SIZE 64
> >
> >  __asm__(".text\n"
> > -        ".balign 32\n"
> > +        ".balign " U_STRINGIFY(X86_ENTRY_SIZE) "\n"
> >          "x86_entry_start:");
> >
> >  #define STUB_ASM_ENTRY(func)        \
> >     ".globl " func "\n"              \
> >     ".type " func ", @function\n"    \
> > -   ".balign 32\n"                   \
> > +   ".balign " U_STRINGIFY(X86_ENTRY_SIZE) "\n" \
> >     func ":"
> >
> >  #define STUB_ASM_CODE(slot)         \
> >     "push %ebx\n\t"                  \
> > -   "call 1f\n\t"                    \
> > +   "call 1f\n"                      \
> >     "1:\n\t"                         \
> >     "popl %ebx\n\t"                  \
> >     "addl $_GLOBAL_OFFSET_TABLE_+[.-1b], %ebx\n\t" \
> > @@ -53,7 +53,7 @@ __asm__(".text\n"
> >     "mov (%eax), %eax\n\t"           \
> >     "testl %eax, %eax\n\t"           \
> >     "jne 1f\n\t"                     \
> > -   "call " ENTRY_CURRENT_TABLE_GET "@PLT\n\t" \
> > +   "call " ENTRY_CURRENT_TABLE_GET "@PLT\n" \
> >     "1:\n\t"                         \
> >     "pop %ebx\n\t"                   \
> >     "jmp *(4 * " slot ")(%eax)"
> > @@ -63,7 +63,7 @@ __asm__(".text\n"
> >
> >  #ifndef MAPI_MODE_BRIDGE
> >
> > -__asm__(".balign 32\n"
> > +__asm__(".balign " U_STRINGIFY(X86_ENTRY_SIZE) "\n"
> >          "x86_entry_end:");
> >
> >  #include <string.h>
> >
> > >
> > > 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 | 22 ++++++++++++++--------
> > >  1 file changed, 14 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/src/mapi/entry_x86_tsd.h b/src/mapi/entry_x86_tsd.h
> > > index 0c28c8ff068..1dec3ed86c4 100644
> > > --- a/src/mapi/entry_x86_tsd.h
> > > +++ b/src/mapi/entry_x86_tsd.h
> > > @@ -31,25 +31,31 @@
> > >  #define HIDDEN
> > >  #endif
> > >
> > > -#define X86_ENTRY_SIZE 32
> > > +#define X86_ENTRY_SIZE 64
> > >
> > >  __asm__(".text\n"
> > > -        ".balign 32\n"
> > > +        ".balign " U_STRINGIFY(X86_ENTRY_SIZE) "\n"
> > >          "x86_entry_start:");
> > >
> > >  #define STUB_ASM_ENTRY(func)        \
> > >     ".globl " func "\n"              \
> > >     ".type " func ", @function\n"    \
> > > -   ".balign 32\n"                   \
> > > +   ".balign " U_STRINGIFY(X86_ENTRY_SIZE) "\n" \
> > >     func ":"
> > >
> > >  #define STUB_ASM_CODE(slot)         \
> > > -   "movl " ENTRY_CURRENT_TABLE ", %eax\n\t" \
> > > +   "push %ebx\n\t"                  \
> > > +   "call 1f\n"                      \
> > > +   "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" \
> > >     "1:\n\t"                         \
> > > -   "call " ENTRY_CURRENT_TABLE_GET "\n\t" \
> > > +   "pop %ebx\n\t"                   \
> > >     "jmp *(4 * " slot ")(%eax)"
> > >
> > >  #define MAPI_TMP_STUB_ASM_GCC
> > > @@ -57,7 +63,7 @@ __asm__(".text\n"
> > >
> > >  #ifndef MAPI_MODE_BRIDGE
> > >
> > > -__asm__(".balign 32\n"
> > > +__asm__(".balign " U_STRINGIFY(X86_ENTRY_SIZE) "\n"
> > >          "x86_entry_end:");
> > >
> > >  #include <string.h>
> > > --
> > > 2.19.1
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >