[FFmpeg-devel] Fix libssh static linkage on Windows

[Matt Oliver]

On 11 February 2014 02:34, Lukasz M <lukasz.m.luki at gmail.com> wrote:

> On 10 February 2014 16:27, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> > On Tue, Feb 11, 2014 at 01:35:03AM +1100, Matt Oliver wrote:
> > > On 11 February 2014 00:39, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> > >
> > > > On Thu, Feb 06, 2014 at 12:13:35PM +1100, Matt Oliver wrote:
> > > > > On 6 February 2014 05:53, Reimar Döffinger <
> Reimar.Doeffinger at gmx.de
> > >
> > > > wrote:
> > > > >
> > > > > > On Thu, Feb 06, 2014 at 01:21:34AM +1100, Matt Oliver wrote:
> > > > > > > On windows there are 2 ways to link to a function found in an
> > > > external
> > > > > > > shared library (.dll on windows). The first is to generate an
> > export
> > > > > > > library with your shared dll. This export library looks just
> > like a
> > > > > > static
> > > > > > > lib but all the functions found in it point out to the external
> > dll.
> > > > The
> > > > > > > second method is to declare each function as dllimport. This
> > bypasses
> > > > > > > needing to link to the export lib by explicitly stating its
> from
> > a
> > > > dll in
> > > > > > > the code.
> > > > > > >
> > > > > > > dllimport is placed in front of any function or data
> > declarations in
> > > > code
> > > > > > > to specify that they are found in an external .dll (.so shared
> > > > library on
> > > > > > > linux). If a function is declared as dllimport then the linker
> > will
> > > > only
> > > > > > > look for that function in an external dll even if that function
> > has
> > > > been
> > > > > > > provided with a static library. So linking will always fail if
> > using
> > > > a
> > > > > > > static library to provide a function as the linker will only
> > allow
> > > > for an
> > > > > > > external shared function.
> > > > > >
> > > > > > Are you sure about that? Because that is not my experience.
> > > > > > As long as you use dllexport for the corresponding functions
> while
> > > > > > compiling the static lib I believe it should work just fine.
> > > > > > Note that you like this risk that libavformat ends up exporting
> > > > > > the libssh functions, so it's not exactly such a great idea,
> > > > > > but the underlying issue should be using dllimport where you use
> > > > > > the function but not using dllexport where you defined it.
> > > > > > Also, to quote Microsoft:
> > > > > > > dllexport of a function exposes the function with its decorated
> > name.
> > > > > > For C++ functions, this includes name mangling. For C functions
> or
> > > > > > functions that are declared as extern "C", this includes
> > > > platform-specific
> > > > > > decoration that's based on the calling convention. If you don't
> > want
> > > > name
> > > > > > decoration, use a .def file (EXPORTS keyword).
> > > > > >
> > > > > > So it's basically a name mangling issue (for C I believe it's
> just
> > > > > > that a _ is prepended).
> > > > > > It also hints that just using a .def file and none of this
> > > > > > dllimport/dllexport
> > > > > > stuff would avoid the issue as well.
> > > > > > _______________________________________________
> > > > > > ffmpeg-devel mailing list
> > > > > > ffmpeg-devel at ffmpeg.org
> > > > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > > >
> > > > >
> > > > > I probably shouldn't send emails at 2am after a late night as my
> > last one
> > > > > was a bit off. Your right in that dllimport is just a name mangling
> > thing
> > > > > that works providing you compiled the original static lib with
> > dllexport.
> > > > > Having dllimport means that it will only link with static libs
> > compiled
> > > > > using LIBSSH_EXPORTS (dllexport) as opposed to the more obvious
> > > > > LIBSSH_STATIC. However building a static with that define requires
> > the
> > > > > linking program to also use that define. That is the reason for the
> > patch
> > > > > as with the define you can link against a static lib without
> exports
> > > > while
> > > > > also being able to link against a shared lib that does use exports.
> > > > >
> > > > > Given that the shared lib project provided by the libssh project
> > defines
> > > > > LIBSSH_STATIC then any static lib using the default libssh builds
> > wont be
> > > > > compatible with ffmpeg due to the conflicting use of dllexport. But
> > by
> > > > > adding the patch then not only will standard libssh static builds
> > work
> > > > but
> > > > > the default shared builds will still work as well.
> > > >
> > > > is LIBSSH_STATIC officially defined to be safe to be defined for
> > > > dynamic linking ? (it does not matter if its now with your installed
> > > > libssh on your box)
> > > > if yes please point me to the documentation
> > > > if its defined not to be safe then this requires some #if id guess
> > > > if its undefined then i think its best if you contact the libssh
> > > > developers and ask so it gets clarified (in their docs) if
> > > > unconditional use of LIBSSH_STATIC is how its supposed to be used.
> > > >
> > > > [...]
> > > >
> > > > --
> > > > Michael     GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > I have often repented speaking, but never of holding my tongue.
> > > > -- Xenocrates
> > > >
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > >
> > > >
> > > is LIBSSH_STATIC officially defined to be safe to be defined for
> > > > dynamic linking ?
> > >
> > >
> > > Officially according to microsoft it is safe as any code in a shared
> dll
> > on
> >
> > microsoft doesnt document LIBSSH_STATIC
> >
> >
> > [...]
> > >
> > > libssh however has no stance on not using dllimport on a shared library
> > and
> > > there docs only state one or the other:
> > > http://api.libssh.org/master/libssh_linking.html
> >
> > you are quite fixed on dllimport, yet your patch does not add dllimport
> > it adds LIBSSH_STATIC.
> >
> > You seem to mix documented API with current behavior
> >
> > My question was, if adding LIBSSH_STATIC unconditional is safe per
> > official libssh documentation
> > AFAIK its not. so your patch uses it in a undocumeted way
> >
> > Using undocumeted things is not good. We would like to have wide
> > compatibility. Both with future and past versions of libs and with
> > potential forks and drop in replacements if such ever exist
> >
> > I suggest again to try to get this clarified at libssh
> > it sure would help people if their docs spell out if using it for
> > dynamic libs or non windows is safe, unsafe or undefined
> >
> > either way above are just my 2 cent
> >
> > patch review left to maintainer
>
>
> I don't know anything about windows linking mechanisms.
> I will try to ask someone from libssh for having a look on it and some
> comment if that is safe solution.
>
> On the other hand, should this device be as a part of building command
> --extra-cflags=-DLIBSSH_STATIC or somehing like that.
>
> Anyway, I will try to ask about it, if that is safe than probably is better
> for the project people doesn't have problems with building it.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

OK because the last email wasnt all that clear and in case people ask for
supporting evidence ill clarify a bit more (and everyone responded to my
email before I could clarify).

DLLs on windows traditionally use module definition files (.DEF) to list
each function that you want exported from your dll. This basically creates
the external visibility or entry points for those functions. DEF files are
often troublesome because you have to manually add each new function to the
list and remove old ones which is why dllexport came in as this declaration
when added to a function will just automatically add that function to your
exports list without you having to do it yourself. Functionally though they
are both the same.

Functions in DLLs have a entry point generally prefixed with __imp__ and
when a function is exported then it will also be added to the accompanying
.LIB file. The .LIB file contain the exported function with a standard c
naming scheme. So you can link against the exported LIB that comes with the
shared DLL and everything works fine. This is because the LIB has the
standard C function name entry points where each of these simply redirects
to the __imp__ versions.

Declaring a function as dllimport explicitly adds the __imp__ prefix and so
bypasses the standard indirection. This can have the advantage of avoiding
the redirect and potentially improving performance. That said an optimizing
compiler is often smart enough to optimize out the indirection anyway and
as ive been told ffmpeg uses alot of DCE as it stands (this time im
actually supporting it).

So to clarify by default unless LIBSSH_STATIC is declared before including
the libssh headers then the libssh headers will automatically default to
using dllimport on windows. This causes a problem as now the linker is
looking for the __imp__ versions which obviously dont exist in a static lib
and so fails. By using LIBSSH_STATIC then the dllimport is removed and
linking is performed through the .LIB file which redirects to the .DLL (and
hopefully the redirect is optimized out). This way both static and shared
dlls can be supported from the same code.

All other ffmpeg dependencies that provide native windows builds use
exports but dont auto define dllimport and just use the LIB indirection.
Only libssh defaults to dllimport which is why this issue hasnt been a
problem previously hence needing the LIBSSH_STATIC this time.

But to make things clear i will quote Microsoft: "You do not need to use
*__declspec(dllimport)* for your code to compile correctly"
As found here:
http://msdn.microsoft.com/en-us/library/aa271769(v=vs.60).aspx

So defining LIBSSH_STATIC prevents the libssh headers defaulting to
dllimport and preventing linking with static libs. Wile still supporting
linking against dlls using the standard approach of a LIB indirection. This
is how all other ffmpeg dependencies are done and as microsoft states it
will work fine.