Thread: [PATCH] Reduce dependancies of postmaster (without --as-needed)
As a recent bug pointed out, you really only want to link libraries you actually need because the others could export functions that you don't want and interfere. Rather than relying a GCC only feature, this takes the good old fashioned approach: filtering out the libraries you don't need. Attached is a patch which applies this filtering to the backend and has the same results as linking with --as-needed. I basically took the filter list of libpq and altered it as follows: libs removed: -lnsl -lresolv libs added: -ldl -lm Incidently, the libs removed don't appear to be needed by anything in any file in the source, so I wonder if they're even required. This probably needs a bit of testing on a few different architechtures to make sure it works. I also only apply it to the non-windows, non-aix case since the building requirements there are different enough that someone with such a platforn should do it. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them.
Attachment
Martijn van Oosterhout wrote: > Attached is a patch which applies this filtering to the backend and has > the same results as linking with --as-needed. I basically took the > filter list of libpq and altered it as follows: > > libs removed: -lnsl -lresolv > libs added: -ldl -lm Hmm, we don't need -ldl? How do we implement OPEN etc if not without dlopen()? [looks around] I see this is with pg_dlopen which in turn is dlopen in Linux and others. Did you try contrib's regression test? plperl's? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Sun, Nov 27, 2005 at 06:30:59PM -0300, Alvaro Herrera wrote: > Martijn van Oosterhout wrote: > > > Attached is a patch which applies this filtering to the backend and has > > the same results as linking with --as-needed. I basically took the > > filter list of libpq and altered it as follows: > > > > libs removed: -lnsl -lresolv > > libs added: -ldl -lm > > Hmm, we don't need -ldl? How do we implement OPEN etc if not without > dlopen()? [looks around] I see this is with pg_dlopen which in turn is > dlopen in Linux and others. Did you try contrib's regression test? > plperl's? ??? I added -ldl, I didn't remove it. Note, it uses $(filter) which removes things not in the list. Maybe you're thinking of $(filter-out)? None of this affects contrib or plperl because they use completely different link lines. I've been thinking about -lnsl and -lresolv and it occurred to me that maybe on some platforms they are needed. However, on Linux they are 100% redundant. There -lnsl provides functions for NIS and YP and -lresolv provides functions for making your own DNS packets, neither of which PostgreSQL does. Unfortunatly the autoconf test doesn't look for a particular function it simply includes the lib if it is present. Does anyone remember what required functions are provided by these libs? Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them.
Attachment
Martijn van Oosterhout wrote: > On Sun, Nov 27, 2005 at 06:30:59PM -0300, Alvaro Herrera wrote: > > Martijn van Oosterhout wrote: > > > > > Attached is a patch which applies this filtering to the backend and has > > > the same results as linking with --as-needed. I basically took the > > > filter list of libpq and altered it as follows: > > > > > > libs removed: -lnsl -lresolv > > > libs added: -ldl -lm > > > > Hmm, we don't need -ldl? How do we implement OPEN etc if not without > > dlopen()? [looks around] I see this is with pg_dlopen which in turn is > > dlopen in Linux and others. Did you try contrib's regression test? > > plperl's? > > ??? I added -ldl, I didn't remove it. Note, it uses $(filter) which > removes things not in the list. Maybe you're thinking of $(filter-out)? > None of this affects contrib or plperl because they use completely > different link lines. Yeah, I got the sense of it backwards. I mentioned plperl and contrib just because they use our dynlib loading facility. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Martijn van Oosterhout <kleptog@svana.org> writes:
> I've been thinking about -lnsl and -lresolv and it occurred to me that
> maybe on some platforms they are needed. However, on Linux they are
> 100% redundant. There -lnsl provides functions for NIS and YP and
> -lresolv provides functions for making your own DNS packets, neither of
> which PostgreSQL does. Unfortunatly the autoconf test doesn't look for
> a particular function it simply includes the lib if it is present. Does
> anyone remember what required functions are provided by these libs?
Pulling those out is just not a good idea; we'd never have included them
in the first place if they weren't needed on some platforms. A lot of
these system libraries are very hard to test for in a reasonable way.
For instance, IIRC the reason libBSD is needed on HP-UX is that it
provides POSIX-compatible signal behavior. The same functions exist in
libc ... but they work differently :-(
I think the patch as proposed is entirely wrongheaded, and what it
should do is filter out the stuff the backend can certainly do without
(ie, readline and termcap), not make unsupported assumptions that we can
do without stuff that was at one time put in for a reason.
regards, tom lane
On Mon, Nov 28, 2005 at 10:18:08AM -0500, Tom Lane wrote: > Pulling those out is just not a good idea; we'd never have included them > in the first place if they weren't needed on some platforms. A lot of > these system libraries are very hard to test for in a reasonable way. > For instance, IIRC the reason libBSD is needed on HP-UX is that it > provides POSIX-compatible signal behavior. The same functions exist in > libc ... but they work differently :-( Yeah, but pulling them in when they're not needed is a waste also. I'm sure that a lot of platforms have -lnsl but I doubt many need it given it's for NIS/YP support. libBSD doesn't bother me as much because it's not going to exist on 99% of platforms. > I think the patch as proposed is entirely wrongheaded, and what it > should do is filter out the stuff the backend can certainly do without > (ie, readline and termcap), not make unsupported assumptions that we can > do without stuff that was at one time put in for a reason. Sure, you can go that way too. I went this way because that's what the libpq Makefile does. It bothers me that we don't even *know* the dependancies or even why they're there. Those autoconf lines have been there ever since autoconf use was started. They were added with only the comment "detect these libraries". If -lnsl or -lresolv is needed for gethostbyname() on some platform then it should check that that function is actually in that library before pulling it in, rather than pulling it unconditionally. I know reducing the library count is not a high priority, but this is the only time in a release cycle these issues can be resolved. All that needs to happen is for someone to rerun the final link command removing all the -l options, noting which symbols it complains about and which lib they are in. Then change the configure test to: AC_CHECK_LIB(<lib>, <function>) Or at least put a comment there saying what platform and what symbol. The reason I'm bringing this up is that unnecessary dependancies make upgrading systems unnecessarily complicated due to libs like libpq pulling in specific versions of libs that conflict with libraries used by the program using libpq. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them.
Attachment
On Mon, Nov 28, 2005 at 01:58:09PM -0500, Tom Lane wrote: > I have no problem with trying to make configure more selective about > which libraries we need at all. That's an orthogonal problem from > what the backend makefile should try to filter out, though. With > respect to system libraries, I would think that the backend needs > everything we need at all --- certainly everything to do with > networking. Threading support and readline are the only things > I can see that are reasonable to omit from the backend link (and > we already take care of the threading bit I believe). Regarding the restrictions for backend libs, consider the attached patch. It just filters out anything readline related. Threading LIBS are not in the LIBS variable at all. Regarding the other, this is the kind of thing the buildfarm would be good for. Would it be possible to arrange for each buildfarm machine to execute the following after successful completion and capture the output? It only needs to be run once. cd $SOURCEDIR/src/backend rm postgres make LIBS=-lm postgres It'll produce a lot of output but for example on my linux machine the only undefined symbols are from -ldl and -lcrypt. This would provide a solid base on which to make improvements. Thanks in advance, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them.
Attachment
Martijn van Oosterhout <kleptog@svana.org> writes:
> It bothers me that we don't even *know* the dependancies or even why
> they're there. Those autoconf lines have been there ever since autoconf
> use was started. They were added with only the comment "detect these
> libraries". If -lnsl or -lresolv is needed for gethostbyname() on some
> platform then it should check that that function is actually in that
> library before pulling it in, rather than pulling it unconditionally.
I have no problem with trying to make configure more selective about
which libraries we need at all. That's an orthogonal problem from
what the backend makefile should try to filter out, though. With
respect to system libraries, I would think that the backend needs
everything we need at all --- certainly everything to do with
networking. Threading support and readline are the only things
I can see that are reasonable to omit from the backend link (and
we already take care of the threading bit I believe).
regards, tom lane
On Mon, Nov 28, 2005 at 09:01:11PM +0100, Martijn van Oosterhout wrote: > Regarding the restrictions for backend libs, consider the attached > patch. It just filters out anything readline related. Threading LIBS > are not in the LIBS variable at all. Sorry, forgot the attachment. -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them.
Attachment
Martijn van Oosterhout wrote: >Regarding the other, this is the kind of thing the buildfarm would be >good for. Would it be possible to arrange for each buildfarm machine to >execute the following after successful completion and capture the >output? It only needs to be run once. > >cd $SOURCEDIR/src/backend >rm postgres >make LIBS=-lm postgres > >It'll produce a lot of output but for example on my linux machine the >only undefined symbols are from -ldl and -lcrypt. This would provide a >solid base on which to make improvements. > > > > Not really, no. The tests performed are hardcoded, and we have no infrastructure for distributing additional tests, nor for doing one-shot tests. cheers andrew
Martijn van Oosterhout <kleptog@svana.org> writes:
>> Regarding the restrictions for backend libs, consider the attached
>> patch. It just filters out anything readline related. Threading LIBS
>> are not in the LIBS variable at all.
> Sorry, forgot the attachment.
Applied in a modified format --- I see no reason why it's not safe for
AIX or Windows in this form.
regards, tom lane
Index: Makefile
===================================================================
RCS file: /cvsroot/pgsql/src/backend/Makefile,v
retrieving revision 1.110
diff -c -r1.110 Makefile
*** Makefile 27 Oct 2005 20:45:29 -0000 1.110
--- Makefile 28 Nov 2005 22:05:04 -0000
***************
*** 24,29 ****
--- 24,32 ----
# We put libpgport into OBJS, so remove it from LIBS
LIBS := $(patsubst -lpgport, , $(LIBS))
+ # The backend doesn't need everything that's in LIBS, however
+ LIBS := $(filter-out -lreadline -ledit -ltermcap -lncurses -lcurses, $(LIBS))
+
ifeq ($(PORTNAME), qnx4)
# This file crashes qnx4's wlink and is therefore not in
# bootstrap/SUBSYS.o on that platform. (Wotta hack ... is it still
Martijn van Oosterhout <kleptog@svana.org> writes: > It bothers me that we don't even *know* the dependancies or even why > they're there. Those autoconf lines have been there ever since autoconf > use was started. Not all of them. I dug a bit in the cvs history, and found that -lresolv was added quite late, apparently in response to this: http://archives.postgresql.org/pgsql-hackers/2001-02/msg00030.php Now that we don't support Kerb4 anymore, maybe we don't need it... but testing with Kerb5 would surely be prudent before deciding that. -lnsl seems to have been there since the beginning. -lPW was added "for SCO support" in 1997: http://developer.postgresql.org/cvsweb.cgi/pgsql/src/Attic/configure.in.diff?r1=1.64;r2=1.65 regards, tom lane
On Tue, Nov 29, 2005 at 01:16:53AM -0500, Tom Lane wrote: > I dug a bit in the cvs history, and found that -lresolv was added > quite late, apparently in response to this: > > http://archives.postgresql.org/pgsql-hackers/2001-02/msg00030.php Marvellous. Another shared library doesn't wasn't linked against a library it needs. This idea that shared libraries should be allowed to have undefined symbols is something that really should be deprecated. Kerberos support on my Debian system doesn't have this bug but then Debian has always been stricter about this kind of thing. > -lPW was added "for SCO support" in 1997: > http://developer.postgresql.org/cvsweb.cgi/pgsql/src/Attic/configure.in.diff?r1=1.64;r2=1.65 Ok, I've added it to the platforms/libraries dependancies list I'm compiling. I've got all the relevent info from the old Makefile.in. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them.