Strip -mmacosx-version-min options from plperl build

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

This is because the link command uses the option
-mmacosx-version-min=11.3, which comes in from perl_embed_ldflags (perl
-MExtUtils::Embed -e ldopts), but the compile commands don't use that
option, which creates a situation that ld considers inconsistent.

I think an appropriate fix is to strip out the undesired option from
perl_embed_ldflags. We already do that for other options. Proposed
patch attached.

Agreed on rejecting -mmacosx-version-min, but I wonder if we should
think about adopting a whitelist-instead-of-blacklist approach to
adopting stuff from perl_embed_ldflags. ISTR that in pltcl we already
use the approach of accepting only -L and -l, and perhaps similar
strictness would serve us well here.

As an example, on a not-too-new MacPorts install, I see

$ /opt/local/bin/perl -MExtUtils::Embed -e ldopts
-L/opt/local/lib -Wl,-headerpad_max_install_names -fstack-protector-strong -L/opt/local/lib/perl5/5.28/darwin-thread-multi-2level/CORE -lperl

I can't see any really good reason why we should allow perl
to be injecting that sort of -f option into the plperl build,
and I'm pretty dubious about the -headerpad_max_install_names
bit too.

I think also that this would allow us to drop the weird dance of
trying to subtract ccdlflags.

regards, tom lane

On 18.08.22 15:53, Tom Lane wrote:

Agreed on rejecting -mmacosx-version-min, but I wonder if we should
think about adopting a whitelist-instead-of-blacklist approach to
adopting stuff from perl_embed_ldflags. ISTR that in pltcl we already
use the approach of accepting only -L and -l, and perhaps similar
strictness would serve us well here.

As an example, on a not-too-new MacPorts install, I see

$ /opt/local/bin/perl -MExtUtils::Embed -e ldopts
-L/opt/local/lib -Wl,-headerpad_max_install_names -fstack-protector-strong -L/opt/local/lib/perl5/5.28/darwin-thread-multi-2level/CORE -lperl

I can't see any really good reason why we should allow perl
to be injecting that sort of -f option into the plperl build,
and I'm pretty dubious about the -headerpad_max_install_names
bit too.

I think also that this would allow us to drop the weird dance of
trying to subtract ccdlflags.

After analyzing the source code of ExtUtils::Embed's ldopts, I think we
can also do this by subtracting $Config{ldflags}, since

my $linkage = "$ccdlflags $ldflags @archives $ld_or_bs";

and we really just want the $ld_or_bs part. (@archives should be empty
for our uses.)

This would get rid of -mmacosx-version-min and -arch and all the things
you showed, including -L/opt/local/lib, which is probably there so that
the build of Perl itself could look there for things, but we don't need it.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

After analyzing the source code of ExtUtils::Embed's ldopts, I think we
can also do this by subtracting $Config{ldflags}, since
my $linkage = "$ccdlflags $ldflags @archives $ld_or_bs";
and we really just want the $ld_or_bs part. (@archives should be empty
for our uses.)

+1, this looks like a nice clean solution. I see that it gets rid
of stuff we don't really want on RHEL8 as well as various generations
of macOS.

This would get rid of -mmacosx-version-min and -arch and all the things
you showed, including -L/opt/local/lib, which is probably there so that
the build of Perl itself could look there for things, but we don't need it.

It is a little weird that they are inserting -L/opt/local/lib or
-L/usr/local/lib on so many different platforms. But I concur
that if we need that, we likely should be inserting it ourselves
rather than absorbing it from their $ldflags.

BTW, I think the -arch business is dead code anyway now that we
desupported PPC-era macOS; I do not see any such switches from
modern macOS' perl. So not having a special case for that is an
additional win.

Patch LGTM; I noted only a trivial typo in the commit message:

-like we already do with $Config{ccdlflags}.  Those flags the choices
+like we already do with $Config{ccdlflags}.  Those flags are the choices

regards, tom lane

Hi,

On 2022-08-19 10:00:35 -0400, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

After analyzing the source code of ExtUtils::Embed's ldopts, I think we
can also do this by subtracting $Config{ldflags}, since
my $linkage = "$ccdlflags $ldflags @archives $ld_or_bs";
and we really just want the $ld_or_bs part. (@archives should be empty
for our uses.)

+1, this looks like a nice clean solution. I see that it gets rid
of stuff we don't really want on RHEL8 as well as various generations
of macOS.

Looks like it'd also get rid of the bogus
-bE:/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE/perl.exp we're
we're picking up on AIX (we had a thread about filtering that out, but I've only
done so inside the meson patch, round tuits).

So +1 from that front.

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically. Statically would be a different story, but we
already insist on a shared build.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically.

I'm certain that there are/were platforms that insist on those libraries
being mentioned anyway. Maybe they are all obsolete now?

regards, tom lane

Hi,

FWIW, looks like Peter's patch unbreaks building plperl on AIX using gcc and
system perl. Before we picked up a bunch of xlc specific flags that prevented
that.

before:
checking for flags to link embedded Perl... -brtl -bdynamic -b64 -L/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE -lperl -lpthread -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc
now:
checking for flags to link embedded Perl... -L/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE -lperl -lpthread -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc

On 2022-08-20 16:53:31 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically.

I'm certain that there are/were platforms that insist on those libraries
being mentioned anyway. Maybe they are all obsolete now?

I don't think any of the supported platforms require it for stuff used inside
the shared library (and we'd be in trouble if so, check e.g. libpq.pc). But of
course that's different if there's inline function / macros getting pulled in.

Which turns out to be an issue on AIX. All the -l flags added by perl can be
removed for xlc, but for gcc, -lpthreads (or -pthread) it is required.

Tried it on Solaris (32 bit, not sure if there's a 64bit perl available),
works.

Greetings,

Andres Freund

On 20.08.22 22:44, Andres Freund wrote:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically. Statically would be a different story, but we
already insist on a shared build.

Looking inside the ExtUtils::Embed source code, I wonder if there are
some installations that have things like -lperl538 or something like
that that it wants to deal with.

On 20.08.22 23:44, Andres Freund wrote:

On 2022-08-20 16:53:31 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically.

I'm certain that there are/were platforms that insist on those libraries
being mentioned anyway. Maybe they are all obsolete now?

I don't think any of the supported platforms require it for stuff used inside
the shared library (and we'd be in trouble if so, check e.g. libpq.pc). But of
course that's different if there's inline function / macros getting pulled in.

Which turns out to be an issue on AIX. All the -l flags added by perl can be
removed for xlc, but for gcc, -lpthreads (or -pthread) it is required.

Tried it on Solaris (32 bit, not sure if there's a 64bit perl available),
works.

Does that mean my proposed patch (v2) is adequate for these platforms,
or does it need further analysis?

Hi,

On 2022-08-22 16:32:36 +0200, Peter Eisentraut wrote:

On 20.08.22 23:44, Andres Freund wrote:

On 2022-08-20 16:53:31 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically.

I'm certain that there are/were platforms that insist on those libraries
being mentioned anyway. Maybe they are all obsolete now?

I don't think any of the supported platforms require it for stuff used inside
the shared library (and we'd be in trouble if so, check e.g. libpq.pc). But of
course that's different if there's inline function / macros getting pulled in.

Which turns out to be an issue on AIX. All the -l flags added by perl can be
removed for xlc, but for gcc, -lpthreads (or -pthread) it is required.

Tried it on Solaris (32 bit, not sure if there's a 64bit perl available),
works.

Does that mean my proposed patch (v2) is adequate for these platforms, or
does it need further analysis?

I think it's a clear improvement over the status quo. Unnecessary -l's are
pretty harmless compared to random other flags.

Greetings,

Andres Freund

Hi,

On 2022-08-22 16:31:53 +0200, Peter Eisentraut wrote:

On 20.08.22 22:44, Andres Freund wrote:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically. Statically would be a different story, but we
already insist on a shared build.

Looking inside the ExtUtils::Embed source code, I wonder if there are some
installations that have things like -lperl538 or something like that that it
wants to deal with.

There definitely are - I wasn't trying to suggest we'd add -lperl ourselves,
just that we'd try to only add -lperl* based on some Config variable.

We have plenty fragile windows specific logic that would be nice to get rid
of. But there's more exciting things to wrangle, I have to admit.

Greetings,

Andres Freund

Hi,

On 2022-08-22 08:37:40 -0700, Andres Freund wrote:

On 2022-08-22 16:32:36 +0200, Peter Eisentraut wrote:

On 20.08.22 23:44, Andres Freund wrote:

On 2022-08-20 16:53:31 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

Maybe a daft question: Why do want any of the -l flags other than -lperl? With
the patch configure spits out the following on my debian system:

checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl... -L/usr/lib/x86_64-linux-gnu/perl/5.34/CORE -lperl -ldl -lm -lpthread -lc -lcrypt

those libraries were likely relevant to build libperl, but don't look relevant
for linking to it dynamically.

I'm certain that there are/were platforms that insist on those libraries
being mentioned anyway. Maybe they are all obsolete now?

I don't think any of the supported platforms require it for stuff used inside
the shared library (and we'd be in trouble if so, check e.g. libpq.pc). But of
course that's different if there's inline function / macros getting pulled in.

Which turns out to be an issue on AIX. All the -l flags added by perl can be
removed for xlc, but for gcc, -lpthreads (or -pthread) it is required.

Tried it on Solaris (32 bit, not sure if there's a 64bit perl available),
works.

Does that mean my proposed patch (v2) is adequate for these platforms, or
does it need further analysis?

I think it's a clear improvement over the status quo. Unnecessary -l's are
pretty harmless compared to random other flags.

FWIW, while trying to mirror the same logic in meson I learned that the new
logic removes the rpath setting from the parameters on at least netbsd and
suse. We'll add them back - unless --disable-rpath is used. I think some
distributions build with --disable-rpath. Not sure if worth worrying about.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

FWIW, while trying to mirror the same logic in meson I learned that the new
logic removes the rpath setting from the parameters on at least netbsd and
suse. We'll add them back - unless --disable-rpath is used.

Hmm ... on my shiny new netbsd buildfarm animal, I see:

$ perl -MConfig -e 'print "$Config{ccdlflags}"'
-Wl,-E -Wl,-R/usr/pkg/lib/perl5/5.34.0/powerpc-netbsd-thread-multi/CORE

$ perl -MConfig -e 'print "$Config{ldflags}"'
-pthread -L/usr/lib -Wl,-R/usr/lib -Wl,-R/usr/pkg/lib -L/usr/pkg/lib

$ perl -MExtUtils::Embed -e ldopts
-Wl,-E -Wl,-R/usr/pkg/lib/perl5/5.34.0/powerpc-netbsd-thread-multi/CORE -pthread -L/usr/lib -Wl,-R/usr/lib -Wl,-R/usr/pkg/lib -L/usr/pkg/lib -L/usr/pkg/lib/perl5/5.34.0/powerpc-netbsd-thread-multi/CORE -lperl -lm -lcrypt -lpthread

So we were *already* stripping the rpath for where libperl.so is,
and now we also strip -L and rpath for /usr/lib (which surely is
pointless) and for /usr/pkg/lib (which in point of fact have to
be added to the PG configuration options anyway). These Perl
options seem a bit inconsistent ...

I think some
distributions build with --disable-rpath. Not sure if worth worrying about.

I believe that policy only makes sense for distros that expect every
shared library to appear in /usr/lib (or /usr/lib64 perhaps). Red Hat
for one does that --- but they follow through: libperl.so is there.

I think we're good here, at least till somebody points out a platform
where we aren't.

regards, tom lane

On 19.08.22 09:12, Peter Eisentraut wrote:

After analyzing the source code of ExtUtils::Embed's ldopts, I think we
can also do this by subtracting $Config{ldflags}, since

my $linkage = "$ccdlflags $ldflags @archives $ld_or_bs";

and we really just want the $ld_or_bs part. (@archives should be empty
for our uses.)

This would get rid of -mmacosx-version-min and -arch and all the things
you showed, including -L/opt/local/lib, which is probably there so that
the build of Perl itself could look there for things, but we don't need it.

This patch has failed on Cygwin lorikeet:

Before:

checking for flags to link embedded Perl...
-Wl,--enable-auto-import -Wl,--export-all-symbols
-Wl,--enable-auto-image-base -fstack-protector-strong
-L/usr/lib/perl5/5.32/x86_64-cygwin-threads/CORE -lperl -lpthread -ldl
-lcrypt

After:

checking for flags to link embedded Perl...
-L/usr/lib/perl5/5.32/x86_64-cygwin-threads/CORE -lperl -lpthread -ldl
-lcrypt

That's as designed. But the plperl tests fail:

CREATE EXTENSION plperl;
+ERROR:  incompatible library 
"/home/andrew/bf/root/HEAD/inst/lib/postgresql/plperl.dll": missing 
magic block
+HINT:  Extension libraries are required to use the PG_MODULE_MAGIC macro.

Among the now-dropped options, we can discount -Wl,--enable-auto-import,
because that is used anyway via src/template/cygwin.

So one of the options

-Wl,--export-all-symbols
-Wl,--enable-auto-image-base
-fstack-protector-strong

is needed. These options aren't used for any other shared libraries
AFAICT, so nothing is clear to me.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

This patch has failed on Cygwin lorikeet:
CREATE EXTENSION plperl;
+ERROR: incompatible library
"/home/andrew/bf/root/HEAD/inst/lib/postgresql/plperl.dll": missing
magic block

Presumably this is caused by not having

-Wl,--export-all-symbols

which is something we ought to be injecting for ourselves if we
aren't doing anything to export the magic-block constant explicitly.
But I too am confused why we haven't seen this elsewhere.

regards, tom lane

On 2022-08-24 We 09:30, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

This patch has failed on Cygwin lorikeet:
CREATE EXTENSION plperl;
+ERROR: incompatible library
"/home/andrew/bf/root/HEAD/inst/lib/postgresql/plperl.dll": missing
magic block

Presumably this is caused by not having

-Wl,--export-all-symbols

which is something we ought to be injecting for ourselves if we
aren't doing anything to export the magic-block constant explicitly.
But I too am confused why we haven't seen this elsewhere.

Me too. I note that we have -Wl,--out-implib=libplperl.a but we don't
appear to do anything with libplperl.a.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-24 We 09:30, Tom Lane wrote:

Presumably this is caused by not having

-Wl,--export-all-symbols

which is something we ought to be injecting for ourselves if we
aren't doing anything to export the magic-block constant explicitly.
But I too am confused why we haven't seen this elsewhere.

Me too. I note that we have -Wl,--out-implib=libplperl.a but we don't
appear to do anything with libplperl.a.

I've poked around and formed a vague theory, based on noting this
from the ld(1) man page:

--export-all-symbols
... When symbols are
explicitly exported via DEF files or implicitly exported via
function attributes, the default is to not export anything else
unless this option is given.

So we could explain the behavior if, say, plperl's _PG_init were
explicitly marked with __attribute__((visibility("default"))) while
its Pg_magic_func was not. That would work anyway as long as
--export-all-symbols was being used at link time, and would produce
the observed symptom as soon as it wasn't.

Now, seeing that both of those functions are surely marked with
PGDLLEXPORT in the source code, how could such a state of affairs
arise? What I'm thinking about is that _PG_init's marking will be
determined by the extern declaration for it in fmgr.h, while
Pg_magic_func's marking will be determined by the extern declaration
obtained from expanding PG_MODULE_MAGIC. And there are a boatload
of Perl-specific header files read between those points in plperl.c.

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

I can't readily check this, since I have no idea exactly which version
of the Perl headers lorikeet uses.

regards, tom lane

On 2022-08-24 We 18:56, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-24 We 09:30, Tom Lane wrote:

Presumably this is caused by not having

-Wl,--export-all-symbols

which is something we ought to be injecting for ourselves if we
aren't doing anything to export the magic-block constant explicitly.
But I too am confused why we haven't seen this elsewhere.

Me too. I note that we have -Wl,--out-implib=libplperl.a but we don't
appear to do anything with libplperl.a.

I've poked around and formed a vague theory, based on noting this
from the ld(1) man page:

--export-all-symbols
... When symbols are
explicitly exported via DEF files or implicitly exported via
function attributes, the default is to not export anything else
unless this option is given.

So we could explain the behavior if, say, plperl's _PG_init were
explicitly marked with __attribute__((visibility("default"))) while
its Pg_magic_func was not. That would work anyway as long as
--export-all-symbols was being used at link time, and would produce
the observed symptom as soon as it wasn't.

Now, seeing that both of those functions are surely marked with
PGDLLEXPORT in the source code, how could such a state of affairs
arise? What I'm thinking about is that _PG_init's marking will be
determined by the extern declaration for it in fmgr.h, while
Pg_magic_func's marking will be determined by the extern declaration
obtained from expanding PG_MODULE_MAGIC. And there are a boatload
of Perl-specific header files read between those points in plperl.c.

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

I can't readily check this, since I have no idea exactly which version
of the Perl headers lorikeet uses.

It's built against cygwin perl 5.32.

I don't see anything like that in perl.h. It's certainly using
__attribute__() a lot.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 25.08.22 02:14, Andrew Dunstan wrote:

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

I can't readily check this, since I have no idea exactly which version
of the Perl headers lorikeet uses.

It's built against cygwin perl 5.32.

I don't see anything like that in perl.h. It's certainly using
__attribute__() a lot.

This could be checked by running plperl.c through the preprocessor
(replace gcc -c plperl.c -o plperl.o by gcc -E plperl.c -o plperl.i) and
seeing what becomes of those symbols.

If we want to get the buildfarm green again sooner, we could force a
--export-all-symbols directly.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

This could be checked by running plperl.c through the preprocessor
(replace gcc -c plperl.c -o plperl.o by gcc -E plperl.c -o plperl.i) and
seeing what becomes of those symbols.

Yeah, that was what I was going to suggest: grep the "-E" output for
_PG_init and Pg_magic_func and confirm what their extern declarations
look like.

If we want to get the buildfarm green again sooner, we could force a
--export-all-symbols directly.

I'm not hugely upset as long as it's just the one machine failing.

regards, tom lane

On 2022-08-25 Th 09:43, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

This could be checked by running plperl.c through the preprocessor
(replace gcc -c plperl.c -o plperl.o by gcc -E plperl.c -o plperl.i) and
seeing what becomes of those symbols.

Yeah, that was what I was going to suggest: grep the "-E" output for
_PG_init and Pg_magic_func and confirm what their extern declarations
look like.

$ egrep '_PG_init|Pg_magic_func'  plperl.i
extern __attribute__((visibility("default"))) void _PG_init(void);
extern __attribute__((visibility("default"))) const Pg_magic_struct
*Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) {
static const Pg_magic_struct Pg_magic_data = { sizeof(Pg_magic_struct),
160000 / 100, 100, 32, 64,
_PG_init(void)

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-08-25 17:39:35 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 09:43, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

This could be checked by running plperl.c through the preprocessor
(replace gcc -c plperl.c -o plperl.o by gcc -E plperl.c -o plperl.i) and
seeing what becomes of those symbols.

Yeah, that was what I was going to suggest: grep the "-E" output for
_PG_init and Pg_magic_func and confirm what their extern declarations
look like.

$ egrep '_PG_init|Pg_magic_func'  plperl.i
extern __attribute__((visibility("default"))) void _PG_init(void);
extern __attribute__((visibility("default"))) const Pg_magic_struct
*Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) {
static const Pg_magic_struct Pg_magic_data = { sizeof(Pg_magic_struct),
160000 / 100, 100, 32, 64,
_PG_init(void)

Could you show objdump -t of the library? Perhaps once with the flags as now,
and once relinking with the "old" flags that we're now omitting?

Greetings,

Andres Freund

On 2022-08-25 Th 17:47, Andres Freund wrote:

Hi,

On 2022-08-25 17:39:35 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 09:43, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

In short: if the Cygwin Perl headers redefine PGDLLEXPORT (unlikely)
or somehow #define "__attribute__()" or "visibility()" into no-ops
(perhaps more likely) then we could explain this failure, and that
would also explain why it doesn't fail elsewhere.

This could be checked by running plperl.c through the preprocessor
(replace gcc -c plperl.c -o plperl.o by gcc -E plperl.c -o plperl.i) and
seeing what becomes of those symbols.

Yeah, that was what I was going to suggest: grep the "-E" output for
_PG_init and Pg_magic_func and confirm what their extern declarations
look like.

$ egrep '_PG_init|Pg_magic_func'  plperl.i
extern __attribute__((visibility("default"))) void _PG_init(void);
extern __attribute__((visibility("default"))) const Pg_magic_struct
*Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) {
static const Pg_magic_struct Pg_magic_data = { sizeof(Pg_magic_struct),
160000 / 100, 100, 32, 64,
_PG_init(void)

Could you show objdump -t of the library? Perhaps once with the flags as now,
and once relinking with the "old" flags that we're now omitting?

current:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[103]: (sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040e0 _PG_init
Pg_magic_func
[105]: (sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040b0 _PG_init

from July 11th build:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[101]: (sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040d0 Pg_magic_func
Pg_magic_func
[103]: (sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040e0 _PG_init

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-08-25 18:04:34 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 17:47, Andres Freund wrote:

$ egrep '_PG_init|Pg_magic_func'  plperl.i
extern __attribute__((visibility("default"))) void _PG_init(void);
extern __attribute__((visibility("default"))) const Pg_magic_struct
*Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) {
static const Pg_magic_struct Pg_magic_data = { sizeof(Pg_magic_struct),
160000 / 100, 100, 32, 64,
_PG_init(void)

Could you show objdump -t of the library? Perhaps once with the flags as now,
and once relinking with the "old" flags that we're now omitting?

current:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[103](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040a0
Pg_magic_func
[105](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040b0 _PG_init

from July 11th build:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[101](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040d0
Pg_magic_func
[103](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040e0 _PG_init

Thanks.

So it looks like it's not the symbol not being exported. I wonder if the image
base thing is somehow the problem? Sounds like it should just be an efficiency
difference, by avoiding some relocations, not a functional difference.

Can you try adding just that to the flags for building and whether that then
allows a LOAD 'plperl' to succeed?

Greetings,

Andres Freund

On 2022-08-25 Th 18:13, Andres Freund wrote:

Hi,

On 2022-08-25 18:04:34 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 17:47, Andres Freund wrote:

$ egrep '_PG_init|Pg_magic_func'  plperl.i
extern __attribute__((visibility("default"))) void _PG_init(void);
extern __attribute__((visibility("default"))) const Pg_magic_struct
*Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) {
static const Pg_magic_struct Pg_magic_data = { sizeof(Pg_magic_struct),
160000 / 100, 100, 32, 64,
_PG_init(void)

Could you show objdump -t of the library? Perhaps once with the flags as now,
and once relinking with the "old" flags that we're now omitting?

current:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[103](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040a0
Pg_magic_func
[105](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040b0 _PG_init

from July 11th build:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[101](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040d0
Pg_magic_func
[103](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040e0 _PG_init

Thanks.

So it looks like it's not the symbol not being exported. I wonder if the image
base thing is somehow the problem? Sounds like it should just be an efficiency
difference, by avoiding some relocations, not a functional difference.

Can you try adding just that to the flags for building and whether that then
allows a LOAD 'plperl' to succeed?

Adding what?

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-08-26 10:04:35 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 18:13, Andres Freund wrote:

Could you show objdump -t of the library? Perhaps once with the flags as now,
and once relinking with the "old" flags that we're now omitting?

current:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[103](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040a0
Pg_magic_func
[105](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040b0 _PG_init

from July 11th build:

$ objdump -t plperl.dll | egrep '_PG_init|Pg_magic_func'
[101](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040d0
Pg_magic_func
[103](sec  1)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000000040e0 _PG_init

Thanks.

So it looks like it's not the symbol not being exported. I wonder if the image
base thing is somehow the problem? Sounds like it should just be an efficiency
difference, by avoiding some relocations, not a functional difference.

Can you try adding just that to the flags for building and whether that then
allows a LOAD 'plperl' to succeed?

Adding what?

-Wl,--enable-auto-image-base

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2022-08-26 10:04:35 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 18:13, Andres Freund wrote:

Can you try adding just that to the flags for building and whether that then
allows a LOAD 'plperl' to succeed?

Adding what?

-Wl,--enable-auto-image-base

And if that doesn't help, try -Wl,--export-all-symbols

regards, tom lane

On 2022-08-26 Fr 12:11, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-08-26 10:04:35 -0400, Andrew Dunstan wrote:

On 2022-08-25 Th 18:13, Andres Freund wrote:

Can you try adding just that to the flags for building and whether that then
allows a LOAD 'plperl' to succeed?

Adding what?

-Wl,--enable-auto-image-base

didn't work

And if that doesn't help, try -Wl,--export-all-symbols

worked

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-26 Fr 12:11, Tom Lane wrote:

And if that doesn't help, try -Wl,--export-all-symbols

worked

Hmph. Hard to see how that isn't a linker bug. As a stopgap
to get the farm green again, I propose adding something like

ifeq ($(PORTNAME), cygwin)
SHLIB_LINK += -Wl,--export-all-symbols
endif

to plperl's makefile.

regards, tom lane

On 2022-08-26 Fr 16:00, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-26 Fr 12:11, Tom Lane wrote:

And if that doesn't help, try -Wl,--export-all-symbols

worked

Hmph. Hard to see how that isn't a linker bug. As a stopgap
to get the farm green again, I propose adding something like

ifeq ($(PORTNAME), cygwin)
SHLIB_LINK += -Wl,--export-all-symbols
endif

to plperl's makefile.

+1

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-08-26 16:00:31 -0400, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-26 Fr 12:11, Tom Lane wrote:

And if that doesn't help, try -Wl,--export-all-symbols

worked

Except that it's only happening for plperl, I'd wonder if it's possibly
related to our magic symbols being prefixed with _. I noticed that the
underscore prefix e.g. changes the behaviour of gcc's "collect2" on AIX, which
is responsible for exporting symbols etc.

Hmph. Hard to see how that isn't a linker bug.

Agreed, given that this is only happening with plperl, and not with any of the
other extensions...

As a stopgap to get the farm green again, I propose adding something like

ifeq ($(PORTNAME), cygwin)
SHLIB_LINK += -Wl,--export-all-symbols
endif

to plperl's makefile.

:(

Greetings,

Andres Freund

On 2022-08-26 Fr 16:25, Andres Freund wrote:

Hi,

On 2022-08-26 16:00:31 -0400, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-26 Fr 12:11, Tom Lane wrote:

And if that doesn't help, try -Wl,--export-all-symbols

worked

Except that it's only happening for plperl, I'd wonder if it's possibly
related to our magic symbols being prefixed with _. I noticed that the
underscore prefix e.g. changes the behaviour of gcc's "collect2" on AIX, which
is responsible for exporting symbols etc.

Hmph. Hard to see how that isn't a linker bug.

Agreed, given that this is only happening with plperl, and not with any of the
other extensions...

As a stopgap to get the farm green again, I propose adding something like

ifeq ($(PORTNAME), cygwin)
SHLIB_LINK += -Wl,--export-all-symbols
endif

to plperl's makefile.

:(

It doesn't make me very happy either, but nobody seems to have a better
idea.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-08-30 09:35:51 -0400, Andrew Dunstan wrote:

On 2022-08-26 Fr 16:25, Andres Freund wrote:

On 2022-08-26 16:00:31 -0400, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2022-08-26 Fr 12:11, Tom Lane wrote:

And if that doesn't help, try -Wl,--export-all-symbols

worked

Except that it's only happening for plperl, I'd wonder if it's possibly
related to our magic symbols being prefixed with _. I noticed that the
underscore prefix e.g. changes the behaviour of gcc's "collect2" on AIX, which
is responsible for exporting symbols etc.

Hmph. Hard to see how that isn't a linker bug.

Agreed, given that this is only happening with plperl, and not with any of the
other extensions...

As a stopgap to get the farm green again, I propose adding something like

ifeq ($(PORTNAME), cygwin)
SHLIB_LINK += -Wl,--export-all-symbols
endif

to plperl's makefile.

:(

It doesn't make me very happy either, but nobody seems to have a better
idea.

The plpython issue I was investigating in
/messages/by-id/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de
feels eerily similar to the issue here.

I wonder if it's the same problem - __attribute__((visibility("default")))
works to export - unless another symbol uses __declspec (dllexport). In the
referenced thread that was PyInit_plpy(), here it could be some perl generated
one.

Does this issue resolved if you add
#define PGDLLEXPORT __declspec (dllexport)
to cygwin.h? Without the -Wl,--export-all-symbols of course.

Greetings,

Andres Freund