After seeing some odd behaviors in a build of the trunk last night, I took a closer look at the configure probe for -fvisibility support and found that recent changes where applied incompletely/incorrectly. What is currently in opal/config/opal_check_visibility.m4:

        AC_LINK_IFELSE([AC_LANG_PROGRAM([[
            __attribute__((visibility("default"))) int foo;
            ]],[[fprintf(stderr, "Hello, world\n");]])],
            [],
            [AS_IF([test -s conftest.err],
                   [$GREP -iq visibility conftest.err
                    # If we find "visibility" in the stderr, then
                    # assume it doesn't work
                    AS_IF([test "$?" = "0"], [opal_add=])])
            ])

Here is a dissection of the args to AC_LINK_IFELSE:
arg1: AC_LANG_PROGRAM
arg2: action-on-success is EMPTY
arg3: action-on-failure is an AS_IF

Unfortunately that is incorrect in 3 ways:

Error #1:
The AS_IF belongs as arg2 (where there is an empty "[]" now).
That is because the intent of that logic is to "double check" a successful link to check the stderr for "visibility". The idea there is that warnings like "unknown attribute visibility ignored" will be treated as a fail.
That was the case in the "original" (r22138) version of the logic as well.

However, it appears that this logic has been broken since r23923 when Jeff recoded AC_TRY_LINK to AC_LINK_IFELSE in Oct 2010. Those changes failed to account for the fact that LINK_IFELSE takes 1 argument for the PROGRAM where TRY_LINK has separate INCLUDE and BODY arguments. That lead to the unintended movement of the AS_IF[...grep...] logic from the on-success to the on-failure slots, and nothing has been the same since.

Error #2:
action-on-failure should be another instance of "[opal_add=]", do avoid using visibility if the link test failed. This had survived r23923 as a "extra" 4th argument to AC_LINK_IFELSE, and was later removed. This error leads to use of -fvisiblity on compilers that totally failed to compile or link the test!

Error #3:
Missing include of stdio.h leads some compilers to fail the test unnecessarily. Unlike the other 2 problems, this leads to REJECTING visibility even though it is working (except that error #2 currently hides this).

These problems, which I previously detailed in off-list emails to Jeff, apparently got lost in the rush to get hwloc-1.3.2 out the door.
Here is the relatively simple correction:

--- ompi-trunk/opal/config/opal_check_visibility.m4     (revision 25941)
+++ ompi-trunk/opal/config/opal_check_visibility.m4     (working copy)
@@ -56,15 +56,15 @@

         AC_MSG_CHECKING([if $CC supports $opal_add])
         AC_LINK_IFELSE([AC_LANG_PROGRAM([[
+            #include <stdio.h>
             __attribute__((visibility("default"))) int foo;
             ]],[[fprintf(stderr, "Hello, world\n");]])],
-            [],
             [AS_IF([test -s conftest.err],
                    [$GREP -iq visibility conftest.err
                     # If we find "visibility" in the stderr, then
                     # assume it doesn't work
                     AS_IF([test "$?" = "0"], [opal_add=])])
-            ])
+            ], [opal_add=])
         AS_IF([test "$opal_add" = ""],
               [AC_MSG_RESULT([no])],
               [AC_MSG_RESULT([yes])])

<digression>
Just to confuse things, the instance of OPAL_CHECK_VISIBLITY in the libevent2013 configure is getting the result right "by accident". In that case the CFLAGS give more verbose warnings and the LINK fails (due to missing stdio.h), while yielding "visibility" in the warning message:
conftest.c(87): remark #1418: external definition with no prior declaration
              __attribute__((visibility("default"))) int foo;
</digression>

Unfortunately, the incorrect logic made it into hwloc-1.3.2.
So, I'd suggest fixing this in OMPI's embedded hwloc and hwloc's trunk also.

My sincere apologies for not having caught this in the hwloc-1.3.2 testing where Jeff and I thought we had this issue fixed. I don't know for sure how I missed re-testing the final cut, but can only guess that I left --disable-visibility in my testing scripts.

-Paul "thorough testing is a double-edged sword" Hargrove

--
Paul H. Hargrove                          phhargr...@lbl.gov
Future Technologies Group
HPC Research Department                   Tel: +1-510-495-2352
Lawrence Berkeley National Laboratory     Fax: +1-510-486-6900

Reply via email to