Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: [OMPI devel] -fvisibility probe broken [w/ 3-line PATCH]
From: Paul H. Hargrove (PHHargrove_at_[hidden])
Date: 2012-02-16 15:20:35


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                          PHHargrove_at_[hidden]
Future Technologies Group
HPC Research Department                   Tel: +1-510-495-2352
Lawrence Berkeley National Laboratory     Fax: +1-510-486-6900