Libtool and Support for Shared Libraries

[Greg Banks]

On Mon, May 14, 2012, at 05:34 PM, Дилян Палаузов wrote:
> Hello,
> 
> I put the libraries initially under $cyrus_prefix/lib (/usr/cyrus/lib), 
> because the directory $cyrus_prefix already existed.
> 
> If the intention is to move the files from $cyrus_prefix / $servicepath 
> (/usr/cyrus/bin), to /usr/libexec and /usr/bin, then the shared 
> libraries can be renamed to libcyrus-imap and libcyrus-sieve and 
> installed under $libdir.

I think that would be better.

> 
> > commit "convert com_err/et/libcom_err.a to a libtool archive"
> >
> > You should also fix the 2nd argument to db_panic() in lib/cyrusdb_berkeley.c.
> 
> I am not sure: gcc does not complain.  I think it matters only when 
> errno is a global variable, in db_panic() it is local.

Many systems define of errno as

extern int errno;

but that's not the only possibility; on some platforms under some
combinations of compiler flags errno is a macro that calls a function
that returns a pointer to a variable which might or might not be
thread-specific at runtime.  See

http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/head/errno.h

55 extern int *___errno();
56 #define      errno (*(___errno()))

Under these conditions, using errno as the name of a function parameter
gets you, if you're lucky, a compile error because the function
definition doesn't match an earlier declaration.  If you're unlucky or
silly, you end up with a function which you thought was defined

int foo(int errno) { ... }

but was actually defined

int foo(int (*___errno())) { ... }

which compiles but is taking a different type of argument to the one you
thought it was taking.  Furthermore it is hiding the global function
___errno() with a parameter declaration of pointer-to-function type, so
any use of errno in the function is going to call through that function
pointer instead of calling the global function. The value passed for
that parameter will be from a caller which thinks that the type of the
parameter is an int, and will be a small integer error number which will
most definitely not be a valid pointer to a function.  The result is
that any use of the parameter in the function will SEGV.

In this specific case, we're getting away with it because

a) db_panic() doesn't use it's errno argument anywhere, and

b) db_panic() is almost never called, and when it is the situation is so
dire that exit() is the only option anyway, so SEGVing here would go
unnoticed and unfixed, and

c) in the one place where db_panic() is used, it's explicitly cast to
the type the author was expecting

static void db_panic(DB_ENV *dbenv __attribute__((unused)),
		     int errno __attribute__((unused)))
...
    dbenv->set_paniccall(dbenv, (void (*)(DB_ENV *, int)) &db_panic);

which suppress any compile error.  I bet someone discovered that compile
error and "fixed" it with a cast.

> 
> > Also, I notice that libcom_err.la is 'noinst'.  How do you expect that cyrus executable will be able to find this code at runtime on a platform which has shared libraries but no system com_err library?
> 
> When the compilation of the internal com_err is requested, the files 
> from libcom_err.la are compiled with -fPIC and statically linked in 
> libimap and libsieve. 

Two questions - 

1) is this happening by accident because we're currently adding -fPIC to
the global CFLAGS via @PERL_CCLDFLAGS@ ?

2) would it not be better to bundle the libcom_err code into libcyrus.so
?

> That is how the files are found at run time. 
> This has the disadvantage, that running timsieved and lmtpd with shared 
> libraries support, loads libimap and libsieve, so the statically linked 
> code of libcim_err is loaded twice.  An option would be to install 
> com_err.la as shared libcyrus_com_err, which is loaded by both libimap 
> and libsieve.

That would also work.

-- 
Greg.