[Mesa-dev] [PATCH] mesa: use build flag to ensure stack is realigned on x86

[Timothy Arceri]

On Sat, 2014-12-06 at 22:31 -0800, Matt Turner wrote:
> On Sat, Dec 6, 2014 at 8:52 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> > Nowadays GCC assumes stack pointer is 16-byte aligned even on 32-bits, but that is an assumption OpenGL drivers (or any dynamic library for that matter) can't afford to make as there are many closed- and open- source application binaries out there that only assume 4-byte stack alignment.
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=86788
> > ---
> >  The bug reporter has tested that this fixes the problem. But I haven't
> >  tested the cross compile stuff. e.g. building 64-bit from 32-bit
> >  not sure if anyone would acctaully be building that way.
> >
> >  Please note if this patch is ok it should also be applied to 10.4 with
> >  the last hunk removed.
> >
> >  configure.ac               | 17 +++++++++++++++++
> >  src/mesa/Makefile.am       |  4 ++++
> >  src/mesa/main/sse_minmax.c |  3 ---
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index b0df1bb..6bb43f8 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -474,6 +474,21 @@ fi
> >  dnl
> >  dnl Arch/platform-specific settings
> >  dnl
> > +align_check_arch=""
> > +if test "x$cross_compiling" = xno; then
> > +    case "$host_cpu" in
> > +        x86_64|amd64)
> > +            align_check_arch=x86_64
> > +    ;;
> > +    esac
> > +else
> > +    case "$host_cpu" in
> > +        i?86)
> > +            align_check_arch=x86_64
> 
> This looks wrong.

Yeah I misunderstood how $host_cpu worked.

> 
> > +    ;;
> 
> This is usually aligned with the code in the case.
> 
> > +    esac
> > +fi
> > +
> >  AC_ARG_ENABLE([asm],
> >      [AS_HELP_STRING([--disable-asm],
> >          [disable assembly usage @<:@default=enabled on supported plaforms@:>@])],
> > @@ -2087,6 +2102,8 @@ AM_CONDITIONAL(HAVE_X11_DRIVER, test "x$enable_xlib_glx" = xyes)
> >  AM_CONDITIONAL(HAVE_OSMESA, test "x$enable_osmesa" = xyes)
> >  AM_CONDITIONAL(HAVE_GALLIUM_OSMESA, test "x$enable_gallium_osmesa" = xyes)
> >
> > +AM_CONDITIONAL(HAVE_16BYTE_ALIGN, test "x$align_check_arch" = xx86_64)
> 
> We definitely need a better name. But, more importantly, isn't the
> test wrong? You only want to do this on 32-bit.
> 
> Søren's patch to pixman does this check rather nicely, I think. See
> http://lists.freedesktop.org/archives/pixman/2013-October/003077.html

The test was right because I was doing another test in the makefile.
Anyway I've cleaned things up and sent V2 thanks for pointing me in the
right direction.