Re: libpq's pollution of application namespace

I find that libpq.so exports the following symbols that have neither
PQ, pq, pg, nor lo_ as a prefix:

EncryptMD5
SockAddr_cidr_mask
fe_getauthname
fe_getauthsvc
fe_sendauth
fe_setauthsvc
freeaddrinfo_all
getaddrinfo_all
getnameinfo_all
md5_hash
rangeSockAddr

md5_hash seems a particularly unforgivable intrusion on application
namespace :-(. Any objection to fixing these things to be prefixed
with pq or pg, which is the convention we usually follow for "internal"
names that can't be static?

Also, these functions strictly speaking violate application namespace,
but given that PQ appears infix, they're probably OK.

appendBinaryPQExpBuffer
appendPQExpBuffer
appendPQExpBufferChar
appendPQExpBufferStr
createPQExpBuffer
destroyPQExpBuffer
enlargePQExpBuffer
initPQExpBuffer
printfPQExpBuffer
resetPQExpBuffer
termPQExpBuffer

It'd be nicer if we could filter out all exported symbols that don't
appear in exports.txt, but I don't know any portable way to do that.

regards, tom lane

On Sun, Oct 16, 2005 at 06:21:37PM -0400, Tom Lane wrote:
> I find that libpq.so exports the following symbols that have neither
> PQ, pq, pg, nor lo_ as a prefix:

<snip>

> It'd be nicer if we could filter out all exported symbols that don't
> appear in exports.txt, but I don't know any portable way to do that.

With GNU LD it is trivial, using the --version-script command. If you
use AWK to create the script from exports.txt like so:

awk 'BEGIN { print "{ global: " } { if( $1 != "#" ) {print $1,";"} } END { print "local: *; };" }' <exports.txt >exports.version

And then add "-Wl,--version-script,exports.version" to the link
command, viola, stray symbols removed. Given we already have a
configure test for GNU ld, it wouldn't be too hard to make it work for
them. For windows it already uses exports.txt. What other linkers do we
need to support?

Another possibility would be to use strip like so:

strip -w -K PQ* -K pq* -K pg* -K lo_* -K *PQ* -o output.so

But then, that may be a GNU strip extention... And it doesn't follow
the exports file then.

Recent gcc versions support visibility directives in the source code but
that's a lot more work (although doing it in the code would produce a
more efficient library). And not portable to other compilers either...

Hope this helps,
--
Martijn van Oosterhout <kleptog(at)svana(dot)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.

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> What other linkers do we need to support?

All the non-GNU ones.

(I'm already desperately unhappy about the thin representation of
non-GNU toolchains in the build farm...)

regards, tom lane

Tom Lane wrote:

>Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
>
>
>>What other linkers do we need to support?
>>
>>
>
>All the non-GNU ones.
>
>(I'm already desperately unhappy about the thin representation of
>non-GNU toolchains in the build farm...)
>
>
>
>

Me too. If you provide a list of the most important platforms/toolsets
missing I will see if I can talk some people into donating resources.
HPUX is a glaring hole although I know you have that covered personally.

cheers

cheers

andrew

On Mon, Oct 17, 2005 at 12:20:06PM -0400, Tom Lane wrote:
> Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> > What other linkers do we need to support?
>
> All the non-GNU ones.
>
> (I'm already desperately unhappy about the thin representation of
> non-GNU toolchains in the build farm...)

Ok, so it's a matter of research and testing. HPUX for example don't
have a version map and doesn't have the strip options either, but
allows you to specify:

+hideallsymbols +e sym +e sym

You can dump them all into a file and pull it in with "-c filename"

I can see a list of supported platforms [1], but not a list of
supported compilers/linkers. If it's just a matter of reasearching the
command-line options that can be done fairly easily, if anyone's
interested...

Have a nice day,

[1] http://www.postgresql.org/docs/8.0/interactive/supported-platforms.html
--
Martijn van Oosterhout <kleptog(at)svana(dot)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.

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> I can see a list of supported platforms [1], but not a list of
> supported compilers/linkers. If it's just a matter of reasearching the
> command-line options that can be done fairly easily, if anyone's
> interested...

(a) This problem is really not worth the trouble.

(b) I dislike portability approaches that try to enumerate supported
cases, rather than being general in the first place. Especially when
we can be pretty certain that this area is so unstandardized that *no*
toolchain you haven't specifically coded a case for will work.

regards, tom lane

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> (I'm already desperately unhappy about the thin representation of
>> non-GNU toolchains in the build farm...)

> Me too. If you provide a list of the most important platforms/toolsets
> missing I will see if I can talk some people into donating resources.

Well, one way to attack it is to look at the current supported-platforms
list and try to get buildfarm representation for everything not covered
already.

http://developer.postgresql.org/docs/postgres/supported-platforms.html

I don't think we need more buildfarms running more random distros of
Linux ;-) --- unless they are running non-gcc compilers. People
should be encouraged to test with non-gcc compilers if they have any
available.

We seem to be short on buildfarm representation for AIX, HPUX, Solaris
(particularly older variants), Tru64; it'd be nice to cover all the
hardware platforms each of these runs on. For that matter, we're a bit
thin on the unusual-hardware ports of the *BSDen.

regards, tom lane

On Mon, Oct 17, 2005 at 01:32:47PM -0400, Tom Lane wrote:
> (a) This problem is really not worth the trouble.
>
> (b) I dislike portability approaches that try to enumerate supported
> cases, rather than being general in the first place. Especially when
> we can be pretty certain that this area is so unstandardized that *no*
> toolchain you haven't specifically coded a case for will work.

Well, cleaning up your exported namespace is recommended as it also
affects the loading time of applications. I'm just wondering given that
libpq can be pulled into any unsuspecting application via PAM
(libpam-pgsql) or NSS (libnss-pgsql1), we should be at least trying to
cut down the exported symbols.

Changing the names to something less likely to conflict is good. I just
think it may be worthwhile to solve it for the common platform (gcc)
and worry about the others later (if ever).

BTW, I think you missed:

promote_v4_to_v6_addr
promote_v4_to_v6_mask

--
Martijn van Oosterhout <kleptog(at)svana(dot)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.

On Mon, 2005-17-10 at 13:32 -0400, Tom Lane wrote:
> I dislike portability approaches that try to enumerate supported cases,
> rather than being general in the first place.

Do we need to have this on every platform we support? The symbols we
want to hide are internal by convention anyway -- using a linker script
or similar technique just improves upon this by preventing applications
from misbehaving (and it also improves performance slightly). If no one
has bothered to add support for a particular platform's linker they
won't get these benefits, but that doesn't seem like a disaster.

-Neil

I think it is a good idea to make the exported symbols clearer.
We should only export the symbols needed. The
output of "dlltool --export-all" is too big.
AFAIK, we can generate *.def for Win32/MSVC++
from a text file like this.
PQclear
PQfn
FooGlobalData DATA

"Neil Conway" <neilc(at)samurai(dot)com> wrote
> On Mon, 2005-17-10 at 13:32 -0400, Tom Lane wrote:
>> I dislike portability approaches that try to enumerate supported cases,
>> rather than being general in the first place.
>
> Do we need to have this on every platform we support? The symbols we
> want to hide are internal by convention anyway -- using a linker script
> or similar technique just improves upon this by preventing applications
> from misbehaving (and it also improves performance slightly). If no one
> has bothered to add support for a particular platform's linker they
> won't get these benefits, but that doesn't seem like a disaster.
>
> -Neil
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

On Thu, Oct 20, 2005 at 06:20:37PM +0800, William ZHANG wrote:
> I think it is a good idea to make the exported symbols clearer.
> We should only export the symbols needed. The
> output of "dlltool --export-all" is too big.
> AFAIK, we can generate *.def for Win32/MSVC++
> from a text file like this.
> PQclear
> PQfn
> FooGlobalData DATA

AIUI, we *do* limit the symbols for Windows (for libpq anyway, see
exports.txt file) we just don't for UNIX since it can't be done
portably. I posted a patch to -patches which does it for Linux and in
principle any platform using GCC but there's no consensus on whether we
should do it if it can't be done for everyone in a simple way.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)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.

Martijn van Oosterhout wrote:
> AIUI, we *do* limit the symbols for Windows (for libpq anyway, see
> exports.txt file) we just don't for UNIX since it can't be done
> portably. I posted a patch to -patches which does it for Linux and in
> principle any platform using GCC but there's no consensus on whether we
> should do it if it can't be done for everyone in a simple way.

Now I understand the problem. We can not find a portable way to do it.
But we can support GCC now, then add support for other compilers.
As time goes on, we can finally solve it.

Regards, William Zhang

This has been saved for the 8.2 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Martijn van Oosterhout wrote:
-- Start of PGP signed section.
> On Sun, Oct 16, 2005 at 06:21:37PM -0400, Tom Lane wrote:
> > I find that libpq.so exports the following symbols that have neither
> > PQ, pq, pg, nor lo_ as a prefix:
>
> <snip>
>
> > It'd be nicer if we could filter out all exported symbols that don't
> > appear in exports.txt, but I don't know any portable way to do that.
>
> With GNU LD it is trivial, using the --version-script command. If you
> use AWK to create the script from exports.txt like so:
>
> awk 'BEGIN { print "{ global: " } { if( $1 != "#" ) {print $1,";"} } END { print "local: *; };" }' <exports.txt >exports.version
>
> And then add "-Wl,--version-script,exports.version" to the link
> command, viola, stray symbols removed. Given we already have a
> configure test for GNU ld, it wouldn't be too hard to make it work for
> them. For windows it already uses exports.txt. What other linkers do we
> need to support?
>
> Another possibility would be to use strip like so:
>
> strip -w -K PQ* -K pq* -K pg* -K lo_* -K *PQ* -o output.so
>
> But then, that may be a GNU strip extention... And it doesn't follow
> the exports file then.
>
> Recent gcc versions support visibility directives in the source code but
> that's a lot more work (although doing it in the code would produce a
> more efficient library). And not portable to other compilers either...
>
> Hope this helps,
> --
> Martijn van Oosterhout <kleptog(at)svana(dot)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.
-- End of PGP section, PGP failed!

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Thread added to TODO:

o Properly mark all libpq-exported functions with "PQ"

---------------------------------------------------------------------------

Tom Lane wrote:
> I find that libpq.so exports the following symbols that have neither
> PQ, pq, pg, nor lo_ as a prefix:
>
> EncryptMD5
> SockAddr_cidr_mask
> fe_getauthname
> fe_getauthsvc
> fe_sendauth
> fe_setauthsvc
> freeaddrinfo_all
> getaddrinfo_all
> getnameinfo_all
> md5_hash
> rangeSockAddr
>
> md5_hash seems a particularly unforgivable intrusion on application
> namespace :-(. Any objection to fixing these things to be prefixed
> with pq or pg, which is the convention we usually follow for "internal"
> names that can't be static?
>
> Also, these functions strictly speaking violate application namespace,
> but given that PQ appears infix, they're probably OK.
>
> appendBinaryPQExpBuffer
> appendPQExpBuffer
> appendPQExpBufferChar
> appendPQExpBufferStr
> createPQExpBuffer
> destroyPQExpBuffer
> enlargePQExpBuffer
> initPQExpBuffer
> printfPQExpBuffer
> resetPQExpBuffer
> termPQExpBuffer
>
> It'd be nicer if we could filter out all exported symbols that don't
> appear in exports.txt, but I don't know any portable way to do that.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
>

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Thread added to TODO:
> o Properly mark all libpq-exported functions with "PQ"

Is that still relevant? I thought we'd done as much as we intended
to do in that specific direction. What would make sense to work on
is making the build step that strips not-officially-exported symbols
work on more platforms.

regards, tom lane

On Wed, Jun 14, 2006 at 05:54:56PM -0400, Bruce Momjian wrote:
>
> Thread added to TODO:
>
> o Properly mark all libpq-exported functions with "PQ"

I thought this was done already. At least, with a recent CVS I get
this:

$ nm -D libpq.so --defined-only |grep -v 'PQ\|pq\|lo_\|pg_'
000171e0 D pgresStatus

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Thread added to TODO:
> > o Properly mark all libpq-exported functions with "PQ"
>
> Is that still relevant? I thought we'd done as much as we intended
> to do in that specific direction. What would make sense to work on

Oh, OK.

> is making the build step that strips not-officially-exported symbols
> work on more platforms.

Uh, well, I had some patches in the patch queue to go in that direction,
but I thought the conclusion in that thread was that it wasn't worth it.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom Lane wrote:
>> What would make sense to work on
>> is making the build step that strips not-officially-exported symbols
>> work on more platforms.

> Uh, well, I had some patches in the patch queue to go in that direction,
> but I thought the conclusion in that thread was that it wasn't worth it.

We currently have coverage for Linux and Darwin. If we had coverage for
the *BSDen I would figure it was close enough ... is it possible to do
the *BSDen with a few more makefile lines, or is it too ugly?

regards, tom lane

On Wed, Jun 14, 2006 at 06:23:36PM -0400, Bruce Momjian wrote:
> > is making the build step that strips not-officially-exported symbols
> > work on more platforms.
>
> Uh, well, I had some patches in the patch queue to go in that direction,
> but I thought the conclusion in that thread was that it wasn't worth it.

That's my recollection too. I had something that supported HPUX for
example but it was decided not worth the effort (can't find it right
now though...).

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.