[Mesa-dev] [PATCH 2/2] configure: Leverage gcc warn options to enable safe use of C99 features where possible.

Jose Fonseca jfonseca at vmware.com
Fri Feb 27 13:58:51 PST 2015


Sorry, this didn't pop up when I built it here.  And I'm afraid I won't have time to look into this today, and possibly the weekend.

If there's not an obvious fix feel free to push a commit commiting out the  .MSVC2013_COMPAT_*FLAGS= in configure.ac

I'll act on other reviews too as soon as I can.

Jose



________________________________________
From: Tom Stellard <tom at stellard.net>
Sent: 27 February 2015 16:21
To: Jose Fonseca
Cc: mesa-dev at lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 2/2] configure: Leverage gcc warn options to enable safe use of C99 features where possible.

Hi,

This patch breaks the build for me:

CFLAGS="-g" CXXFLAGS="$CFLAGS" CC="ccache gcc" CXX="ccache g++"
./autogen.sh \
--prefix=/usr/local \
--with-dri-drivers="no" \
--with-gallium-drivers="r600,radeonsi" \
--enable-glx-tls \
--enable-debug \
--enable-shared-glapi \
--with-egl-platforms=x11,drm \
--enable-gallium-egl \
--enable-gallium-gbm \
--with-llvm-prefix=/usr/local/llvm/3.7 \
--enable-gallium-drm-loader \
--enable-gallium-compute-api \
--enable-opencl-icd \
--enable-opengl \
--disable-dri3 \
--enable-texture-float \
--with-llvm-shared-libs \
--enable-opencl \
--enable-gbm

  CC       glapi_libglapi_la-entry.lo
In file included from entry.c:49:0:
entry_x86-64_tls.h: In function 'entry_generate':
entry_x86-64_tls.h:105:29: error: pointer of type 'void *' used in arithmetic [-Werror=pointer-arith]
    *((unsigned int *) (code + 5)) = addr;
                             ^
cc1: some warnings being treated as errors
Makefile:1425: recipe for target 'shared_glapi_libglapi_la-entry.lo'
failed


On Fri, Feb 27, 2015 at 07:59:47AM -0800, Ian Romanick wrote:
> I like the idea as it should prevent future thrash.  There are a couple
> comments below.
>
> On 02/26/2015 08:51 AM, Jose Fonseca wrote:
> > The main objective of this change is to enable Linux developers to use
> > more of C99 throughout Mesa, with confidence that the portions that need
> > to be built with MSVC -- and only those portions --, stay portable.
> >
> > This is achieved by using the appropriate -Werror= options only on the
> > places they need to be used.
> >
> > Unfortunately we still need MSVC 2008 on a few portions of the code
> > (namely llvmpipe and its dependencies).  I hope to eventually eliminate
> > this so that we can use C99 everywhere, but there are technical/logistic
> > challenges (specifically, newer Windows SDKs no longer bundle MSVC,
> > instead require a full installation of Visual Studio, and that has
> > hindered adoption of newer MSVC versions on our build processes.)
> > Thankfully we have more directy control over our OpenGL driver, which is
> > why we're now able to migrate to MSVC 2013 for most of the tree.
> > ---
> >  configure.ac                               | 17 +++++++++++++++++
> >  src/egl/main/Makefile.am                   |  1 +
> >  src/gallium/auxiliary/Makefile.am          |  7 +++++--
> >  src/gallium/drivers/llvmpipe/Makefile.am   |  6 ++++--
> >  src/gallium/state_trackers/egl/Makefile.am |  3 ++-
> >  src/gallium/targets/egl-static/Makefile.am |  3 ++-
> >  src/glsl/Makefile.am                       |  8 ++++++--
> >  src/loader/Makefile.am                     |  1 +
> >  src/mapi/Makefile.am                       |  4 +++-
> >  src/mesa/Makefile.am                       | 10 ++++++++--
> >  src/util/Makefile.am                       |  3 ++-
> >  11 files changed, 51 insertions(+), 12 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 5fbb7bc..22dc023 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -263,6 +263,18 @@ if test "x$GCC" = xyes; then
> >      # gcc's builtin memcmp is slower than glibc's
> >      # https://urldefense.proofpoint.com/v2/url?u=http-3A__gcc.gnu.org_bugzilla_show-5Fbug.cgi-3Fid-3D43052&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=tyrvqAm0EVC5AwhM7HUFBP2BLPSrfRoV_sdwABGjAPs&s=jkc8mkso0tyQ_eIVNoxTYLSLWc9UWUzC02Lg6STYlAU&e=
> >      CFLAGS="$CFLAGS -fno-builtin-memcmp"
> > +
> > +    # Flags to help ensure that certain portions of the code -- and only those
> > +    # portions -- can be built with MSVC:
> > +    # - src/util, src/gallium/auxiliary, and src/gallium/drivers/llvmpipe needs
> > +    #   to build with Windows SDK 7.0.7600, which bundles MSVC 2008
> > +    # - non-Linux/Posix OpenGL portions needs to build on MSVC 2013 (which
> > +    #   supports most of C99)
> > +    # - the rest has no compiler compiler restrictions
> > +    MSVC2013_COMPAT_CFLAGS="-Werror=vla -Werror=pointer-arith"
> > +    MSVC2013_COMPAT_CXXFLAGS="-Werror=vla -Werror=pointer-arith"
> > +    MSVC2008_COMPAT_CFLAGS="$MSVC2013_COMPAT_CFLAGS -Werror=declaration-after-statement"
> > +    MSVC2008_COMPAT_CXXFLAGS="$MSVC2013_COMPAT_CXXFLAGS"
> >  fi
> >  if test "x$GXX" = xyes; then
> >      CXXFLAGS="$CXXFLAGS -Wall"
> > @@ -288,6 +300,11 @@ if test "x$GXX" = xyes; then
> >      CXXFLAGS="$CXXFLAGS -fno-builtin-memcmp"
> >  fi
> >
> > +AC_SUBST([MSVC2013_COMPAT_CFLAGS])
> > +AC_SUBST([MSVC2013_COMPAT_CXXFLAGS])
> > +AC_SUBST([MSVC2008_COMPAT_CFLAGS])
> > +AC_SUBST([MSVC2008_COMPAT_CXXFLAGS])
> > +
> >  dnl even if the compiler appears to support it, using visibility attributes isn't
> >  dnl going to do anything useful currently on cygwin apart from emit lots of warnings
> >  case "$host_os" in
> > diff --git a/src/egl/main/Makefile.am b/src/egl/main/Makefile.am
> > index d21d8a9..a4db210 100644
> > --- a/src/egl/main/Makefile.am
> > +++ b/src/egl/main/Makefile.am
> > @@ -26,6 +26,7 @@ AM_CFLAGS = \
> >     -I$(top_srcdir)/src/gbm/main \
> >     $(DEFINES) \
> >     $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS) \
> >     $(EGL_CFLAGS) \
> >     -D_EGL_NATIVE_PLATFORM=$(EGL_NATIVE_PLATFORM) \
> >     -D_EGL_DRIVER_SEARCH_DIR=\"$(libdir)/egl\" \
> > diff --git a/src/gallium/auxiliary/Makefile.am b/src/gallium/auxiliary/Makefile.am
> > index 4b62057..27a8b3f 100644
> > --- a/src/gallium/auxiliary/Makefile.am
> > +++ b/src/gallium/auxiliary/Makefile.am
> > @@ -12,9 +12,12 @@ noinst_LTLIBRARIES = libgallium.la
> >  AM_CFLAGS = \
> >     -I$(top_srcdir)/src/gallium/auxiliary/util \
> >     $(GALLIUM_CFLAGS) \
> > -   $(VISIBILITY_CFLAGS)
> > +   $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2008_COMPAT_CXXFLAGS)
> >
> > -AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
> > +AM_CXXFLAGS = \
> > +   $(VISIBILITY_CXXFLAGS) \
> > +   $(MSVC2008_COMPAT_CXXFLAGS)
> >
> >  libgallium_la_SOURCES = \
> >     $(C_SOURCES) \
> > diff --git a/src/gallium/drivers/llvmpipe/Makefile.am b/src/gallium/drivers/llvmpipe/Makefile.am
> > index 0bd4282..1d3853e 100644
> > --- a/src/gallium/drivers/llvmpipe/Makefile.am
> > +++ b/src/gallium/drivers/llvmpipe/Makefile.am
> > @@ -25,10 +25,12 @@ include $(top_srcdir)/src/gallium/Automake.inc
> >
> >  AM_CFLAGS = \
> >     $(GALLIUM_DRIVER_CFLAGS) \
> > -   $(LLVM_CFLAGS)
> > +   $(LLVM_CFLAGS) \
> > +   $(MSVC2008_COMPAT_CFLAGS)
> >  AM_CXXFLAGS= \
> >     $(GALLIUM_DRIVER_CXXFLAGS) \
> > -   $(LLVM_CXXFLAGS)
> > +   $(LLVM_CXXFLAGS) \
> > +   $(MSVC2008_COMPAT_CXXFLAGS)
> >
> >  noinst_LTLIBRARIES = libllvmpipe.la
> >
> > diff --git a/src/gallium/state_trackers/egl/Makefile.am b/src/gallium/state_trackers/egl/Makefile.am
> > index 31546a7..f13fcb2 100644
> > --- a/src/gallium/state_trackers/egl/Makefile.am
> > +++ b/src/gallium/state_trackers/egl/Makefile.am
> > @@ -27,7 +27,8 @@ include $(top_srcdir)/src/gallium/Automake.inc
> >
> >  AM_CFLAGS = \
> >     $(GALLIUM_CFLAGS) \
> > -   $(VISIBILITY_CFLAGS)
> > +   $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS)
> >
> >  AM_CPPFLAGS = \
> >     -I$(top_srcdir)/src/egl/main \
> > diff --git a/src/gallium/targets/egl-static/Makefile.am b/src/gallium/targets/egl-static/Makefile.am
> > index 85f2ac1..51c3f05 100644
> > --- a/src/gallium/targets/egl-static/Makefile.am
> > +++ b/src/gallium/targets/egl-static/Makefile.am
> > @@ -31,7 +31,8 @@
> >  include $(top_srcdir)/src/gallium/Automake.inc
> >
> >  AM_CFLAGS = \
> > -   $(GALLIUM_TARGET_CFLAGS)
> > +   $(GALLIUM_TARGET_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS)
> >
> >  AM_CPPFLAGS = \
> >     -I$(top_srcdir)/src/gallium/state_trackers/egl \
> > diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
> > index 5a0a643..b466a3b 100644
> > --- a/src/glsl/Makefile.am
> > +++ b/src/glsl/Makefile.am
> > @@ -33,8 +33,12 @@ AM_CPPFLAGS = \
> >     -I$(top_srcdir)/src/gtest/include \
> >     -I$(top_builddir)/src/glsl/nir \
> >     $(DEFINES)
> > -AM_CFLAGS = $(VISIBILITY_CFLAGS)
> > -AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
> > +AM_CFLAGS = \
> > +   $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS)
> > +AM_CXXFLAGS = \
> > +   $(VISIBILITY_CXXFLAGS) \
> > +   $(MSVC2013_COMPAT_CXXFLAGS)
> >
> >  EXTRA_DIST = tests glcpp/tests README TODO glcpp/README    \
> >     glsl_lexer.ll                                   \
> > diff --git a/src/loader/Makefile.am b/src/loader/Makefile.am
> > index 36ddba8..3d32279 100644
> > --- a/src/loader/Makefile.am
> > +++ b/src/loader/Makefile.am
> > @@ -30,6 +30,7 @@ libloader_la_CPPFLAGS = \
> >     -I$(top_srcdir)/include \
> >     -I$(top_srcdir)/src \
> >     $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS) \
> >     $(LIBUDEV_CFLAGS)
>
> I don't think this is necessary for src/loader.  Isn't that only built
> for DRI?
>
> >  libloader_la_SOURCES = $(LOADER_C_FILES)
> > diff --git a/src/mapi/Makefile.am b/src/mapi/Makefile.am
> > index 6794682..b0a6c8c 100644
> > --- a/src/mapi/Makefile.am
> > +++ b/src/mapi/Makefile.am
> > @@ -39,7 +39,9 @@ EXTRA_DIST = \
> >     glapi/SConscript \
> >     shared-glapi/SConscript
> >
> > -AM_CFLAGS = $(PTHREAD_CFLAGS)
> > +AM_CFLAGS = \
> > +   $(PTHREAD_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS)
> >  AM_CPPFLAGS =                                                      \
> >     $(DEFINES)                                              \
> >     $(SELINUX_CFLAGS)                                       \
> > diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am
> > index b6cb8f1..5e9a820 100644
> > --- a/src/mesa/Makefile.am
> > +++ b/src/mesa/Makefile.am
> > @@ -136,8 +136,14 @@ noinst_LTLIBRARIES += libmesagallium.la
> >  endif
> >
> >  AM_CPPFLAGS = $(DEFINES) $(INCLUDE_DIRS)
> > -AM_CFLAGS = $(LLVM_CFLAGS) $(VISIBILITY_CFLAGS)
> > -AM_CXXFLAGS = $(LLVM_CFLAGS) $(VISIBILITY_CXXFLAGS)
> > +AM_CFLAGS = \
> > +   $(LLVM_CFLAGS) \
> > +   $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2013_COMPAT_CFLAGS)
> > +AM_CXXFLAGS = \
> > +   $(LLVM_CFLAGS) \
> > +   $(VISIBILITY_CXXFLAGS) \
> > +   $(MSVC2013_COMPAT_CXXFLAGS)
>
> Forgive my ignorance about the build system... does applying this here
> affect building files down in src/mesa/drivers/dri?
>
> >  ARCH_LIBS =
> >
> > diff --git a/src/util/Makefile.am b/src/util/Makefile.am
> > index 9af2330..29b66e7 100644
> > --- a/src/util/Makefile.am
> > +++ b/src/util/Makefile.am
> > @@ -34,7 +34,8 @@ libmesautil_la_CPPFLAGS = \
> >     -I$(top_srcdir)/src/gallium/include \
> >     -I$(top_srcdir)/src/gallium/auxiliary \
> >     $(SHA1_CFLAGS) \
> > -   $(VISIBILITY_CFLAGS)
> > +   $(VISIBILITY_CFLAGS) \
> > +   $(MSVC2008_COMPAT_CFLAGS)
> >
> >  libmesautil_la_SOURCES = \
> >     $(MESA_UTIL_FILES) \
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=tyrvqAm0EVC5AwhM7HUFBP2BLPSrfRoV_sdwABGjAPs&s=JcfCT6dXyIWrFUu7OYokAYGUHA1XjdpL59HsvKtVyCc&e=


More information about the mesa-dev mailing list