[PATCH v2] Check if gcc complains about __attribute__ (visibility(..))

[PATCH v2] Check if gcc complains about __attribute__ (visibility(..))

[Ulf Hermann]

If so, define attribute_hidden to be empty. Also, use attribute_hidden
in all places where we hide symbols. If this attribute is missing, it
simply means that we cannot hide private symbols in the binary using
attributes. This disables some optimizations and may increase the risk 
of symbol name clashes with other libraries, but is not fatal. 

However, we still employ linker version scripts to explicitly define 
the exported symbols. This serves much of the same purpose. Also, as
all our symbols are prefixed with the library name, and "__" for 
private ones, the chance of clashes is low anyway.

(I'm reposting this patch, rebased and with extended commit message. v1
was apparently not accepted because the patch's implications were
misunderstood.)

Signed-off-by: Ulf Hermann <ulf.hermann@qt.io>
---
 ChangeLog           |  5 +++++
 configure.ac        | 16 ++++++++++++++++
 lib/ChangeLog       |  5 +++++
 lib/eu-config.h     |  4 ++++
 libdw/ChangeLog     |  5 +++++
 libdw/libdwP.h      |  2 +-
 libdw/libdw_alloc.c |  2 +-
 libelf/ChangeLog    |  4 ++++
 libelf/libelfP.h    |  2 +-
 9 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 62146227..84fd2555 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-27  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* configure.ac: Check if the compiler supports
+	__attribute__((visibility(...))).
+
+2017-04-27  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* configure.ac: Check if -fPIC, -fPIE, -Wl,-z,defs,
 	and -Wl,-z,relro are supported by the compiler.
 
diff --git a/configure.ac b/configure.ac
index e6e3b675..c4fc7e3d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -127,6 +127,22 @@ CFLAGS="$old_CFLAGS"])
 AS_IF([test "x$ac_cv_c99" != xyes],
       AC_MSG_ERROR([gcc with GNU99 support required]))
 
+AC_CACHE_CHECK([whether gcc supports __attribute__((visibility()))],
+	ac_cv_visibility, [dnl
+save_CFLAGS="$CFLAGS"
+CFLAGS="$save_CFLAGS -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([dnl
+int __attribute__((visibility("hidden")))
+foo (int a)
+{
+  return a;
+}])], ac_cv_visibility=yes, ac_cv_visibility=no)
+CFLAGS="$save_CFLAGS"])
+if test "$ac_cv_visibility" = "yes"; then
+	AC_DEFINE([HAVE_VISIBILITY], [1],
+		  [Defined if __attribute__((visibility())) is supported])
+fi
+
 AC_CACHE_CHECK([whether gcc supports -fPIC], ac_cv_fpic, [dnl
 save_CFLAGS="$CFLAGS"
 CFLAGS="$save_CFLAGS -fPIC -Werror"
diff --git a/lib/ChangeLog b/lib/ChangeLog
index 67ef2792..23c0f41b 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-27  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* eu-config.h: Define attribute_hidden to be empty if the compiler
+	doesn't support it.
+
+2017-04-27  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* Makefile.am: Use fpic_CFLAGS.
 
 2017-07-18  Mark Wielaard  <mark@klomp.org>
diff --git a/lib/eu-config.h b/lib/eu-config.h
index 400cdc6e..07098282 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -68,8 +68,12 @@
 #define internal_strong_alias(name, aliasname) \
   extern __typeof (name) aliasname __attribute__ ((alias (#name))) internal_function;
 
+#ifdef HAVE_VISIBILITY
 #define attribute_hidden \
   __attribute__ ((visibility ("hidden")))
+#else
+#define attribute_hidden /* empty */
+#endif
 
 /* Define ALLOW_UNALIGNED if the architecture allows operations on
    unaligned memory locations.  */
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 67d7799d..c13344af 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,5 +1,10 @@
 2017-02-27  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* libdwP.h: Use attribute_hidden.
+	* libdw_alloc.c: Likewise.
+
+2017-02-27  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* Makefile.am: Use fpic_CFLAGS and dso_LDFLAGS.
 
 2017-07-26  Mark Wielaard  <mark@klomp.org>
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index 6ad322c1..78c00132 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -434,7 +434,7 @@ extern void *__libdw_allocate (Dwarf *dbg, size_t minsize, size_t align)
      __attribute__ ((__malloc__)) __nonnull_attribute__ (1);
 
 /* Default OOM handler.  */
-extern void __libdw_oom (void) __attribute ((noreturn, visibility ("hidden")));
+extern void __libdw_oom (void) __attribute ((noreturn)) attribute_hidden;
 
 /* Allocate the internal data for a unit not seen before.  */
 extern struct Dwarf_CU *__libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
diff --git a/libdw/libdw_alloc.c b/libdw/libdw_alloc.c
index 28a8cf6e..d6af23a2 100644
--- a/libdw/libdw_alloc.c
+++ b/libdw/libdw_alloc.c
@@ -70,7 +70,7 @@ dwarf_new_oom_handler (Dwarf *dbg, Dwarf_OOM handler)
 
 
 void
-__attribute ((noreturn, visibility ("hidden")))
+__attribute ((noreturn)) attribute_hidden
 __libdw_oom (void)
 {
   while (1)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index a0736467..9793d068 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,5 +1,9 @@
 2017-04-27  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* libelfP.h: Use attribute_hidden.
+
+2017-04-27  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* Makefile.am: Use fpic_CFLAGS and dso_LDFLAGS.
 
 2017-08-15  Mark Wielaard  <mark@klomp.org>
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index 7ee6625a..a4a0a3a9 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -578,7 +578,7 @@ extern Elf_Data *__elf64_xlatetof_internal (Elf_Data *__dest,
 extern unsigned int __elf_version_internal (unsigned int __version)
      attribute_hidden;
 extern unsigned long int __elf_hash_internal (const char *__string)
-       __attribute__ ((__pure__, visibility ("hidden")));
+       __attribute__ ((__pure__)) attribute_hidden;
 extern long int __elf32_checksum_internal (Elf *__elf) attribute_hidden;
 extern long int __elf64_checksum_internal (Elf *__elf) attribute_hidden;
 
-- 
2.11.0

Re: [PATCH v2] Check if gcc complains about __attribute__ (visibility(..))

[Mark Wielaard]

On Fri, Aug 18, 2017 at 01:06:36PM +0200, Ulf Hermann wrote:
> If so, define attribute_hidden to be empty. Also, use attribute_hidden
> in all places where we hide symbols. If this attribute is missing, it
> simply means that we cannot hide private symbols in the binary using
> attributes. This disables some optimizations and may increase the risk 
> of symbol name clashes with other libraries, but is not fatal. 
> 
> However, we still employ linker version scripts to explicitly define 
> the exported symbols. This serves much of the same purpose. Also, as
> all our symbols are prefixed with the library name, and "__" for 
> private ones, the chance of clashes is low anyway.

Like the previous one, I have my doubts. But it actually makes the
source code slightly more consistent. So applied.

Thanks,

Mark