diff mbox

[libcilkrts,build] Only use visibility if supported

Message ID ydd38mzgjp3.fsf@lokon.CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth Nov. 14, 2013, 2:08 p.m. UTC
libcilkrts.so fails to link on Solaris 9/x86 with Sun as since this
configuration lacks visibility support:

Text relocation remains                         referenced
    against symbol                  offset      in file
__cilkrts_get_tls_worker            0x41        .libs/cilk-abi.o
__cilkrts_get_tls_worker            0x254       .libs/cilk-abi.o
__cilkrts_get_tls_worker            0x5c9       .libs/cilk-abi.o
__cilkrts_get_tls_worker            0x609       .libs/cilk-abi.o
__cilkrts_get_tls_worker            0x6e7       .libs/cilk-abi.o
__cilkrts_get_tls_worker            0x774       .libs/cilk-abi.o
__cilkrts_get_tls_worker            0x271       .libs/cilk-abi-cilk-for.o
__cilkrts_get_tls_worker            0x361       .libs/cilk-abi-cilk-for.o
__cilkrts_get_tls_worker            0x10        .libs/cilk_api.o
__cilkrts_get_tls_worker            0xc1        .libs/cilk_api.o
__cilkrts_get_tls_worker            0x194       .libs/cilk_api.o
__cilkrts_get_tls_worker            0x241       .libs/cilk_api.o
__cilkrts_get_tls_worker            0x324       .libs/cilk_api.o
__cilkrts_get_tls_worker            0x706       .libs/reducer_impl.o
__cilkrts_get_tls_worker            0x893       .libs/reducer_impl.o
[...]
__cilkrts_bump_worker_rank_internal 0x32c       .libs/cilk_api.o
__cilkrts_init_worker_sysdep        0x262c      .libs/scheduler.o
ld: fatal: relocations remain against allocatable but non-writable sections
collect2: error: ld returned 1 exit status
make[2]: *** [libcilkrts.la] Error 1

During the build, one sees many warnings like this:

/vol/gcc/src/hg/trunk/local/libcilkrts/runtime/cilk-abi.c: In function '__cilkrt
s_enter_frame_fast':
/vol/gcc/src/hg/trunk/local/libcilkrts/runtime/cilk-abi.c:144:1: warning: visibi
lity attribute not supported in this configuration; ignored [-Wattributes]

This can be cured by actually checking if a given configuration supports
__attribute((visibility)) instead of just assuming gcc on Unix systems
does.  The following patch does just that.

Tested by reconfiguring and rebuilding libcilkrts on i386-pc-solaris2.9
(which now builds) and i386-pc-solaris2.11 (where elfdump -s still show
many symbols as protected as they should be).

Ok for mainline?

	Rainer


2013-11-14  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.ac (libcilkrts_cv_have_attribute_visibility): Check
	for __attribute__((visibility)).
	* configure: Regenerate.
	* include/cilk/common.h (CILK_EXPORT, CILK_EXPORT_DATA): Only use
	__attribute__((visibility)) if HAVE_ATTRIBUTE_VISIBILITY.

Comments

Paolo Bonzini Nov. 15, 2013, 11:10 a.m. UTC | #1
Il 14/11/2013 15:08, Rainer Orth ha scritto:
> libcilkrts.so fails to link on Solaris 9/x86 with Sun as since this
> configuration lacks visibility support:
> 
> Text relocation remains                         referenced
>     against symbol                  offset      in file
> __cilkrts_get_tls_worker            0x41        .libs/cilk-abi.o
> __cilkrts_get_tls_worker            0x254       .libs/cilk-abi.o
> __cilkrts_get_tls_worker            0x5c9       .libs/cilk-abi.o
> __cilkrts_get_tls_worker            0x609       .libs/cilk-abi.o
> __cilkrts_get_tls_worker            0x6e7       .libs/cilk-abi.o
> __cilkrts_get_tls_worker            0x774       .libs/cilk-abi.o
> __cilkrts_get_tls_worker            0x271       .libs/cilk-abi-cilk-for.o
> __cilkrts_get_tls_worker            0x361       .libs/cilk-abi-cilk-for.o
> __cilkrts_get_tls_worker            0x10        .libs/cilk_api.o
> __cilkrts_get_tls_worker            0xc1        .libs/cilk_api.o
> __cilkrts_get_tls_worker            0x194       .libs/cilk_api.o
> __cilkrts_get_tls_worker            0x241       .libs/cilk_api.o
> __cilkrts_get_tls_worker            0x324       .libs/cilk_api.o
> __cilkrts_get_tls_worker            0x706       .libs/reducer_impl.o
> __cilkrts_get_tls_worker            0x893       .libs/reducer_impl.o
> [...]
> __cilkrts_bump_worker_rank_internal 0x32c       .libs/cilk_api.o
> __cilkrts_init_worker_sysdep        0x262c      .libs/scheduler.o
> ld: fatal: relocations remain against allocatable but non-writable sections
> collect2: error: ld returned 1 exit status
> make[2]: *** [libcilkrts.la] Error 1
> 
> During the build, one sees many warnings like this:
> 
> /vol/gcc/src/hg/trunk/local/libcilkrts/runtime/cilk-abi.c: In function '__cilkrt
> s_enter_frame_fast':
> /vol/gcc/src/hg/trunk/local/libcilkrts/runtime/cilk-abi.c:144:1: warning: visibi
> lity attribute not supported in this configuration; ignored [-Wattributes]
> 
> This can be cured by actually checking if a given configuration supports
> __attribute((visibility)) instead of just assuming gcc on Unix systems
> does.  The following patch does just that.
> 
> Tested by reconfiguring and rebuilding libcilkrts on i386-pc-solaris2.9
> (which now builds) and i386-pc-solaris2.11 (where elfdump -s still show
> many symbols as protected as they should be).
> 
> Ok for mainline?
> 
> 	Rainer
> 
> 
> 2013-11-14  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	* configure.ac (libcilkrts_cv_have_attribute_visibility): Check
> 	for __attribute__((visibility)).
> 	* configure: Regenerate.
> 	* include/cilk/common.h (CILK_EXPORT, CILK_EXPORT_DATA): Only use
> 	__attribute__((visibility)) if HAVE_ATTRIBUTE_VISIBILITY.
> 
> 
> 
> 

Any chance to move the test to config/?

Otherwise ok.

Paolo
Rainer Orth Nov. 22, 2013, 12:12 p.m. UTC | #2
Paolo Bonzini <bonzini@gnu.org> writes:

>> 2013-11-14  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>> 
>> 	* configure.ac (libcilkrts_cv_have_attribute_visibility): Check
>> 	for __attribute__((visibility)).
>> 	* configure: Regenerate.
>> 	* include/cilk/common.h (CILK_EXPORT, CILK_EXPORT_DATA): Only use
>> 	__attribute__((visibility)) if HAVE_ATTRIBUTE_VISIBILITY.
>> 
>
> Any chance to move the test to config/?

I wondered about this myself.  A quick assement of visibility checks in
gcc revealed the following:

* gcc: test needs to be kept separate since it checks assembler and
  linker support directly

* libatomic, libcilkrts, libgfortran, libgomp, libitm, libquadmath,
  libssp: all define HAVE_ATTRIBUTE_VISIBILITY in either acinclude.m4 or
  configure.ac

  This set is the least problematic, most libs only use hidden
  visibility in their code, libcilkrts differs: protected and default
  instead.  Perhaps the autoconf macro needs to be parameterized to
  select which visibilities to check for?

* libffi: defines HAVE_HIDDEN_VISIBILITY_ATTRIBUTE instead, need to
  check if including new config/visibility.m4 is acceptable in upstream
  repo

* libgcc: used to define make variable instead, should be adaptable,
  uses protected, hidden, default

* libstdc++-v3: used to define _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY,
  prefix might fall from libstdc++-v3 configury automatically

> Otherwise ok.

Since this fixes a bootstrap failure, I've installed the current patch
for the moment.

	Rainer
diff mbox

Patch

# HG changeset patch
# Parent fc2fe9ba5a63f3442f35bc4dc8cfece05c7577ca
Only use visibility if supported

diff --git a/libcilkrts/configure.ac b/libcilkrts/configure.ac
--- a/libcilkrts/configure.ac
+++ b/libcilkrts/configure.ac
@@ -54,6 +54,18 @@  AC_CONFIG_FILES([Makefile])
 AM_ENABLE_MULTILIB(, ..)
 AC_FUNC_ALLOCA
 
+# Check whether the target supports protected visibility.
+save_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -Werror"
+AC_TRY_COMPILE([void __attribute__((visibility("protected"))) foo(void) { }],
+	       [], libcilkrts_cv_have_attribute_visibility=yes,
+	       libcilkrts_cv_have_attribute_visibility=no)
+CFLAGS="$save_CFLAGS"
+if test $libcilkrts_cv_have_attribute_visibility = yes; then
+  AC_DEFINE(HAVE_ATTRIBUTE_VISIBILITY, 1,
+    [Define to 1 if the target supports __attribute__((visibility(...))).])
+fi
+
 # Get target configury.
 . ${srcdir}/configure.tgt
 if test -n "$UNSUPPORTED"; then
diff --git a/libcilkrts/include/cilk/common.h b/libcilkrts/include/cilk/common.h
--- a/libcilkrts/include/cilk/common.h
+++ b/libcilkrts/include/cilk/common.h
@@ -101,7 +101,7 @@  namespace cilk {
 #   define CILK_EXPORT      /* nothing */
 #   define CILK_EXPORT_DATA /* nothing */
 #else /* Unix/gcc */
-#   ifdef IN_CILK_RUNTIME
+#   if defined(IN_CILK_RUNTIME) && defined(HAVE_ATTRIBUTE_VISIBILITY)
 #       define CILK_EXPORT      __attribute__((visibility("protected")))
 #       define CILK_EXPORT_DATA __attribute__((visibility("protected")))
 #   else