I'm actually waiting for Matt for a decission on the alternative solution. See
[1]. If you like, you could they the solution mentioned there, as it would be
cleaner than just "hiding" the problem.
Marc
[1] [Mesa-dev] [PATCH 1/4] configure: add visibility macro detection to configure
Hi Marc
This adds clang/gcc visibility macro detection to configure and util/macros.h.
This is can be used to conveniently add e.g. a "HIDDEN" attribute to a function.
I believe this should be OK to go in regardless of the status of patch
2. There are just a couple of trivial nitpicks.
Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
v2: use VISIBILITY_*FLAGS instead of *FLAGS directly
v3: no changeconfigure.ac | 28 ++++++----------------------
src/util/macros.h | 6 ++++++
2 files changed, 12 insertions(+), 22 deletions(-)diff --git a/configure.ac b/configure.ac
index 351027b..266764a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -189,6 +189,7 @@ AX_GCC_FUNC_ATTRIBUTE([flatten])
AX_GCC_FUNC_ATTRIBUTE([format])
AX_GCC_FUNC_ATTRIBUTE([malloc])
AX_GCC_FUNC_ATTRIBUTE([packed])
+AX_GCC_FUNC_ATTRIBUTE([visibility])AM_CONDITIONAL([GEN_ASM_OFFSETS], test "x$GEN_ASM_OFFSETS" = xyes)
@@ -245,17 +246,13 @@ if test "x$GCC" = xyes; then
AC_MSG_RESULT([yes]),
[CFLAGS="$save_CFLAGS -Wmissing-prototypes";
AC_MSG_RESULT([no])]);
+ CFLAGS=$save_CFLAGS
I'm not sure we want/need this one ?
# Enable -fvisibility=hidden if using a gcc that supports it
As spotted by Sedat, please update the comment.
- save_CFLAGS="$CFLAGS"
- AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
- VISIBILITY_CFLAGS="-fvisibility=hidden"
- CFLAGS="$CFLAGS $VISIBILITY_CFLAGS"
- AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
- [VISIBILITY_CFLAGS=""; AC_MSG_RESULT([no])]);
-
- # Restore CFLAGS; VISIBILITY_CFLAGS are added to it where needed.
- CFLAGS=$save_CFLAGS
+ if test "x${ax_cv_have_func_attribute_visibility}" = xyes; then
+ VISIBILITY_CFLAGS="-fvisibility=hidden"
+ VISIBILITY_CXXFLAGS="-fvisibility=hidden"
+ fi
As these two are no longer GCC "specific" we can move them out of the if
test x$GCC = xyes, conditional.
# Work around aliasing bugs - developers should comment this out
CFLAGS="$CFLAGS -fno-strict-aliasing"
@@ -267,19 +264,6 @@ fi
if test "x$GXX" = xyes; then
CXXFLAGS="$CXXFLAGS -Wall"- # Enable -fvisibility=hidden if using a gcc that supports it
- save_CXXFLAGS="$CXXFLAGS"
- AC_MSG_CHECKING([whether $CXX supports -fvisibility=hidden])
- VISIBILITY_CXXFLAGS="-fvisibility=hidden"
- CXXFLAGS="$CXXFLAGS $VISIBILITY_CXXFLAGS"
- AC_LANG_PUSH([C++])
- AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
- [VISIBILITY_CXXFLAGS="" ; AC_MSG_RESULT([no])]);
- AC_LANG_POP([C++])
-
- # Restore CXXFLAGS; VISIBILITY_CXXFLAGS are added to it where needed.
- CXXFLAGS=$save_CXXFLAGS
-
# Work around aliasing bugs - developers should comment this out
CXXFLAGS="$CXXFLAGS -fno-strict-aliasing"diff --git a/src/util/macros.h b/src/util/macros.h
index eec8b93..7682511 100644
--- a/src/util/macros.h
+++ b/src/util/macros.h
@@ -117,6 +117,12 @@ do { \
#define PRINTFLIKE(f, a)
#endif+#ifdef HAVE_FUNC_ATTRIBUTE_VISIBILITY
+#define HIDDEN __attribute__((visibility("hidden")))
+#else
+#define HIDDEN
+#endif
+
We already define HIDDEN in the asm code.
Can you check (build or otherwise) that things don't clash for
--enable-asm and --{en,dis}able-shared-glapi. Check the configure log,
as we conveniently toggle the latter.
Thanks
Emil
Hi Marc
> This adds clang/gcc visibility macro detection to configure and
> util/macros.h. This is can be used to conveniently add e.g. a "HIDDEN"
> attribute to a function.
I believe this should be OK to go in regardless of the status of patch
2. There are just a couple of trivial nitpicks.
as pointed out, not if there is a better solution. It would be nice if people
could test the alternative first.
> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> ---
> v2: use VISIBILITY_*FLAGS instead of *FLAGS directly
> v3: no change
>
> configure.ac | 28 ++++++----------------------
> src/util/macros.h | 6 ++++++
> 2 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 351027b..266764a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -189,6 +189,7 @@ AX_GCC_FUNC_ATTRIBUTE([flatten])
>
> AX_GCC_FUNC_ATTRIBUTE([format])
> AX_GCC_FUNC_ATTRIBUTE([malloc])
> AX_GCC_FUNC_ATTRIBUTE([packed])
>
> +AX_GCC_FUNC_ATTRIBUTE([visibility])
>
> AM_CONDITIONAL([GEN_ASM_OFFSETS], test "x$GEN_ASM_OFFSETS" = xyes)
>
> @@ -245,17 +246,13 @@ if test "x$GCC" = xyes; then
>
> AC_MSG_RESULT([yes]),
> [CFLAGS="$save_CFLAGS -Wmissing-prototypes";
>
> AC_MSG_RESULT([no])]);
>
> + CFLAGS=$save_CFLAGSI'm not sure we want/need this one ?
it restores the CFLAGS from the test above. In fact I just moved it from the
blow upwards. Maybe the diff could be shorter.
> # Enable -fvisibility=hidden if using a gcc that supports it
As spotted by Sedat, please update the comment.
> - save_CFLAGS="$CFLAGS"
> - AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
> - VISIBILITY_CFLAGS="-fvisibility=hidden"
> - CFLAGS="$CFLAGS $VISIBILITY_CFLAGS"
> - AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
> - [VISIBILITY_CFLAGS=""; AC_MSG_RESULT([no])]);
> -
> - # Restore CFLAGS; VISIBILITY_CFLAGS are added to it where needed.
> - CFLAGS=$save_CFLAGS
> + if test "x${ax_cv_have_func_attribute_visibility}" = xyes; then
> + VISIBILITY_CFLAGS="-fvisibility=hidden"
> + VISIBILITY_CXXFLAGS="-fvisibility=hidden"
> + fiAs these two are no longer GCC "specific" we can move them out of the if
test x$GCC = xyes, conditional.
right.
> # Work around aliasing bugs - developers should comment this out
> CFLAGS="$CFLAGS -fno-strict-aliasing"
>
> @@ -267,19 +264,6 @@ fi
>
> if test "x$GXX" = xyes; then
>
> CXXFLAGS="$CXXFLAGS -Wall"
>
> - # Enable -fvisibility=hidden if using a gcc that supports it
> - save_CXXFLAGS="$CXXFLAGS"
> - AC_MSG_CHECKING([whether $CXX supports -fvisibility=hidden])
> - VISIBILITY_CXXFLAGS="-fvisibility=hidden"
> - CXXFLAGS="$CXXFLAGS $VISIBILITY_CXXFLAGS"
> - AC_LANG_PUSH([C++])
> - AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
> - [VISIBILITY_CXXFLAGS="" ; AC_MSG_RESULT([no])]);
> - AC_LANG_POP([C++])
> -
> - # Restore CXXFLAGS; VISIBILITY_CXXFLAGS are added to it where needed.
> - CXXFLAGS=$save_CXXFLAGS
> -
>
> # Work around aliasing bugs - developers should comment this out
> CXXFLAGS="$CXXFLAGS -fno-strict-aliasing"
>
> diff --git a/src/util/macros.h b/src/util/macros.h
> index eec8b93..7682511 100644
> --- a/src/util/macros.h
> +++ b/src/util/macros.h
> @@ -117,6 +117,12 @@ do { \
>
> #define PRINTFLIKE(f, a)
> #endif
>
> +#ifdef HAVE_FUNC_ATTRIBUTE_VISIBILITY
> +#define HIDDEN __attribute__((visibility("hidden")))
> +#else
> +#define HIDDEN
> +#endif
> +We already define HIDDEN in the asm code.
Can you check (build or otherwise) that things don't clash for
--enable-asm and --{en,dis}able-shared-glapi. Check the configure log,
as we conveniently toggle the latter.
I always build with --enable-asm (or better without --disable-asm) and saw no
symbol clash. I guess because the required header is not included at the same
time. Is this possible at all (asm vs. c files)?
Another interesting point is in which case we can build without shared-glapi.
I failed to build ES1 or ES2 alone, because it seem to always require glapi.
This would mean that the dispatch table always begins with the glapi. Maybe I
confuse things, but in this case, just using
extern void shared_dispatch_stub_0();
would solve all problems without the HIDDEN hacks.
Marc
Hi Marc
This adds clang/gcc visibility macro detection to configure and
util/macros.h. This is can be used to conveniently add e.g. a "HIDDEN"
attribute to a function.I believe this should be OK to go in regardless of the status of patch
2. There are just a couple of trivial nitpicks.as pointed out, not if there is a better solution. It would be nice if people
could test the alternative first.
Can you point out which alternative you have in mind. I'm a bit lost in
this thread.
...
@@ -245,17 +246,13 @@ if test "x$GCC" = xyes; then
AC_MSG_RESULT([yes]),
[CFLAGS="$save_CFLAGS -Wmissing-prototypes";
AC_MSG_RESULT([no])]);+ CFLAGS=$save_CFLAGS
I'm not sure we want/need this one ?
it restores the CFLAGS from the test above. In fact I just moved it from the
blow upwards. Maybe the diff could be shorter.
Indeed it seems like it's slightly busted currently.
Although the extra line does not seem to make it better though 
Presently it adds to the final CFLAGS,
-Werror=implicit-function-declaration
-Werror=missing-prototypes
or optionally
-Wmissing-prototypes
but with your change it won't add either one.
...
I always build with --enable-asm (or better without --disable-asm) and saw no
symbol clash. I guess because the required header is not included at the same
time. Is this possible at all (asm vs. c files)?
Pretty sure you can - #include and #define are preprocessor commands.
But as you noticed there was no clash, so we're ok.
Another interesting point is in which case we can build without shared-glapi.
I failed to build ES1 or ES2 alone, because it seem to always require glapi.
Yes, to prevent even greater chaos or build permutations we've been
mandating shared-glapi if more than one GL* api is selected. To solve
this, just drop es{1,2} from the configure line.
This would mean that the dispatch table always begins with the glapi. Maybe I
confuse things, but in this case, just using
extern void shared_dispatch_stub_0();
would solve all problems without the HIDDEN hacks.
The so called hack is patch 2, afaict. Having a single generic
definition of the attribute sounds like good patch regardless of that patch.
I'm guessing scons and Android could use a
-DHAVE_FUNC_ATTRIBUTE_VISIBILITY somewhere but that can be done as a
follow-up.
Emil
Please CC me on patches for testing.
( I have updated my LLVM/Clang to v3.6.0rc4. )
- Sedat -
Hmm. following this thread I don't see a real patch for application,
please point to that one (patchwork preferred).
Thanks.
- Sedat -