pgsql: Default to hidden visibility for extension libraries where possi
Default to hidden visibility for extension libraries where possible
Until now postgres built extension libraries with global visibility, i.e.
exporting all symbols. On the one platform where that behavior is not
natively available, namely windows, we emulate it by analyzing the input files
to the shared library and exporting all the symbols therein.
Not exporting all symbols is actually desirable, as it can improve loading
speed, reduces the likelihood of symbol conflicts and can improve intra
extension library function call performance. It also makes the non-windows
builds more similar to windows builds.
Additionally, with meson implementing the export-all-symbols behavior for
windows, turns out to be more verbose than desirable.
This patch adds support for hiding symbols by default and, to counteract that,
explicit symbol visibility annotation for compilers that support
__attribute__((visibility("default"))) and -fvisibility=hidden. That is
expected to be most, if not all, compilers except msvc (for which we already
support explicit symbol export annotations).
Now that extension library symbols are explicitly exported, we don't need to
export all symbols on windows anymore, hence remove that behavior from
src/tools/msvc. The supporting code can't be removed, as we still need to
export all symbols from the main postgres binary.
Author: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: /messages/by-id/20211101020311.av6hphdl6xbjbuif@alap3.anarazel.de
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/089480c077056fc20fa8d8f5a3032a9dcf5ed812
Modified Files
--------------
configure | 152 +++++++++++++++++++++++++++++++++++++++++++++
configure.ac | 13 ++++
src/Makefile.global.in | 3 +
src/Makefile.shlib | 13 ++++
src/include/c.h | 13 ++--
src/include/pg_config.h.in | 3 +
src/makefiles/pgxs.mk | 5 +-
src/tools/msvc/Project.pm | 7 ---
src/tools/msvc/Solution.pm | 1 +
9 files changed, 198 insertions(+), 12 deletions(-)
Hi,
On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
Default to hidden visibility for extension libraries where possible
Looking at the odd failures, not sure what went wrong.
Greetings,
Andres Freund
On 2022-07-17 18:39:35 -0700, Andres Freund wrote:
On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
Default to hidden visibility for extension libraries where possible
Looking at the odd failures, not sure what went wrong.
I don't know how the configure exec bit got removed, shell history doesn't
show anything odd in that regard. I do see a mistake in locally merging the
symbol-visibility branch (leading to a crucial commit being missed). I do see
in shell history that I ran check-world without a failure just before pushing,
which should have shown the failure, but didn't.
Definitely a brown paper bag moment.
Andres Freund <andres@anarazel.de> writes:
I don't know how the configure exec bit got removed, shell history doesn't
show anything odd in that regard. I do see a mistake in locally merging the
symbol-visibility branch (leading to a crucial commit being missed). I do see
in shell history that I ran check-world without a failure just before pushing,
which should have shown the failure, but didn't.
check-world would not have re-run configure even if it was out of date
(only config.status), so that part of it's not so surprising.
Definitely a brown paper bag moment.
We've all been there :-)
regards, tom lane
On Sun, Jul 17, 2022 at 07:01:55PM -0700, Andres Freund wrote:
On 2022-07-17 18:39:35 -0700, Andres Freund wrote:
On 2022-07-18 01:05:56 +0000, Andres Freund wrote:
Default to hidden visibility for extension libraries where possible
Looking at the odd failures, not sure what went wrong.
I don't know how the configure exec bit got removed, shell history doesn't
I think git reflog -p would help to answer that, but I suppose you already
looked.
--
Justin
Hi
po 18. 7. 2022 v 3:06 odesílatel Andres Freund <andres@anarazel.de> napsal:
Default to hidden visibility for extension libraries where possible
Until now postgres built extension libraries with global visibility, i.e.
exporting all symbols. On the one platform where that behavior is not
natively available, namely windows, we emulate it by analyzing the input
files
to the shared library and exporting all the symbols therein.Not exporting all symbols is actually desirable, as it can improve loading
speed, reduces the likelihood of symbol conflicts and can improve intra
extension library function call performance. It also makes the non-windows
builds more similar to windows builds.Additionally, with meson implementing the export-all-symbols behavior for
windows, turns out to be more verbose than desirable.This patch adds support for hiding symbols by default and, to counteract
that,
explicit symbol visibility annotation for compilers that support
__attribute__((visibility("default"))) and -fvisibility=hidden. That is
expected to be most, if not all, compilers except msvc (for which we
already
support explicit symbol export annotations).Now that extension library symbols are explicitly exported, we don't need
to
export all symbols on windows anymore, hence remove that behavior from
src/tools/msvc. The supporting code can't be removed, as we still need to
export all symbols from the main postgres binary.Author: Andres Freund <andres@anarazel.de>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion:
/messages/by-id/20211101020311.av6hphdl6xbjbuif@alap3.anarazel.de
Unfortunately, this commit definitely breaks plpgsql_check. Can the
following routines be visible?
AssertVariableIsOfType(&plpgsql_build_datatype,
plpgsql_check__build_datatype_t);
plpgsql_check__build_datatype_p = (plpgsql_check__build_datatype_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_build_datatype");
AssertVariableIsOfType(&plpgsql_compile, plpgsql_check__compile_t);
plpgsql_check__compile_p = (plpgsql_check__compile_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_compile");
AssertVariableIsOfType(&plpgsql_parser_setup,
plpgsql_check__parser_setup_t);
plpgsql_check__parser_setup_p = (plpgsql_check__parser_setup_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_parser_setup");
AssertVariableIsOfType(&plpgsql_stmt_typename,
plpgsql_check__stmt_typename_t);
plpgsql_check__stmt_typename_p = (plpgsql_check__stmt_typename_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_stmt_typename");
AssertVariableIsOfType(&plpgsql_exec_get_datum_type,
plpgsql_check__exec_get_datum_type_t);
plpgsql_check__exec_get_datum_type_p =
(plpgsql_check__exec_get_datum_type_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql",
"plpgsql_exec_get_datum_type");
AssertVariableIsOfType(&plpgsql_recognize_err_condition,
plpgsql_check__recognize_err_condition_t);
plpgsql_check__recognize_err_condition_p =
(plpgsql_check__recognize_err_condition_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql",
"plpgsql_recognize_err_condition");
AssertVariableIsOfType(&plpgsql_ns_lookup, plpgsql_check__ns_lookup_t);
plpgsql_check__ns_lookup_p = (plpgsql_check__ns_lookup_t)
LOAD_EXTERNAL_FUNCTION("$libdir/plpgsql", "plpgsql_ns_lookup");
Regards
Pavel
Show quoted text
Branch
------
masterDetails
-------https://git.postgresql.org/pg/commitdiff/089480c077056fc20fa8d8f5a3032a9dcf5ed812
Modified Files -------------- configure | 152 +++++++++++++++++++++++++++++++++++++++++++++ configure.ac | 13 ++++ src/Makefile.global.in | 3 + src/Makefile.shlib | 13 ++++ src/include/c.h | 13 ++-- src/include/pg_config.h.in | 3 + src/makefiles/pgxs.mk | 5 +- src/tools/msvc/Project.pm | 7 --- src/tools/msvc/Solution.pm | 1 + 9 files changed, 198 insertions(+), 12 deletions(-)
On 2022-Jul-19, Pavel Stehule wrote:
po 18. 7. 2022 v 3:06 odesílatel Andres Freund <andres@anarazel.de> napsal:
Default to hidden visibility for extension libraries where possible
Unfortunately, this commit definitely breaks plpgsql_check. Can the
following routines be visible?
Do you just need to send a patch to add an exports.txt file to
src/pl/plpgsql/src/ for these functions?
plpgsql_build_datatype
plpgsql_compile
plpgsql_parser_setup
plpgsql_stmt_typename
plpgsql_exec_get_datum_type
plpgsql_recognize_err_condition
plpgsql_ns_lookup
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
On 2022-Jul-19, Pavel Stehule wrote:
Unfortunately, this commit definitely breaks plpgsql_check. Can the
following routines be visible?
Do you just need to send a patch to add an exports.txt file to
src/pl/plpgsql/src/ for these functions?
The precedent of plpython says that PGDLLEXPORT markers are sufficient.
But yeah, we need a list of exactly which functions need to be
re-exposed. I imagine pldebugger has its own needs.
regards, tom lane
[ Redirecting thread to -hackers from -committers ]
On 2022-Jul-19, Tom Lane wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Do you just need to send a patch to add an exports.txt file to
src/pl/plpgsql/src/ for these functions?The precedent of plpython says that PGDLLEXPORT markers are sufficient.
But yeah, we need a list of exactly which functions need to be
re-exposed. I imagine pldebugger has its own needs.
A reasonable guess. I went as far as downloading pldebugger and
compiling it, but it doesn't have a test suite of its own, so I couldn't
verify anything about it. I did notice that plpgsql_check is calling
function load_external_function(), and that doesn't appear in
pldebugger. I wonder if the find_rendezvous_variable business is at
play.
Anyway, the minimal patch that makes plpgsql_check tests pass is
attached. This seems a bit random. Maybe it'd be better to have a
plpgsql_internal.h with functions that are exported only for plpgsql
itself, and keep plpgsql.h with a set of functions, all marked
PGDLLEXPORT, that are for external use.
... oh, and:
$ postmaster -c shared_preload_libraries=plugin_debugger
2022-07-19 16:27:24.006 CEST [742142] FATAL: cannot request additional shared memory outside shmem_request_hook
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Attachments:
0001-add-PGDLLEXPORTS-to-plpgsql.patchtext/x-diff; charset=us-asciiDownload
From e0759245de3e0fcad7a6b2c3e9a637d3bdffe2a4 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 19 Jul 2022 16:19:03 +0200
Subject: [PATCH] add PGDLLEXPORTS to plpgsql
for plpgsql_check benefit
---
src/pl/plpgsql/src/plpgsql.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 1914160272..20dd5ec060 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1231,10 +1231,10 @@ extern PLpgSQL_plugin **plpgsql_plugin_ptr;
/*
* Functions in pl_comp.c
*/
-extern PLpgSQL_function *plpgsql_compile(FunctionCallInfo fcinfo,
+extern PGDLLEXPORT PLpgSQL_function *plpgsql_compile(FunctionCallInfo fcinfo,
bool forValidator);
extern PLpgSQL_function *plpgsql_compile_inline(char *proc_source);
-extern void plpgsql_parser_setup(struct ParseState *pstate,
+extern PGDLLEXPORT void plpgsql_parser_setup(struct ParseState *pstate,
PLpgSQL_expr *expr);
extern bool plpgsql_parse_word(char *word1, const char *yytxt, bool lookup,
PLwdatum *wdatum, PLword *word);
@@ -1246,7 +1246,7 @@ extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident);
extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents);
extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
-extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
+extern PGDLLEXPORT PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
Oid collation,
TypeName *origtypname);
extern PLpgSQL_variable *plpgsql_build_variable(const char *refname, int lineno,
@@ -1257,7 +1257,7 @@ extern PLpgSQL_rec *plpgsql_build_record(const char *refname, int lineno,
bool add2namespace);
extern PLpgSQL_recfield *plpgsql_build_recfield(PLpgSQL_rec *rec,
const char *fldname);
-extern int plpgsql_recognize_err_condition(const char *condname,
+extern PGDLLEXPORT int plpgsql_recognize_err_condition(const char *condname,
bool allow_sqlstate);
extern PLpgSQL_condition *plpgsql_parse_err_condition(char *condname);
extern void plpgsql_adddatum(PLpgSQL_datum *newdatum);
@@ -1280,7 +1280,7 @@ extern void plpgsql_exec_event_trigger(PLpgSQL_function *func,
extern void plpgsql_xact_cb(XactEvent event, void *arg);
extern void plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
SubTransactionId parentSubid, void *arg);
-extern Oid plpgsql_exec_get_datum_type(PLpgSQL_execstate *estate,
+extern PGDLLEXPORT Oid plpgsql_exec_get_datum_type(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum);
extern void plpgsql_exec_get_datum_type_info(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum,
@@ -1296,7 +1296,7 @@ extern void plpgsql_ns_push(const char *label,
extern void plpgsql_ns_pop(void);
extern PLpgSQL_nsitem *plpgsql_ns_top(void);
extern void plpgsql_ns_additem(PLpgSQL_nsitem_type itemtype, int itemno, const char *name);
-extern PLpgSQL_nsitem *plpgsql_ns_lookup(PLpgSQL_nsitem *ns_cur, bool localmode,
+extern PGDLLEXPORT PLpgSQL_nsitem *plpgsql_ns_lookup(PLpgSQL_nsitem *ns_cur, bool localmode,
const char *name1, const char *name2,
const char *name3, int *names_used);
extern PLpgSQL_nsitem *plpgsql_ns_lookup_label(PLpgSQL_nsitem *ns_cur,
@@ -1306,7 +1306,7 @@ extern PLpgSQL_nsitem *plpgsql_ns_find_nearest_loop(PLpgSQL_nsitem *ns_cur);
/*
* Other functions in pl_funcs.c
*/
-extern const char *plpgsql_stmt_typename(PLpgSQL_stmt *stmt);
+extern PGDLLEXPORT const char *plpgsql_stmt_typename(PLpgSQL_stmt *stmt);
extern const char *plpgsql_getdiag_kindname(PLpgSQL_getdiag_kind kind);
extern void plpgsql_free_function_memory(PLpgSQL_function *func);
extern void plpgsql_dumptree(PLpgSQL_function *func);
--
2.30.2
On Tue, Jul 19, 2022 at 04:28:07PM +0200, Alvaro Herrera wrote:
... oh, and:
$ postmaster -c shared_preload_libraries=plugin_debugger
2022-07-19 16:27:24.006 CEST [742142] FATAL: cannot request additional shared memory outside shmem_request_hook
This has been reported multiple times (including on one of my own projects),
see
/messages/by-id/81f82c00-8818-91f3-96fa-47976f94662b@pm.me
for the last report.
Hi
út 19. 7. 2022 v 16:28 odesílatel Alvaro Herrera <alvherre@alvh.no-ip.org>
napsal:
[ Redirecting thread to -hackers from -committers ]
On 2022-Jul-19, Tom Lane wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Do you just need to send a patch to add an exports.txt file to
src/pl/plpgsql/src/ for these functions?The precedent of plpython says that PGDLLEXPORT markers are sufficient.
But yeah, we need a list of exactly which functions need to be
re-exposed. I imagine pldebugger has its own needs.A reasonable guess. I went as far as downloading pldebugger and
compiling it, but it doesn't have a test suite of its own, so I couldn't
verify anything about it. I did notice that plpgsql_check is calling
function load_external_function(), and that doesn't appear in
pldebugger. I wonder if the find_rendezvous_variable business is at
play.Anyway, the minimal patch that makes plpgsql_check tests pass is
attached. This seems a bit random. Maybe it'd be better to have a
plpgsql_internal.h with functions that are exported only for plpgsql
itself, and keep plpgsql.h with a set of functions, all marked
PGDLLEXPORT, that are for external use.
I can confirm that the attached patch fixes plpgsql_check.
Thank you
Pavel
Show quoted text
... oh, and:
$ postmaster -c shared_preload_libraries=plugin_debugger
2022-07-19 16:27:24.006 CEST [742142] FATAL: cannot request additional
shared memory outside shmem_request_hook--
Álvaro Herrera Breisgau, Deutschland —
https://www.EnterpriseDB.com/
Hi,
On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
Anyway, the minimal patch that makes plpgsql_check tests pass is
attached. This seems a bit random. Maybe it'd be better to have a
plpgsql_internal.h with functions that are exported only for plpgsql
itself, and keep plpgsql.h with a set of functions, all marked
PGDLLEXPORT, that are for external use.
It does seem a bit random. But I think we probably should err on the side of
adding more declarations, rather than the oposite.
I like the plpgsql_internal.h idea, but probably done separately?
Greetings,
Andres Freund
Hi
út 19. 7. 2022 v 17:31 odesílatel Andres Freund <andres@anarazel.de> napsal:
Hi,
On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
Anyway, the minimal patch that makes plpgsql_check tests pass is
attached. This seems a bit random. Maybe it'd be better to have a
plpgsql_internal.h with functions that are exported only for plpgsql
itself, and keep plpgsql.h with a set of functions, all marked
PGDLLEXPORT, that are for external use.It does seem a bit random. But I think we probably should err on the side
of
adding more declarations, rather than the oposite.
This list can be extended. I think plpgsql_check is maybe one extension
that uses code from another extension directly. This is really not common
usage.
I like the plpgsql_internal.h idea, but probably done separately?
can be
I have not any problem with it or with exports.txt file.
Show quoted text
Greetings,
Andres Freund
Hi,
On 2022-07-19 17:37:11 +0200, Pavel Stehule wrote:
�t 19. 7. 2022 v 17:31 odes�latel Andres Freund <andres@anarazel.de> napsal:
Hi,
On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
Anyway, the minimal patch that makes plpgsql_check tests pass is
attached. This seems a bit random. Maybe it'd be better to have a
plpgsql_internal.h with functions that are exported only for plpgsql
itself, and keep plpgsql.h with a set of functions, all marked
PGDLLEXPORT, that are for external use.It does seem a bit random. But I think we probably should err on the side
of
adding more declarations, rather than the oposite.This list can be extended. I think plpgsql_check is maybe one extension
that uses code from another extension directly. This is really not common
usage.
There's a few more use cases, e.g. transform modules. Hence exposing e.g. many
plpython helpers.
I have not any problem with it or with exports.txt file.
Just to be clear, there shouldn't be any use exports.txt here, just a few
PGDLLEXPORTs.
Greetings,
Andres Freund
Hi,
On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
Anyway, the minimal patch that makes plpgsql_check tests pass is
attached.
Do you want to commit that or should I?
Greetings,
Andres Freund
Hi,
On 2022-07-19 08:31:49 -0700, Andres Freund wrote:
But I think we probably should err on the side of adding more
declarations, rather than the oposite.
To see if there's other declarations that should be added, I used
https://codesearch.debian.net/search?q=load_external_function&literal=1&perpkg=1
which shows plpgsql_check and hstore_pllua. All the hstore symbols for
the latter are exported already.
Greetings,
Andres Freund
Hello
On 2022-Jul-19, Andres Freund wrote:
On 2022-07-19 16:28:07 +0200, Alvaro Herrera wrote:
Anyway, the minimal patch that makes plpgsql_check tests pass is
attached.Do you want to commit that or should I?
Done.
No immediate plans for splitting plpgsql.h, so if anyone wants to take a
stab at that, be my guest.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Aprender sin pensar es inútil; pensar sin aprender, peligroso" (Confucio)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
No immediate plans for splitting plpgsql.h, so if anyone wants to take a
stab at that, be my guest.
ISTM that a comment pointing out that the functions marked PGDLLEXPORT
are meant to be externally accessible should be sufficient.
I'll try to do some research later today to identify anything else
we need to mark in plpgsql. I recall doing some work specifically
creating functions for pldebugger's use, but I'll need to dig.
regards, tom lane
On Wed, Jul 20, 2022 at 9:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
ISTM that a comment pointing out that the functions marked PGDLLEXPORT
are meant to be externally accessible should be sufficient.
The name PGDLLEXPORT is actually slightly misleading, now, because
there's no longer anything about it that is specific to DLLs.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 2022-Jul-20, Tom Lane wrote:
I'll try to do some research later today to identify anything else
we need to mark in plpgsql. I recall doing some work specifically
creating functions for pldebugger's use, but I'll need to dig.
I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
expose functions directly, but through plpgsql_plugin_ptr. Maybe that
one does need to be made PGDLLEXPORT, since currently it isn't.
That was also reported by Pavel. He was concerned about plpgsql_check,
though, not pldebugger.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree. (Don Knuth)
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jul 20, 2022 at 9:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
ISTM that a comment pointing out that the functions marked PGDLLEXPORT
are meant to be externally accessible should be sufficient.
The name PGDLLEXPORT is actually slightly misleading, now, because
there's no longer anything about it that is specific to DLLs.
True, but the mess of changing it seems to outweigh any likely clarity
gain. As long as there's adequate commentary about what it means,
I'm okay with the existing naming.
regards, tom lane
Hi,
On July 20, 2022 3:54:03 PM GMT+02:00, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jul 20, 2022 at 9:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
ISTM that a comment pointing out that the functions marked PGDLLEXPORT
are meant to be externally accessible should be sufficient.The name PGDLLEXPORT is actually slightly misleading, now, because
there's no longer anything about it that is specific to DLLs.
How so? Right now it's solely used for making symbols in DLLs as exported?
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> writes:
On July 20, 2022 3:54:03 PM GMT+02:00, Robert Haas <robertmhaas@gmail.com> wrote:
The name PGDLLEXPORT is actually slightly misleading, now, because
there's no longer anything about it that is specific to DLLs.
How so? Right now it's solely used for making symbols in DLLs as exported?
I suspect Robert is reading "DLL" as meaning only a Windows thing.
You're right, if you read it as a generic term for loadable libraries,
it's more or less applicable everywhere.
regards, tom lane
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
expose functions directly, but through plpgsql_plugin_ptr. Maybe that
one does need to be made PGDLLEXPORT, since currently it isn't.
Hm. Not sure if the rules are the same for global variables as
they are for functions, but if so, yeah ...
regards, tom lane
Hi,
On July 20, 2022 4:20:04 PM GMT+02:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
expose functions directly, but through plpgsql_plugin_ptr. Maybe that
one does need to be made PGDLLEXPORT, since currently it isn't.Hm. Not sure if the rules are the same for global variables as
they are for functions, but if so, yeah ...
They're the same on the export side. On windows the rules for linking to variables are stricter (they need declspec dllimport), but that doesn't matter for dlsym style stuff.
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
On 2022-Jul-20, Tom Lane wrote:
I'll try to do some research later today to identify anything else
we need to mark in plpgsql. I recall doing some work specifically
creating functions for pldebugger's use, but I'll need to dig.
I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
expose functions directly, but through plpgsql_plugin_ptr. Maybe that
one does need to be made PGDLLEXPORT, since currently it isn't.
After some experimentation, it does not need to be marked: pldebugger
gets at that via find_rendezvous_variable(), so there is no need for
any explicit linkage at all between plpgsql.so and plugin_debugger.so.
Along the way, I made a quick hack to get pldebugger to load into
v15/HEAD. It lacks #ifdef's which'd be needed so that it'd still
compile against older branches, but perhaps this'll save someone
some time.
regards, tom lane
Attachments:
pldebugger-shmem.patchtext/x-diff; charset=us-ascii; name=pldebugger-shmem.patchDownload
diff --git a/plugin_debugger.c b/plugin_debugger.c
index 6620021..1bd2057 100644
--- a/plugin_debugger.c
+++ b/plugin_debugger.c
@@ -114,6 +114,8 @@ static debugger_language_t *debugger_languages[] = {
NULL
};
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
+
/**********************************************************************
* Function declarations
**********************************************************************/
@@ -124,6 +126,7 @@ void _PG_init( void ); /* initialize this module when we are dynamically load
* Local (hidden) function prototypes
**********************************************************************/
+static void pldebugger_shmem_request( void );
//static char ** fetchArgNames( PLpgSQL_function * func, int * nameCount );
static void * writen( int peer, void * src, size_t len );
static bool connectAsServer( void );
@@ -154,6 +157,15 @@ void _PG_init( void )
for (i = 0; debugger_languages[i] != NULL; i++)
debugger_languages[i]->initialize();
+ prev_shmem_request_hook = shmem_request_hook;
+ shmem_request_hook = pldebugger_shmem_request;
+}
+
+static void pldebugger_shmem_request( void )
+{
+ if (prev_shmem_request_hook)
+ prev_shmem_request_hook();
+
reserveBreakpoints();
dbgcomm_reserve();
}
On Wed, 20 Jul 2022 at 16:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
On 2022-Jul-20, Tom Lane wrote:
I'll try to do some research later today to identify anything else
we need to mark in plpgsql. I recall doing some work specifically
creating functions for pldebugger's use, but I'll need to dig.I suppose you're probably thinking of commit 53ef6c40f1e7; that didn't
expose functions directly, but through plpgsql_plugin_ptr. Maybe that
one does need to be made PGDLLEXPORT, since currently it isn't.After some experimentation, it does not need to be marked: pldebugger
gets at that via find_rendezvous_variable(), so there is no need for
any explicit linkage at all between plpgsql.so and plugin_debugger.so.Along the way, I made a quick hack to get pldebugger to load into
v15/HEAD. It lacks #ifdef's which'd be needed so that it'd still
compile against older branches, but perhaps this'll save someone
some time.
Thanks Tom - I've pushed that patch with the relevant #ifdefs added.
--
Dave Page
PostgreSQL Core Team
http://www.postgresql.org/