Re: [2.2.7a] (patch) runtime-linking and soname-equivalent together on AIX

[Ralf Wildenhues]

Hello Michael,

apologies for the long delay.

* Michael Haubenwallner wrote on Fri, Nov 21, 2008 at 03:33:35PM CET:
> 
> now here is a patch to have both runtime-linking (-brtl) _and_ a
> 'soname'-equivalent at the same time on AIX, and this by default!

I have a number of questions; reordering your mail:

> +     _LT_TAGVAR(lt_prog_compiler_shared_as_static, $1)='${wl}-bstatic'
> +     _LT_TAGVAR(lt_prog_compiler_shared_as_shared, $1)='${wl}-bdynamic'

What is the semantic difference between these switches and this proposal
<http://thread.gmane.org/gmane.comp.gnu.libtool.patches/6727/focus=6730>?
If the difference is too small, we should apply the other patch first
and adjust your patch accordingly, no?

This patch is missing NEWS and ChangeLog entries.  It should also have
some documentation bit about AIX, see doc/notes.texi, and documentation
for new libtool variables, see near the end of doc/libtool.texi.
The patch is quite large, it would be nice if it were possible to split
it logically.

I still haven't grasped whether, and if, how, this patch creates a flag
day for Libtool users on AIX.  IOW, if you have a number of packages
that create shared libraries, modules, module loaders using libltdl,
all with or without runtimelinking, in what way will packages play
together when they are not all updated to the new Libtool at once?

If there are upgrading issues, then can they be resolved by leaving the
addition of -Wl,-brtl up to the user?  More generally, why does your
patch mix the two (more or less) unrelated items of changing the
default, and changing the semantics of shared items in static libs?

Thanks also for doing all the testing.  Out of curiosity, have you
tested or are you able to test AIX 4.3.3?  (I can do so otherwise.)

Generally, it would be nice to have somebody with good AIX experience
comment on this.

A couple of nits inline.

> --- a/libltdl/config/ltmain.m4sh
> +++ b/libltdl/config/ltmain.m4sh
> @@ -515,6 +515,27 @@ func_ltwrapper_p ()
>      func_ltwrapper_script_p "$1" || func_ltwrapper_executable_p "$1"
>  }
>  
> +# func_dlname_to_filename dlname
> +# Extracts the real filename out of dlname.
> +# This is necessary for AIX, where dlname might be 'filename(soname)'.
> +func_dlname_to_filename ()
> +{
> +    if test -n "$dlname_spec" ; then
> +     save_ifs=$IFS; IFS='()'; set dummy $1; shift; IFS=$save_ifs
> +    fi
> +    func_dlname_to_filename_result=$1
> +}
> +
> +# func_dlname_to_soname dlname
> +# Extracts the soname out of 'dlname', see func_dlname_to_filename().

No need to add '()' when referring to functions.  (The GNU Coding
Standards actually recommend against doing so.)

> +func_dlname_to_soname ()
> +{
> +    if test -n "$dlname_spec" ; then
> +     save_ifs=$IFS; IFS='()'; set dummy $1; shift; IFS=$save_ifs
> +     test "$#" -lt 1 || shift

What is this line for?  The '-lt 1' looks wrong, maybe -le, or
  test $# -gt 1 && shift

The $# doesn't need quoting.

> +    fi
> +    func_dlname_to_soname_result=$1
> +}


> @@ -5447,7 +5473,17 @@ func_mode_link ()
>           fi
>           if test "$linkmode" = prog; then
>             test -n "$add_dir" && compile_deplibs="$add_dir $compile_deplibs"
> +           # AIX can link 'shared objects' statically:
> +           test "$use_static_libs" != no &&
> +             test "$use_static_libs" != all &&
> +             test -n "$link_shared_as_static_flag" &&
> +             eval compile_deplibs=\"$link_shared_as_static_flag 
> \$compile_deplibs\"

I haven't grasped all the logic involved here yet, but this line needs
quoting, like
  eval "compile_deplibs=\"$link_shared_as_static_flag \$compile_deplibs\""

(several instances).  Also, the logic seems quite redundant, with
several repetitions.

> --- a/libltdl/loaders/dlopen.c
> +++ b/libltdl/loaders/dlopen.c
> @@ -169,6 +169,11 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char 
> *filename,
>    int                module_flags = LT_LAZY_OR_NOW;
>    lt_module  module;
>  
> +#ifdef RTLD_MEMBER
> +  if (filename && strchr(filename, '(') && strchr(filename, ')'))

Sure you don't want to start looking for the closing parenthesis _after_
the position of the opening one?

> +    module_flags |= RTLD_MEMBER;
> +#endif
> +
>    if (advise)
>      {
>  #ifdef RTLD_GLOBAL

> --- a/libltdl/m4/libtool.m4
> +++ b/libltdl/m4/libtool.m4

> +      case "$aix_use_runtimelinking:$aix_runtimelinking_flag_set" in

No need to quote here.

> +      yes:no)
> +     LDFLAGS="-Wl,-brtl ${LDFLAGS}"
> +     export LDFLAGS
> +     AC_MSG_RESULT([adding -Wl,-brtl to LDFLAGS])
> +     ;;
> +      yes:yes)
> +     AC_MSG_RESULT([present])
> +     ;;
> +      no:*)
> +     AC_MSG_RESULT([not wanted])
> +     ;;
> +      esac
> +      ;;
> +    *)
> +      aix_use_runtimelinking=no
> +      AC_MSG_RESULT([not available])
> +      ;;
> +    esac
> +  fi
>  esac
>  
>  # Global variables:

> +    # Although standalone shared objects (libname.so) are supported since
> +    # AIX4.2, the only way to get some 'soname' support (on Power*) is to
> +    # store the shared object into an archive library.

s/into/in/

> +    dlname_spec='`case "${soname}:${allow_undefined}" in 
> "${libname}${release}.so:yes") echo "${soname}";; *":yes") echo 
> "${libname}${release}.so(${soname})";; *) echo 
> "${libname}${release}.a(${soname})";; esac`'

Likewise, no need for quoting after 'case'.

> @@ -2628,6 +2680,8 @@ _LT_DECL([], [soname_spec], [1],
>      [[The coded name of the library, if different from the real name]])
>  _LT_DECL([], [install_override_mode], [1],
>      [Permission mode override for installation of shared libraries])
> +_LT_DECL([], [dlname_spec], [1],
> +    [[The loadable name of the library, if different from the soname, as on 
> AIX]])

Needs documenting in libtool.texi.

> @@ -4072,6 +4138,10 @@ _LT_LINKER_OPTION([if $compiler static flag 
> $lt_tmp_static_flag works],
>    [_LT_TAGVAR(lt_prog_compiler_static, $1)=])
>  _LT_TAGDECL([link_static_flag], [lt_prog_compiler_static], [1],
>       [Compiler flag to prevent dynamic linking])
> +_LT_TAGDECL([link_shared_as_static_flag], 
> [lt_prog_compiler_shared_as_static], [1],
> +     [Compiler flag to link subsequent shared objects statically - AIX can 
> do that])
> +_LT_TAGDECL([link_shared_as_shared_flag], 
> [lt_prog_compiler_shared_as_shared], [1],
> +     [Compiler flag to link subsequent shared objects dynamically])

Likewise.

> +     if test "$aix_use_runtimelinking" = no; then
> +       # In case we do not have runtime-linking, we cannot allow undefined
> +       # symbols with '-berok', as this would produce broken shared objects.
> +       _LT_TAGVAR(allow_undefined_flag, $1)=unsupported
> +     else
> +       # In case we have runtime-linking enabled, we create shared objects

With runtime-linking, we ...

> +       # with '-G', which enables both '-brtl' and '-berok' among others.
> +       # Any executable linking against any shared object with undefined
> +       # symbols needs to be linked with '-brtl', to allow them being
> +       # resolved at runtime.
> +       _LT_TAGVAR(prelink_cmds, $1)='func_append compile_command " 
> ${wl}-brtl"~func_append finalize_command " ${wl}-brtl"'
> +     fi

> +     # REMEMBER1: libtool's default is to enable runtime-linking.
> +     # REMEMBER2: libtool's default is to allow undefined symbols.
> +     # We could create 'libname.so' as 'standalone import file' containing
> +     # the loadable name along the list of exported symbols to enforce
> +     # autoloading of this shared object, either as standalone shared object
> +     # or as shared archive member - which seems like the easiest way to
> +     # implement the 'soname' feature on AIX.
> +     # But then we are unable to inform the linker of undefined symbols
> +     # in that shared object any more.
> +     #
> +     # So we put the shared object into an archive library - libltdl can
> +     # dynamically load that now too - to get the 'soname' support.
> +     # This is similar to how AIX traditionally builds its shared libraries.

This is good reasoning, but:
- it would be better if it were written not for the person looking at
  the code prior to your patch, but for one looking at the code which
  has had your patch incorporated for a while already.
- Ideally, this would belong in a new chapter/section in the manual
  ("history of AIX support/semantics" or so).  Nobody will find this
  in the code.

> +     # See also comments around 'dlname_spec' definition for AIX.
> +     #
> +     # To have runtime-linking enabled in subsequent link steps with libtool,
> +     # '-brtl' is added to the 'inherited_linker_flags' libtool variable.
> +     #
> +     # A shared object as part of an archive library is recorded into the
> +     # resulting executable as to-be-loaded _only_ when there are direct
> +     # symbol references from statically linked objects, as long as the
> +     # referencing symbols are not garbage-collected too.
> +     # With runtime-linking, only standalone shared objects are recorded
> +     # into the executable as to-be-loaded, even when not referenced.
> +     # This breaks situations where one shared object has undefined
> +     # symbols which are defined in other shared objects being an archive
> +     # member and not referenced otherways - this IMO is an AIX linker bug
> +     # (seen on AIX 5.3, can't say for others).
> +     # So we tell the linker to prevent all undefined symbols in
> +     # this shared object from being garbage-collected, using
> +     # 'inherited_linker_flags' libtool variable again.
> +     #
> +     # Create the shared object with 'soname' as filename:
> +     # We add -bnortllib after $compiler_flags, because we might have
> +     # inherited -brtl from $LDFLAGS or $inherited_linker_flags, which
> +     # re-enables rtllib, even when disabled within -G before.
> +     # We don't put -G at the end, because the user might want to
> +     # override -bnosymbolic or -bnoautoexp.
> +     # From ld(1) man page:
> +     #   The -G flag is equivalent to specifying the erok, rtl, nortllib,
> +     #   nosymbolic, noautoexp, and M:SRE options with the -b flag.
> +     #   Subsequent options can override these options.

I'm not so sure whether letting users inherit flags is such a great
idea.  It would be more elegant to be able to do without.

> +     cmds="\$CC $shared_flag"' -o $output_objdir/$soname $libobjs $deplibs 
> ${wl}-bnoentry $compiler_flags ${wl}-bnortllib 
> ${wl}-bE:${export_symbols}${allow_undefined_flag}'
> +     #
> +     # In case of undefined symbols, enforce '-brtl' for subsequent linking:
> +     #
> +     cmds="${cmds}"'~test "x${allow_undefined}" = "xno" || func_append 
> new_inherited_linker_flags " \${wl}-brtl"'
> +     #
> +     # Explicitly inform subsequent link steps of undefined symbols:
> +     #
> +     cmds="${cmds}"'~test "x${allow_undefined}" = "xno" || undefs=`dump -Tv 
> ${output_objdir}/${soname} | $SED -n '"'"'s/.* EXTref  *\.\.  *\([[^ ]]*\) 
> *$/ \${wl}-u\1/p'"'"' | \$NL2SP`'

We shouldn't use 'dump' unadorned like this.  No idea whether there are
cross compiling setups where $host is AIX but $build is not, but for
them, this should be factorized.

> +     cmds="${cmds}"'~test -z "${undefs}" || func_append 
> new_inherited_linker_flags "`echo '"' '"'${undefs}`"'
> +     #
> +     # Reuse the decision from 'dlname_spec':
> +     # whether to keep 'libname.so' as standalone shared object,
> +     #      or to create 'libname.so' as archive library,
> +     #      or to create 'libname.a' as archive library.
> +     #
> +     cmds="${cmds}"'~func_dlname_to_filename "${dlname}"'
> +     cmds="${cmds}"'~test "x${func_dlname_to_filename_result}" = 
> "x${soname}" || $AR $AR_FLAGS 
> ${output_objdir}/${func_dlname_to_filename_result} ${output_objdir}/${soname}'
> +     cmds="${cmds}"'~test "x${func_dlname_to_filename_result}" = 
> "x${soname}" || $RM ${output_objdir}/${soname}'
> +     #
> +     # When creating/keeping 'libname.so', still create the old 'libname.a':
> +     #
> +     cmds="${cmds}"'~case "${func_dlname_to_filename_result}" in *".a") ;; 
> *) build_old_libs=yes; func_append oldlibs " $output_objdir/$libname.a";; 
> esac'
> +     _LT_TAGVAR(archive_expsym_cmds, $1)=${cmds}
>        fi
>        ;;
[...]

> --- a/tests/lt_dladvise.at
> +++ b/tests/lt_dladvise.at
> @@ -318,8 +318,9 @@ dlpreloadable='preload'
>  # are reporting not able to accept the global hint to lt_dlopenadvise().    #
>  # ------------------------------------------------------------------------- #
>  
> -case $host_os in
> -cygwin* | mingw* | cegcc*)
> +eval `$LIBTOOL --config | grep '^allow_undefined_flag=`
> +case $host_os:$allow_undefined_flag in
> +cygwin* | mingw* | cegcc* | *:unsupported)
>    # These hosts do not support linking without -no-undefined
>    CPPFLAGS="$CPPFLAGS -DHAVE_UNDEFINED_SYMBOLS=0"
>    ;;

Doesn't this fix a bug already?  If yes, then this can be applied as a
separate patch.

Thanks,
Ralf