readline build questions

readline build questions

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I'm trying to resolve a bash bug caused by relying on auto-import rather
than __declspec(dllimport): http://cygwin.com/ml/cygwin/2008-11/msg00340.html

I've managed to mark up the readline headers with:

#ifdef READLINE_NO_IMPORT
# ifdef RL_SHARED
#  define RL_IMPORT __declspec(dllexport)
# else
#  define RL_IMPORT
# endif
#else
# define RL_IMPORT __declspec(dllimport)
#endif

then adding RL_IMPORT to every extern declaration, along with appropriate
definitions for READLINE_NO_IMPORT and RL_SHARED in the makefiles that
build the dynamic and static readline libraries.  This appears to solve
the shared library issue, such that bash dynamically linked against this
library does the right thing.  But where I used to be able to link a
single .o both dynamically and statically, I'm now getting failures with
the static link:

make[1]: Entering directory `/home/eblake/readline-5.2.13-11/build/examples'
gcc -g -L../shlib -L..  -o fileman.exe fileman.o ../shlib/cygreadline6.dll
- -lcurses
gcc -g -L../shlib -L..  -o fileman-stat.exe fileman.o ../libreadline.a
- -lcurses
fileman.o:fileman.c:(.text+0x42): undefined reference to
`__imp__rl_readline_name'

Am I still doing something wrong?

I noticed that the ld info pages suggest the following idiom:

    /* Note: auto-export is assumed (no __declspec(dllexport)) */
    #if (defined(_WIN32) || defined(__CYGWIN__)) && \
      !(defined(FOO_BUILD_DLL) || defined(FOO_STATIC))
    #define FOO_IMPORT __declspec(dllimport)
    #else
    #define FOO_IMPORT
    #endif

Is it still possible to allow clients to choose between static and dynamic
readline at link-time without supplying any compile-time flags, or does
the fact that I am conditionally using __declspec mean that I have to
adjust my conditions in readline.h, and that clients that want to link
statically now have to define READLINE_STATIC in their own source?

One other idea I had was to quit distributing a static libreadline.a, and
only offer the dynamic version.  Does anyone see any problems with the
idea of no longer providing a static library?

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkkuxggACgkQ84KuGfSFAYAnxQCffLRZh/9i5kqqYfqJSZlub/m1
PzYAn17xac68Ct3+P+uhNdJC2FRCF9aN
=eRkO
-----END PGP SIGNATURE-----

Re: readline build questions

[Brian Dessent]

Eric Blake wrote:

> I'm trying to resolve a bash bug caused by relying on auto-import rather
> than __declspec(dllimport): http://cygwin.com/ml/cygwin/2008-11/msg00340.html

There is another potential workaround that doesn't involve modifying the
readline headers.  If there are only a few places that compare function
pointers, then just compare against the _imp__FOO symbol directly.

extern char *_imp__rl_tab_insert;

...

if (function_returning_function_pointer () == _imp__rl_tab_insert)
  {
    /* function returned pointer to rl_tab_insert() in the DLL.  */
  }

This will however fail if you try to link bash against a static readline
because there will be no sythesized _imp__FOO symbol, causing link
failure.  But here the breakage/restriction is limited to bash, rather
than being in the readline headers, i.e. it doesn't affect all readline
clients.

> Is it still possible to allow clients to choose between static and dynamic
> readline at link-time without supplying any compile-time flags, or does
> the fact that I am conditionally using __declspec mean that I have to
> adjust my conditions in readline.h, and that clients that want to link
> statically now have to define READLINE_STATIC in their own source?

Unfortunately, yes.  A call to a function explicitly declared dllimport
can only be satisfied by linking to a DLL containing that function, as
essentially the compiler has inlined the indirection of the thunk stub
into the call site.  So the client of such a library needs an explicit
compile-time way of declaring intent to link statically, removing the
__declspec.

> One other idea I had was to quit distributing a static libreadline.a, and
> only offer the dynamic version.  Does anyone see any problems with the
> idea of no longer providing a static library?

You could make the argument that any consumers of libreadline in the
distro should already be using the shared version anyway on general
principle, so this shouldn't affect them.  But then there could be
potential users that want to statically link it into their own
programs.  I'd say continue to provide the static version, but just note
in the readme the new requirement to configure with
CPPFLAGS=-DREADLINE_STATIC if they intend to statically link.  It's a
reasonable request given that it's not an uncommon idiom which is found
in other libraries, and it fixes a real bug.  (What's more, if anyone
complains you can tell them that the alternative under consideration was
simple removal of the static version in We're Just Mean fashion.)

Brian

Re: readline build questions

[Charles Wilson]

Brian Dessent wrote:
> Unfortunately, yes.  A call to a function explicitly declared dllimport
> can only be satisfied by linking to a DLL containing that function, as
> essentially the compiler has inlined the indirection of the thunk stub
> into the call site.  So the client of such a library needs an explicit
> compile-time way of declaring intent to link statically, removing the
> __declspec.

Well...there IS another way. If you look at the build machinery of
gettext and libiconv, Bruno Haible has implemented a mechanism where ALL
of the library's symbols are ALWAYS decorated using declspec(dllimport)
-- whether the client is linking statically or dynamically.  However,
when building the static library, he has a bit of magic that explicitly
synthesizes the _imp_* symbols.

This still requires all symbols in the headers to be "uglified" with
some READLINE_IMPEXP symbol, that is "" on non-win32, but
unconditionally becomes declspec(dllimport) on win32.

> You could make the argument that any consumers of libreadline in the
> distro should already be using the shared version anyway on general
> principle, so this shouldn't affect them.  But then there could be
> potential users that want to statically link it into their own
> programs.  I'd say continue to provide the static version, but just note
> in the readme the new requirement to configure with
> CPPFLAGS=-DREADLINE_STATIC if they intend to statically link.  It's a
> reasonable request given that it's not an uncommon idiom which is found
> in other libraries, and it fixes a real bug.  (What's more, if anyone
> complains you can tell them that the alternative under consideration was
> simple removal of the static version in We're Just Mean fashion.)

Wow. Full circle: back before we had auto-import, I used to maintain
libreadline in exactly this fashion. It was a PAIN -- I sympathize with
Eric, because readline has a LOT of symbols that need decorating if you
go that route.

Further, Chet Ramey felt strongly about NOT uglifying the code with
those READLINE_IMPEXP symbols everywhere, so at the time those changes
were forever destined to be maintained out-of-tree, by hand. I don't
know if Chet has relaxed his position...if not, then I recommend that
Eric do the following (unless he's just a glutton for punishment):

1) Don't explicitly mark symbols as declspec(foo), unless Chet is
willing to accept the changes
2) Do pointer comparisons explicitly against _imp_foo. This will break
static builds, but at least it will be a much smaller out-of-tree patch
to maintain in perpetuity.
3) Drop the static library.

The only non-painful alternative is *if* Chet accepts the
READLINE_IMPEXP uglifications, then you should consider some of the
other options, such as requiring -DREADLINE_STATIC, or attempting to
duplicate Bruno's machinery.

--
Chuck

Re: readline build questions

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Charles Wilson on 11/27/2008 6:28 PM:
> Wow. Full circle: back before we had auto-import, I used to maintain
> libreadline in exactly this fashion. It was a PAIN -- I sympathize with
> Eric, because readline has a LOT of symbols that need decorating if you
> go that route.

For now, I went with no decorations in readline, and manually patching the
few instances in bash that try to refer to the address of a function
provided by readline (at least, I hope I found all the instances).

> 
> Further, Chet Ramey felt strongly about NOT uglifying the code with
> those READLINE_IMPEXP symbols everywhere, so at the time those changes
> were forever destined to be maintained out-of-tree, by hand. I don't
> know if Chet has relaxed his position...if not, then I recommend that
> Eric do the following (unless he's just a glutton for punishment):
> 
> 1) Don't explicitly mark symbols as declspec(foo), unless Chet is
> willing to accept the changes

Well, I asked Chet if he will add the uglification for the upcoming
readline 6.0 (now in beta), on the grounds that the gcc manual
*recommends* compiling libraries with -fvisibility=hidden and explicit
__attribute__((__visibility__ "default")).  I still haven't heard back
from him, but if I am successful at convincing him that mainstream Linux
would benefit from the uglification, rather than just fringe cygwin, then
it will be much easier to coerce that uglification into working for
cygwin's needs.

> 2) Do pointer comparisons explicitly against _imp_foo. This will break
> static builds, but at least it will be a much smaller out-of-tree patch
> to maintain in perpetuity.

I did this for bash; no changes needed for readline to do this hack (but
every client that dynamically uses readline must evaluate their code for
any references to functions imported from readline, and do the same hack).

> 3) Drop the static library.

Unless/until readline is uglified with decorations, there's nothing wrong
with providing both static and dynamic.  But I may decide to go with
dynamic-only when readline 6.0 is released, even if Chet doesn't use the
uglification route.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkkxgtEACgkQ84KuGfSFAYACnACgrF3RzDVrmVZYi8HM6/lnqIfT
9qsAoMxN2Jguc7wFD0/hG95mclkZwnj+
=11g7
-----END PGP SIGNATURE-----