Doh!
Many thanks -- I'll apply this in all the right places...
On Feb 16, 2012, at 3:20 PM, Paul H. Hargrove wrote:
>
> 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
>
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
--
Jeff Squyres
jsquy...@cisco.com
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/
