Fix some ubsan/asan related issues
Patch 1:
Passing NULL as a second argument to memcpy breaks ubsan, and there
didn't seem to be anything preventing that in the LogLogicalMessage()
codepath. Here is a preventative measure in LogLogicalMessage() and an
Assert() in CopyXLogRecordToWAL().
Patch 2:
Support building with -Db_sanitize=address in Meson. Various executables
are leaky which can cause the builds to fail and tests to fail, when we
are fine leaking this memory.
Personally, I am a big stickler for always freeing my memory in
executables even if it gets released at program exit because it makes
the leak sanitizer much more effective. However (!), I am not here to
change the philosophy of memory management in one-off executables, so
I have just silenced memory leaks in various executables for now.
Patch 3:
THIS DOESN'T WORK. DO NOT COMMIT. PROPOSING FOR DISCUSSION!
In my effort to try to see if the test suite would pass with asan
enabled, I ran into a max_stack_depth issue. I tried maxing it out
(hence, the patch), but that still didn't remedy my issue. I tried to
look on the list for any relevant emails, but nothing turned up. Maybe
others have not attempted this before? Seems doubtful.
Not entirely sure how to fix this issue. I personally find asan
extremely effective, even more than valgrind, so it would be great if
I could run Postgres with asan enabled to catch various stupid C issues
I might create. On my system, ulimit -a returns 8192 kbytes, so Postgres
just lets me set 7680 kbytes as the max. Whatever asan is doing doesn't
seem to leave enough stack space for Postgres.
---
I would like to see patch 1 reviewed and committed. Patch 2 honestly
doesn't matter unless asan support can be fixed. I can also add a patch
that errors out the Meson build if asan support is requested. That way
others don't spend time heading down a dead end.
--
Tristan Partin
Neon (https://neon.tech)
Spend so much time writing out the email, I once again forget
attachments...UGH.
--
Tristan Partin
Neon (https://neon.tech)
Attachments:
v1-0001-Refuse-to-register-message-in-LogLogicalMessage-i.patchtext/x-patch; charset=utf-8; name=v1-0001-Refuse-to-register-message-in-LogLogicalMessage-i.patchDownload
From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 29 Jan 2024 18:03:39 -0600
Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if
NULL
If this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks
the API contract of memcpy in glibc. The two pointer arguments are
marked as nonnull, even in the event the amount to copy is 0 bytes.
---
src/backend/access/transam/xlog.c | 1 +
src/backend/replication/logical/message.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 478377c4a2..929888beb5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1288,6 +1288,7 @@ CopyXLogRecordToWAL(int write_len, bool isLogSwitch, XLogRecData *rdata,
}
Assert(CurrPos % XLOG_BLCKSZ >= SizeOfXLogShortPHD || rdata_len == 0);
+ Assert(rdata_data != NULL);
memcpy(currpos, rdata_data, rdata_len);
currpos += rdata_len;
CurrPos += rdata_len;
diff --git a/src/backend/replication/logical/message.c b/src/backend/replication/logical/message.c
index 2ac34e7781..126c57ef6e 100644
--- a/src/backend/replication/logical/message.c
+++ b/src/backend/replication/logical/message.c
@@ -67,7 +67,8 @@ LogLogicalMessage(const char *prefix, const char *message, size_t size,
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfLogicalMessage);
XLogRegisterData(unconstify(char *, prefix), xlrec.prefix_size);
- XLogRegisterData(unconstify(char *, message), size);
+ if (message)
+ XLogRegisterData(unconstify(char *, message), size);
/* allow origin filtering */
XLogSetRecordFlags(XLOG_INCLUDE_ORIGIN);
--
Tristan Partin
Neon (https://neon.tech)
v1-0002-meson-Support-compiling-with-Db_sanitize-address.patchtext/x-patch; charset=utf-8; name=v1-0002-meson-Support-compiling-with-Db_sanitize-address.patchDownload
From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Wed, 24 Jan 2024 17:07:01 -0600
Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=address
The ecpg is parser is extremely leaky, so we need to silence leak
detection.
---
meson.build | 3 +++
src/bin/initdb/initdb.c | 11 +++++++++++
src/bin/pg_config/pg_config.c | 10 ++++++++++
src/bin/pg_resetwal/pg_resetwal.c | 10 ++++++++++
src/include/pg_config.h.in | 5 +++++
src/interfaces/ecpg/preproc/ecpg.c | 11 +++++++++++
6 files changed, 50 insertions(+)
diff --git a/meson.build b/meson.build
index 8ed51b6aae..d8c524d6f6 100644
--- a/meson.build
+++ b/meson.build
@@ -2530,6 +2530,9 @@ cdata.set_quoted('PG_VERSION_STR',
)
)
+if get_option('b_sanitize').contains('address')
+ cdata.set('USE_ADDRESS_SANITIZER', 1)
+endif
###############################################################
# NLS / Gettext
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ac409b0006..e18e716d9c 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -338,6 +338,17 @@ do { \
output_failed = true, output_errno = errno; \
} while (0)
+#ifdef USE_ADDRESS_SANITIZER
+
+const char *__asan_default_options(void);
+
+const char *__asan_default_options(void)
+{
+ return "detect_leaks=0";
+}
+
+#endif
+
/*
* Escape single quotes and backslashes, suitably for insertions into
* configuration files or SQL E'' strings.
diff --git a/src/bin/pg_config/pg_config.c b/src/bin/pg_config/pg_config.c
index 77d09ccfc4..26d0b2f62b 100644
--- a/src/bin/pg_config/pg_config.c
+++ b/src/bin/pg_config/pg_config.c
@@ -67,6 +67,16 @@ static const InfoItem info_items[] = {
{NULL, NULL}
};
+#ifdef USE_ADDRESS_SANITIZER
+
+const char *__asan_default_options(void);
+
+const char *__asan_default_options(void)
+{
+ return "detect_leaks=0";
+}
+
+#endif
static void
help(void)
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index e9dcb5a6d8..54f1ce5e44 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -89,6 +89,16 @@ static void KillExistingWALSummaries(void);
static void WriteEmptyXLOG(void);
static void usage(void);
+#ifdef USE_ADDRESS_SANITIZER
+
+const char *__asan_default_options(void);
+
+const char *__asan_default_options(void)
+{
+ return "detect_leaks=0";
+}
+
+#endif
int
main(int argc, char *argv[])
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 07e73567dc..ce0c700b6d 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -668,6 +668,11 @@
/* Define to 1 if strerror_r() returns int. */
#undef STRERROR_R_INT
+/* Define to 1 if using the address sanitizer. Typically this can be detecte
+ * with __has_feature(address_sanitizer), but GCC doesn't support it with C99.
+ * Remove it when the standard is bumped. */
+#undef USE_ADDRESS_SANITIZER
+
/* Define to 1 to use ARMv8 CRC Extension. */
#undef USE_ARMV8_CRC32C
diff --git a/src/interfaces/ecpg/preproc/ecpg.c b/src/interfaces/ecpg/preproc/ecpg.c
index e4db21e0c1..9664de3f77 100644
--- a/src/interfaces/ecpg/preproc/ecpg.c
+++ b/src/interfaces/ecpg/preproc/ecpg.c
@@ -30,6 +30,17 @@ struct typedefs *types = NULL;
struct _defines *defines = NULL;
struct declared_list *g_declared_list = NULL;
+#ifdef USE_ADDRESS_SANITIZER
+
+const char *__asan_default_options(void);
+
+const char *__asan_default_options(void)
+{
+ return "detect_leaks=0";
+}
+
+#endif
+
static void
help(const char *progname)
{
--
Tristan Partin
Neon (https://neon.tech)
v1-0003-Max-out-max_stack_depth-when-asan-is-enabled.patchtext/x-patch; charset=utf-8; name=v1-0003-Max-out-max_stack_depth-when-asan-is-enabled.patchDownload
From 79c85683b49dd92c6b197fc21a92e09201bc163d Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 29 Jan 2024 18:00:03 -0600
Subject: [PATCH v1 3/3] Max out max_stack_depth when asan is enabled
AddressSanitizer requires more stack than normal. Tests will not run at
2KB.
---
src/backend/utils/misc/guc.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 8f65ef3d89..f163701229 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1619,7 +1619,11 @@ InitializeGUCOptionsFromEnvironment(void)
source = PGC_S_ENV_VAR;
else
{
+#ifdef USE_ADDRESS_SANITIZER
+ new_limit = 7680;
+#else
new_limit = 2048;
+#endif
source = PGC_S_DYNAMIC_DEFAULT;
}
snprintf(limbuf, sizeof(limbuf), "%ld", new_limit);
--
Tristan Partin
Neon (https://neon.tech)
On 30/01/2024 17:57, Tristan Partin wrote:
Patch 1:
Passing NULL as a second argument to memcpy breaks ubsan, and there
didn't seem to be anything preventing that in the LogLogicalMessage()
codepath. Here is a preventative measure in LogLogicalMessage() and an
Assert() in CopyXLogRecordToWAL().
For the audience: We ran into this one with the neon extension. The
solution is to call LogLogicalMessage("", 0, ...) instead of
LogLogicalMessage(NULL, 0, ...). . But it's true that it's pointlessfor
LogLogicalMessage to call XLogRegisterData() if the message is empty. If
we do this, we should check for 'size == 0' rather than 'message == NULL'.
Patch 2:
Support building with -Db_sanitize=address in Meson. Various executables
are leaky which can cause the builds to fail and tests to fail, when we
are fine leaking this memory.Personally, I am a big stickler for always freeing my memory in
executables even if it gets released at program exit because it makes
the leak sanitizer much more effective. However (!), I am not here to
change the philosophy of memory management in one-off executables, so
I have just silenced memory leaks in various executables for now.Patch 3:
THIS DOESN'T WORK. DO NOT COMMIT. PROPOSING FOR DISCUSSION!
In my effort to try to see if the test suite would pass with asan
enabled, I ran into a max_stack_depth issue. I tried maxing it out
(hence, the patch), but that still didn't remedy my issue. I tried to
look on the list for any relevant emails, but nothing turned up. Maybe
others have not attempted this before? Seems doubtful.Not entirely sure how to fix this issue. I personally find asan
extremely effective, even more than valgrind, so it would be great if
I could run Postgres with asan enabled to catch various stupid C issues
I might create. On my system, ulimit -a returns 8192 kbytes, so Postgres
just lets me set 7680 kbytes as the max. Whatever asan is doing doesn't
seem to leave enough stack space for Postgres.
I'm a bit confused by these. We already run with sanitizer in the cirrus
CI. What does this enable that we're not already doing?
--
Heikki Linnakangas
Neon (https://neon.tech)
Hi,
On 2024-01-30 22:05:28 +0200, Heikki Linnakangas wrote:
On 30/01/2024 17:57, Tristan Partin wrote:
In my effort to try to see if the test suite would pass with asan
enabled, I ran into a max_stack_depth issue. I tried maxing it out
(hence, the patch), but that still didn't remedy my issue. I tried to
look on the list for any relevant emails, but nothing turned up. Maybe
others have not attempted this before? Seems doubtful.Not entirely sure how to fix this issue. I personally find asan
extremely effective, even more than valgrind, so it would be great if
I could run Postgres with asan enabled to catch various stupid C issues
I might create. On my system, ulimit -a returns 8192 kbytes, so Postgres
just lets me set 7680 kbytes as the max. Whatever asan is doing doesn't
seem to leave enough stack space for Postgres.I'm a bit confused by these. We already run with sanitizer in the cirrus CI.
What does this enable that we're not already doing?
The reason asan fails is that it uses a "shadow stack" to track stack variable
lifetimes. These confuse our stack depth check. CI doesn't have the issue
because the compiler doesn't yet enable the feature, locally I get around it
by using ASAN_OPTIONS=detect_stack_use_after_return=0:...
The checks are actually quite useful, so making our stack depth check work
with asan would be worthwhile.
I discussed this in a bit more in
/messages/by-id/20231129193920.4vphw7dqxjzf5v5b@awork3.anarazel.de
Greetings,
Andres Freund
Hi,
On 2024-01-30 09:59:25 -0600, Tristan Partin wrote:
From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 29 Jan 2024 18:03:39 -0600
Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if
NULL
If this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks
the API contract of memcpy in glibc. The two pointer arguments are
marked as nonnull, even in the event the amount to copy is 0 bytes.
It seems pretty odd to call LogLogicalMessage() with a NULL argument. Why is
that something useful?
From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Wed, 24 Jan 2024 17:07:01 -0600
Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=addressThe ecpg is parser is extremely leaky, so we need to silence leak
detection.
This does stuff beyond epcg...
+if get_option('b_sanitize').contains('address') + cdata.set('USE_ADDRESS_SANITIZER', 1) +endif############################################################### # NLS / Gettext diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index ac409b0006..e18e716d9c 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -338,6 +338,17 @@ do { \ output_failed = true, output_errno = errno; \ } while (0)+#ifdef USE_ADDRESS_SANITIZER
When asan is used __SANITIZE_ADDRESS__ is defined, so we don't need to
implement this ourselves.
+const char *__asan_default_options(void); + +const char *__asan_default_options(void) +{ + return "detect_leaks=0"; +} + +#endif
Wonder if we should move this into some static library and link it into all
binaries that don't want leak detection? It doesn't seem great to have to
adjust this in a bunch of files if we want to adjust the options...
Greetings,
Andres Freund
Hello,
30.01.2024 18:57, Tristan Partin wrote:
Patch 1:
Passing NULL as a second argument to memcpy breaks ubsan, ...
Maybe you would like to fix also one more similar place, reached with:
create extension xml2;
select xslt_process('<x/>',
$$<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:template match="@*|node()">
</xsl:template>
</xsl:stylesheet>$$);
varlena.c:201:26: runtime error: null pointer passed as argument 2, which is declared to never be null
There is also an issue with pg_bsd_indent, I stumble upon when doing
`make check-world` with the sanitizers enabled:
/messages/by-id/591971ce-25c1-90f3-0526-5f54e3ebb32e@gmail.com
31.01.2024 00:23, Andres Freund wrote:
The reason asan fails is that it uses a "shadow stack" to track stack variable
lifetimes. These confuse our stack depth check. CI doesn't have the issue
because the compiler doesn't yet enable the feature, locally I get around it
by using ASAN_OPTIONS=detect_stack_use_after_return=0:...
Even with detect_stack_use_after_return=0, clang-18's asan makes the test
012_subtransactions.pl fail:
2024-01-31 03:24:25.691 UTC [4112455] 012_subtransactions.pl LOG: statement: SELECT hs_subxids(201);
2024-01-31 03:24:25.714 UTC [4112455] 012_subtransactions.pl ERROR: stack depth limit exceeded
2024-01-31 03:24:25.714 UTC [4112455] 012_subtransactions.pl HINT: Increase the configuration parameter max_stack_depth
(currently 2048kB), after ensuring the platform's stack depth limit is adequate.
(All the other tests pass.)
Though the same test passes when I use clang-16.
Best regards,
Alexander
On Tue Jan 30, 2024 at 10:00 PM CST, Alexander Lakhin wrote:
Hello,
30.01.2024 18:57, Tristan Partin wrote:
Patch 1:
Passing NULL as a second argument to memcpy breaks ubsan, ...
Maybe you would like to fix also one more similar place, reached with:
create extension xml2;
select xslt_process('<x/>',
$$<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
<xsl:template match="@*|node()">
</xsl:template>
</xsl:stylesheet>$$);varlena.c:201:26: runtime error: null pointer passed as argument 2, which is declared to never be null
There is also an issue with pg_bsd_indent, I stumble upon when doing
`make check-world` with the sanitizers enabled:
/messages/by-id/591971ce-25c1-90f3-0526-5f54e3ebb32e@gmail.com31.01.2024 00:23, Andres Freund wrote:
The reason asan fails is that it uses a "shadow stack" to track stack variable
lifetimes. These confuse our stack depth check. CI doesn't have the issue
because the compiler doesn't yet enable the feature, locally I get around it
by using ASAN_OPTIONS=detect_stack_use_after_return=0:...Even with detect_stack_use_after_return=0, clang-18's asan makes the test
012_subtransactions.pl fail:
2024-01-31 03:24:25.691 UTC [4112455] 012_subtransactions.pl LOG: statement: SELECT hs_subxids(201);
2024-01-31 03:24:25.714 UTC [4112455] 012_subtransactions.pl ERROR: stack depth limit exceeded
2024-01-31 03:24:25.714 UTC [4112455] 012_subtransactions.pl HINT: Increase the configuration parameter max_stack_depth
(currently 2048kB), after ensuring the platform's stack depth limit is adequate.(All the other tests pass.)
Though the same test passes when I use clang-16.
Thanks Alexander! I will try and take a look at these.
--
Tristan Partin
Neon (https://neon.tech)
On Tue Jan 30, 2024 at 3:58 PM CST, Andres Freund wrote:
Hi,
On 2024-01-30 09:59:25 -0600, Tristan Partin wrote:
From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 29 Jan 2024 18:03:39 -0600
Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if
NULLIf this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks
the API contract of memcpy in glibc. The two pointer arguments are
marked as nonnull, even in the event the amount to copy is 0 bytes.It seems pretty odd to call LogLogicalMessage() with a NULL argument. Why is
that something useful?
Dropped. Will change on the Neon side. Should we add an assert
somewhere for good measure? Near the memcpy in question?
From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Wed, 24 Jan 2024 17:07:01 -0600
Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=addressThe ecpg is parser is extremely leaky, so we need to silence leak
detection.This does stuff beyond epcg...
Dropped.
+if get_option('b_sanitize').contains('address') + cdata.set('USE_ADDRESS_SANITIZER', 1) +endif############################################################### # NLS / Gettext diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index ac409b0006..e18e716d9c 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -338,6 +338,17 @@ do { \ output_failed = true, output_errno = errno; \ } while (0)+#ifdef USE_ADDRESS_SANITIZER
When asan is used __SANITIZE_ADDRESS__ is defined, so we don't need to
implement this ourselves.
Thanks!
+const char *__asan_default_options(void); + +const char *__asan_default_options(void) +{ + return "detect_leaks=0"; +} + +#endifWonder if we should move this into some static library and link it into all
binaries that don't want leak detection? It doesn't seem great to have to
adjust this in a bunch of files if we want to adjust the options...
See attached patches. Here is what I found to be necessary to get
a -Db_sanitize=address,undefined build to successfully make it through
tests. I do have a few concerns about the patch.
1. For whatever reason, __SANITIZE_LEAK__ is not defined when the leak
sanitizer is enabled. So you will see me use this, to make some
include directives work. I don't like this as a final solution
because someone could use -fsanitize=leak.
2. I tracked down what seems to be a valid leak in adt/xml.c. Attached
(test.sql) is a fairly minimal reproduction of what is needed to show
the leak. I didn't spend too much time tracking it down. Might get to
it later, who knows. Below you will find the backtrace, and whoever
wants to try their hand at fixing it will need to comment out
xmlNewNode in the leak.supp file.
3. I don't love the new library name. Maybe it should be name more lsan
specific.
4. Should pg_attribute_no_asan be renamed to
pg_attribute_no_sanitize_address? That would match
pg_attribute_no_sanitize_alignment.
I will also attach a Meson test log for good measure. I didn't try
testing anything that requires PG_TEST_EXTRA, but I suspect that
everything will be fine.
Alexander, I haven't yet gotten to the things you pointed out in the
sibling thread.
==221848==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 120 byte(s) in 1 object(s) allocated from:
#0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c)
#1 0x7fac4a48427d in xmlNewNode (/lib64/libxml2.so.2+0x5d27d) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c)
#2 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754
#3 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020
#4 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537
#5 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881
#6 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355
#7 0x109655d in ExecProject ../src/include/executor/executor.h:389
#8 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136
#9 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464
#10 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273
#11 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670
#12 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365
#13 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309
#14 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924
#15 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768
#16 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273
#17 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653
#18 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464
#19 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140
#20 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776
#21 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475
#22 0x11a5f67 in main ../src/backend/main/main.c:198
#23 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#24 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#25 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b)
Indirect leak of 13 byte(s) in 1 object(s) allocated from:
#0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c)
#1 0x7fac4a4e106f in xmlStrndup (/lib64/libxml2.so.2+0xba06f) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c)
#2 0x7fac4a4842c0 in xmlNewNode (/lib64/libxml2.so.2+0x5d2c0) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c)
#3 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754
#4 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020
#5 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537
#6 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881
#7 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355
#8 0x109655d in ExecProject ../src/include/executor/executor.h:389
#9 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136
#10 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464
#11 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273
#12 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670
#13 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365
#14 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309
#15 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924
#16 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768
#17 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273
#18 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653
#19 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464
#20 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140
#21 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776
#22 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475
#23 0x11a5f67 in main ../src/backend/main/main.c:198
#24 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#25 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#26 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b)
SUMMARY: AddressSanitizer: 133 byte(s) leaked in 2 allocation(s).
--
Tristan Partin
Neon (https://neon.tech)
Attachments:
v2-0001-Add-pg_attribute_no_asan.patchtext/x-patch; charset=utf-8; name=v2-0001-Add-pg_attribute_no_asan.patchDownload
From 40f56251ef629eed2ebb6d9b2f875eac06678cab Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 5 Feb 2024 20:18:59 -0600
Subject: [PATCH v2 1/4] Add pg_attribute_no_asan
Putting this attribute on a function will disable AddressSanitizer
instrumentation in a function.
---
src/include/c.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/include/c.h b/src/include/c.h
index 2e3ea206e1..11e7df0fd7 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -272,6 +272,13 @@
#endif /* defined(__MINGW64__) && __GNUC__ == 8 &&
* __GNUC_MINOR__ == 1 */
+
+#if __SANITIZE_ADDRESS__
+#define pg_attribute_no_asan __attribute__((no_sanitize("address")))
+#else
+#define pg_attribute_no_asan
+#endif
+
/*
* Mark a point as unreachable in a portable fashion. This should preferably
* be something that the compiler understands, to aid code generation.
--
Tristan Partin
Neon (https://neon.tech)
v2-0002-Fix-stack-depth-checking-with-AddressSanitizer-en.patchtext/x-patch; charset=utf-8; name=v2-0002-Fix-stack-depth-checking-with-AddressSanitizer-en.patchDownload
From 7192614059f6a7bddc5e32df21e5956aa714b820 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 5 Feb 2024 20:21:13 -0600
Subject: [PATCH v2 2/4] Fix stack depth checking with AddressSanitizer enabled
The AddressSanitizer instrumentation caused stack depth checking to
fail.
---
src/backend/tcop/postgres.c | 4 ++--
src/include/miscadmin.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 1a34bd3715..7cc18dbed9 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3466,7 +3466,7 @@ ProcessInterrupts(void)
*
* Returns the old reference point, if any.
*/
-pg_stack_base_t
+pg_stack_base_t pg_attribute_no_asan
set_stack_base(void)
{
#ifndef HAVE__BUILTIN_FRAME_ADDRESS
@@ -3530,7 +3530,7 @@ check_stack_depth(void)
}
}
-bool
+bool pg_attribute_no_asan
stack_is_too_deep(void)
{
char stack_top_loc;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0b01c1f093..538adfe757 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -292,10 +292,10 @@ extern PGDLLIMPORT bool VacuumCostActive;
typedef char *pg_stack_base_t;
-extern pg_stack_base_t set_stack_base(void);
+extern pg_stack_base_t set_stack_base(void) pg_attribute_no_asan;
extern void restore_stack_base(pg_stack_base_t base);
extern void check_stack_depth(void);
-extern bool stack_is_too_deep(void);
+extern bool stack_is_too_deep(void) pg_attribute_no_asan;
/* in tcop/utility.c */
extern void PreventCommandIfReadOnly(const char *cmdname);
--
Tristan Partin
Neon (https://neon.tech)
v2-0003-Add-libpgbincommon.patchtext/x-patch; charset=utf-8; name=v2-0003-Add-libpgbincommon.patchDownload
From f40310fddfc361d5ed24ec1fb5043ce211179e35 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 5 Feb 2024 20:24:27 -0600
Subject: [PATCH v2 3/4] Add libpgbincommon
This library is supposed to be linked wholly into executables. It
defines a function which turns leak detection off when using the
AddressSanitizer.
---
src/bin/common/asan.c | 12 ++++++++++++
src/bin/common/meson.build | 8 ++++++++
src/bin/meson.build | 1 +
3 files changed, 21 insertions(+)
create mode 100644 src/bin/common/asan.c
create mode 100644 src/bin/common/meson.build
diff --git a/src/bin/common/asan.c b/src/bin/common/asan.c
new file mode 100644
index 0000000000..d2739b2ad7
--- /dev/null
+++ b/src/bin/common/asan.c
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * src/bin/common/asan.c
+ */
+
+const char *__asan_default_options(void);
+
+const char *__asan_default_options(void)
+{
+ return "detect_leaks=0";
+}
diff --git a/src/bin/common/meson.build b/src/bin/common/meson.build
new file mode 100644
index 0000000000..f63b9d664b
--- /dev/null
+++ b/src/bin/common/meson.build
@@ -0,0 +1,8 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+libpgbincommon = static_library(
+ 'pgbincommon',
+ 'asan.c',
+)
+
+libpgbincommon_dep = declare_dependency(link_whole: libpgbincommon)
diff --git a/src/bin/meson.build b/src/bin/meson.build
index aa60ebaa30..fb18d618be 100644
--- a/src/bin/meson.build
+++ b/src/bin/meson.build
@@ -1,5 +1,6 @@
# Copyright (c) 2022-2024, PostgreSQL Global Development Group
+subdir('common')
subdir('initdb')
subdir('pg_amcheck')
subdir('pg_archivecleanup')
--
Tristan Partin
Neon (https://neon.tech)
v2-0004-Add-instrumentation-to-disable-the-LeakSanitizer-.patchtext/x-patch; charset=utf-8; name=v2-0004-Add-instrumentation-to-disable-the-LeakSanitizer-.patchDownload
From 0c1b9bf1ab5c178ffa4c731b541deedfc9a552ed Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 5 Feb 2024 20:29:38 -0600
Subject: [PATCH v2 4/4] Add instrumentation to disable the LeakSanitizer in
various paths
We knowingly leak memory in certain executables, so we should turn leak
detection off. Other code paths also leak memory, like plpython, when
registering global data structures.
---
leak.supp | 4 ++++
meson.build | 3 ++-
src/bin/initdb/meson.build | 2 +-
src/bin/pg_amcheck/meson.build | 2 +-
src/bin/pg_basebackup/meson.build | 2 +-
src/bin/pg_combinebackup/meson.build | 2 +-
src/bin/pg_config/meson.build | 2 +-
src/bin/pg_controldata/meson.build | 2 +-
src/bin/pg_ctl/meson.build | 2 +-
src/bin/pg_dump/meson.build | 6 +++---
src/bin/pg_resetwal/meson.build | 2 +-
src/bin/pg_rewind/meson.build | 2 +-
src/bin/pg_upgrade/meson.build | 2 +-
src/bin/pg_verifybackup/meson.build | 2 +-
src/bin/pg_waldump/meson.build | 2 +-
src/bin/pg_walsummary/meson.build | 2 +-
src/bin/pgbench/meson.build | 2 +-
src/bin/psql/meson.build | 2 +-
src/bin/scripts/meson.build | 2 +-
src/interfaces/ecpg/preproc/meson.build | 2 +-
src/interfaces/ecpg/test/meson.build | 4 ++--
src/interfaces/ecpg/test/thread/meson.build | 2 +-
src/interfaces/libpq/test/meson.build | 2 +-
src/pl/plpython/plpy_exec.c | 12 ++++++++++++
src/pl/plpython/plpy_main.c | 14 ++++++++++++++
src/pl/plpython/plpy_procedure.c | 14 ++++++++++++++
src/test/isolation/meson.build | 4 ++--
src/test/modules/libpq_pipeline/meson.build | 2 +-
src/test/regress/meson.build | 2 +-
29 files changed, 74 insertions(+), 29 deletions(-)
create mode 100644 leak.supp
diff --git a/leak.supp b/leak.supp
new file mode 100644
index 0000000000..d72c5b368f
--- /dev/null
+++ b/leak.supp
@@ -0,0 +1,4 @@
+leak:PyImport_ImportModule
+leak:RAND_status
+leak:save_ps_display_args
+leak:xmlNewNode
diff --git a/meson.build b/meson.build
index 8ed51b6aae..03fe3e0d89 100644
--- a/meson.build
+++ b/meson.build
@@ -3100,6 +3100,7 @@ if library_path_var != ''
test_env.prepend(library_path_var, test_install_location / get_option('libdir'))
endif
+test_env.set('LSAN_OPTIONS', 'print_suppressions=0:suppressions=@0@'.format(meson.source_root() / 'leak.supp'))
# Create (and remove old) initdb template directory. Tests use that, where
# possible, to make it cheaper to run tests.
@@ -3215,7 +3216,7 @@ foreach test_dir : tests
test_kwargs = {
'protocol': 'tap',
'priority': 10,
- 'timeout': 1000,
+ 'timeout': get_option('b_sanitize') == '' ? 1000 : 2000,
'depends': test_deps + t.get('deps', []),
'env': env,
} + t.get('test_kwargs', {})
diff --git a/src/bin/initdb/meson.build b/src/bin/initdb/meson.build
index 7dc5ed6e77..7afe6cd326 100644
--- a/src/bin/initdb/meson.build
+++ b/src/bin/initdb/meson.build
@@ -21,7 +21,7 @@ initdb = executable('initdb',
# shared library from a different PG version. Define
# USE_PRIVATE_ENCODING_FUNCS to ensure that that happens.
c_args: ['-DUSE_PRIVATE_ENCODING_FUNCS'],
- dependencies: [frontend_code, libpq, icu, icu_i18n],
+ dependencies: [frontend_code, libpq, icu, icu_i18n, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += initdb
diff --git a/src/bin/pg_amcheck/meson.build b/src/bin/pg_amcheck/meson.build
index 292b33eb09..8bcaab36db 100644
--- a/src/bin/pg_amcheck/meson.build
+++ b/src/bin/pg_amcheck/meson.build
@@ -12,7 +12,7 @@ endif
pg_amcheck = executable('pg_amcheck',
pg_amcheck_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_amcheck
diff --git a/src/bin/pg_basebackup/meson.build b/src/bin/pg_basebackup/meson.build
index f7e60e6670..157fe95c0c 100644
--- a/src/bin/pg_basebackup/meson.build
+++ b/src/bin/pg_basebackup/meson.build
@@ -12,7 +12,7 @@ common_sources = files(
'walmethods.c',
)
-pg_basebackup_deps = [frontend_code, libpq, lz4, zlib, zstd]
+pg_basebackup_deps = [frontend_code, libpq, libpgbincommon_dep, lz4, zlib, zstd]
pg_basebackup_common = static_library('libpg_basebackup_common',
common_sources,
dependencies: pg_basebackup_deps,
diff --git a/src/bin/pg_combinebackup/meson.build b/src/bin/pg_combinebackup/meson.build
index 30dbbaa6cf..ff7e4e796e 100644
--- a/src/bin/pg_combinebackup/meson.build
+++ b/src/bin/pg_combinebackup/meson.build
@@ -17,7 +17,7 @@ endif
pg_combinebackup = executable('pg_combinebackup',
pg_combinebackup_sources,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_combinebackup
diff --git a/src/bin/pg_config/meson.build b/src/bin/pg_config/meson.build
index b4fddd297a..9d717072d4 100644
--- a/src/bin/pg_config/meson.build
+++ b/src/bin/pg_config/meson.build
@@ -12,7 +12,7 @@ endif
pg_config = executable('pg_config',
pg_config_sources,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_config
diff --git a/src/bin/pg_controldata/meson.build b/src/bin/pg_controldata/meson.build
index 8009bd8c9a..ff54169084 100644
--- a/src/bin/pg_controldata/meson.build
+++ b/src/bin/pg_controldata/meson.build
@@ -12,7 +12,7 @@ endif
pg_controldata = executable('pg_controldata',
pg_controldata_sources,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_controldata
diff --git a/src/bin/pg_ctl/meson.build b/src/bin/pg_ctl/meson.build
index da05bd8a8d..da013dec86 100644
--- a/src/bin/pg_ctl/meson.build
+++ b/src/bin/pg_ctl/meson.build
@@ -12,7 +12,7 @@ endif
pg_ctl = executable('pg_ctl',
pg_ctl_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_ctl
diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build
index ecd0a0a0e1..3e465f9711 100644
--- a/src/bin/pg_dump/meson.build
+++ b/src/bin/pg_dump/meson.build
@@ -41,7 +41,7 @@ endif
pg_dump = executable('pg_dump',
pg_dump_sources,
link_with: [pg_dump_common],
- dependencies: [frontend_code, libpq, zlib],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep, zlib],
kwargs: default_bin_args,
)
bin_targets += pg_dump
@@ -60,7 +60,7 @@ endif
pg_dumpall = executable('pg_dumpall',
pg_dumpall_sources,
link_with: [pg_dump_common],
- dependencies: [frontend_code, libpq, zlib],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep, zlib],
kwargs: default_bin_args,
)
bin_targets += pg_dumpall
@@ -79,7 +79,7 @@ endif
pg_restore = executable('pg_restore',
pg_restore_sources,
link_with: [pg_dump_common],
- dependencies: [frontend_code, libpq, zlib],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep, zlib],
kwargs: default_bin_args,
)
bin_targets += pg_restore
diff --git a/src/bin/pg_resetwal/meson.build b/src/bin/pg_resetwal/meson.build
index c1239528db..d1397fb7ce 100644
--- a/src/bin/pg_resetwal/meson.build
+++ b/src/bin/pg_resetwal/meson.build
@@ -12,7 +12,7 @@ endif
pg_resetwal = executable('pg_resetwal',
pg_resetwal_sources,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_resetwal
diff --git a/src/bin/pg_rewind/meson.build b/src/bin/pg_rewind/meson.build
index e0f88bde22..75b74e89ae 100644
--- a/src/bin/pg_rewind/meson.build
+++ b/src/bin/pg_rewind/meson.build
@@ -21,7 +21,7 @@ endif
pg_rewind = executable('pg_rewind',
pg_rewind_sources,
- dependencies: [frontend_code, libpq, lz4, zstd],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep, lz4, zstd],
c_args: ['-DFRONTEND'], # needed for xlogreader et al
kwargs: default_bin_args,
)
diff --git a/src/bin/pg_upgrade/meson.build b/src/bin/pg_upgrade/meson.build
index 9825fa3305..11e55a2bbe 100644
--- a/src/bin/pg_upgrade/meson.build
+++ b/src/bin/pg_upgrade/meson.build
@@ -27,7 +27,7 @@ endif
pg_upgrade = executable('pg_upgrade',
pg_upgrade_sources,
c_pch: pch_postgres_fe_h,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_upgrade
diff --git a/src/bin/pg_verifybackup/meson.build b/src/bin/pg_verifybackup/meson.build
index 7c7d31a035..13c583ef55 100644
--- a/src/bin/pg_verifybackup/meson.build
+++ b/src/bin/pg_verifybackup/meson.build
@@ -12,7 +12,7 @@ endif
pg_verifybackup = executable('pg_verifybackup',
pg_verifybackup_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_verifybackup
diff --git a/src/bin/pg_waldump/meson.build b/src/bin/pg_waldump/meson.build
index bb30f0fe08..05d23f7fee 100644
--- a/src/bin/pg_waldump/meson.build
+++ b/src/bin/pg_waldump/meson.build
@@ -18,7 +18,7 @@ endif
pg_waldump = executable('pg_waldump',
pg_waldump_sources,
- dependencies: [frontend_code, lz4, zstd],
+ dependencies: [frontend_code, libpgbincommon_dep, lz4, zstd],
c_args: ['-DFRONTEND'], # needed for xlogreader et al
kwargs: default_bin_args,
)
diff --git a/src/bin/pg_walsummary/meson.build b/src/bin/pg_walsummary/meson.build
index f886a4cb36..bac71a5776 100644
--- a/src/bin/pg_walsummary/meson.build
+++ b/src/bin/pg_walsummary/meson.build
@@ -12,7 +12,7 @@ endif
pg_walsummary = executable('pg_walsummary',
pg_walsummary_sources,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += pg_walsummary
diff --git a/src/bin/pgbench/meson.build b/src/bin/pgbench/meson.build
index d330bb5eb0..5a4eab226f 100644
--- a/src/bin/pgbench/meson.build
+++ b/src/bin/pgbench/meson.build
@@ -27,7 +27,7 @@ endif
pgbench = executable('pgbench',
pgbench_sources,
- dependencies: [frontend_code, libpq, thread_dep],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep, thread_dep],
include_directories: include_directories('.'),
c_pch: pch_postgres_fe_h,
c_args: host_system == 'windows' ? ['-DFD_SETSIZE=1024'] : [],
diff --git a/src/bin/psql/meson.build b/src/bin/psql/meson.build
index f3a6392138..9614a1fbd0 100644
--- a/src/bin/psql/meson.build
+++ b/src/bin/psql/meson.build
@@ -48,7 +48,7 @@ psql = executable('psql',
psql_sources,
c_pch: pch_postgres_fe_h,
include_directories: include_directories('.'),
- dependencies: [frontend_code, libpq, readline],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep, readline],
kwargs: default_bin_args,
)
bin_targets += psql
diff --git a/src/bin/scripts/meson.build b/src/bin/scripts/meson.build
index cef24d7806..dc06580f9e 100644
--- a/src/bin/scripts/meson.build
+++ b/src/bin/scripts/meson.build
@@ -29,7 +29,7 @@ foreach binary : binaries
binary = executable(binary,
binary_sources,
link_with: [scripts_common],
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args,
)
bin_targets += binary
diff --git a/src/interfaces/ecpg/preproc/meson.build b/src/interfaces/ecpg/preproc/meson.build
index ddd7a66547..fc21e3fc36 100644
--- a/src/interfaces/ecpg/preproc/meson.build
+++ b/src/interfaces/ecpg/preproc/meson.build
@@ -94,7 +94,7 @@ ecpg_exe = executable('ecpg',
ecpg_sources,
include_directories: ['.', ecpg_inc, postgres_inc, libpq_inc],
c_pch: pch_postgres_fe_h,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpgbincommon_dep],
kwargs: default_bin_args,
)
ecpg_targets += ecpg_exe
diff --git a/src/interfaces/ecpg/test/meson.build b/src/interfaces/ecpg/test/meson.build
index c1e508ccc8..c4c5a3e602 100644
--- a/src/interfaces/ecpg/test/meson.build
+++ b/src/interfaces/ecpg/test/meson.build
@@ -18,7 +18,7 @@ pg_regress_ecpg = executable('pg_regress_ecpg',
pg_regress_ecpg_sources,
c_args: pg_regress_cflags,
include_directories: [pg_regress_inc, include_directories('.')],
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args + {
'install': false
},
@@ -27,7 +27,7 @@ testprep_targets += pg_regress_ecpg
# create .c files and executables from .pgc files
ecpg_test_exec_kw = {
- 'dependencies': [frontend_code, libpq],
+ 'dependencies': [frontend_code, libpq, libpgbincommon_dep],
'include_directories': [ecpg_inc],
'link_with': [ecpglib_so, ecpg_compat_so, ecpg_pgtypes_so],
'build_by_default': false,
diff --git a/src/interfaces/ecpg/test/thread/meson.build b/src/interfaces/ecpg/test/thread/meson.build
index 5ed67ccbcd..d7e590db61 100644
--- a/src/interfaces/ecpg/test/thread/meson.build
+++ b/src/interfaces/ecpg/test/thread/meson.build
@@ -18,6 +18,6 @@ foreach pgc_file : pgc_files
ecpg_test_dependencies += executable(pgc_file,
exe_input,
- kwargs: ecpg_test_exec_kw + {'dependencies': [frontend_code, libpq, thread_dep,]},
+ kwargs: ecpg_test_exec_kw + {'dependencies': [frontend_code, libpq, libpgbincommon_dep, thread_dep,]},
)
endforeach
diff --git a/src/interfaces/libpq/test/meson.build b/src/interfaces/libpq/test/meson.build
index 21dd37f69b..070c2cc72f 100644
--- a/src/interfaces/libpq/test/meson.build
+++ b/src/interfaces/libpq/test/meson.build
@@ -12,7 +12,7 @@ endif
testprep_targets += executable('libpq_uri_regress',
libpq_uri_regress_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args + {
'install': false,
}
diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index e06fde1dd9..e7a8372b30 100644
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -23,6 +23,10 @@
#include "utils/rel.h"
#include "utils/typcache.h"
+#ifdef __SANITIZE_ADDRESS__
+#include <sanitizer/lsan_interface.h>
+#endif
+
/* saved state for a set-returning function */
typedef struct PLySRFState
{
@@ -1040,6 +1044,10 @@ PLy_procedure_call(PLyProcedure *proc, const char *kargs, PyObject *vargs)
PG_TRY();
{
+#ifdef __SANITIZE_ADDRESS__
+ __lsan_disable();
+#endif
+
#if PY_VERSION_HEX >= 0x03020000
rv = PyEval_EvalCode(proc->code,
proc->globals, proc->globals);
@@ -1048,6 +1056,10 @@ PLy_procedure_call(PLyProcedure *proc, const char *kargs, PyObject *vargs)
proc->globals, proc->globals);
#endif
+#ifdef __SANITIZE_ADDRESS__
+ __lsan_enable();
+#endif
+
/*
* Since plpy will only let you close subtransactions that you
* started, you cannot *unnest* subtransactions, only *nest* them
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 010a97378c..86d409ca35 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -24,6 +24,10 @@
#include "utils/rel.h"
#include "utils/syscache.h"
+#ifdef __SANITIZE_ADDRESS__
+#include <sanitizer/lsan_interface.h>
+#endif
+
/*
* exported functions
*/
@@ -113,14 +117,24 @@ PLy_initialize(void)
if (inited)
return;
+#ifdef __SANITIZE_ADDRESS__
+ __lsan_disable();
+#endif
+
PyImport_AppendInittab("plpy", PyInit_plpy);
Py_Initialize();
PyImport_ImportModule("plpy");
PLy_init_interp();
PLy_init_plpy();
+
+#ifdef __SANITIZE_ADDRESS__
+ __lsan_enable();
+#endif
+
if (PyErr_Occurred())
PLy_elog(FATAL, "untrapped error in initialization");
+
init_procedure_caches();
explicit_subtransactions = NIL;
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index 79b6ef6a44..2ff4337d2a 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -22,6 +22,10 @@
#include "utils/memutils.h"
#include "utils/syscache.h"
+#ifdef __SANITIZE_ADDRESS__
+#include <sanitizer/lsan_interface.h>
+#endif
+
static HTAB *PLy_procedure_cache = NULL;
static PLyProcedure *PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger);
@@ -370,7 +374,17 @@ PLy_procedure_compile(PLyProcedure *proc, const char *src)
msrc = PLy_procedure_munge_source(proc->pyname, src);
/* Save the mangled source for later inclusion in tracebacks */
proc->src = MemoryContextStrdup(proc->mcxt, msrc);
+
+#ifdef __SANITIZE_ADDRESS__
+ __lsan_disable();
+#endif
+
crv = PyRun_String(msrc, Py_file_input, proc->globals, NULL);
+
+#ifdef __SANITIZE_ADDRESS__
+ __lsan_enable();
+#endif
+
pfree(msrc);
if (crv != NULL)
diff --git a/src/test/isolation/meson.build b/src/test/isolation/meson.build
index 1082887a44..e2c54ba268 100644
--- a/src/test/isolation/meson.build
+++ b/src/test/isolation/meson.build
@@ -35,7 +35,7 @@ pg_isolation_regress = executable('pg_isolation_regress',
isolation_sources,
c_args: pg_regress_cflags,
include_directories: pg_regress_inc,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args + {
'install_dir': dir_pgxs / 'src/test/isolation',
},
@@ -52,7 +52,7 @@ endif
isolationtester = executable('isolationtester',
isolationtester_sources,
include_directories: include_directories('.'),
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args + {
'install_dir': dir_pgxs / 'src/test/isolation',
},
diff --git a/src/test/modules/libpq_pipeline/meson.build b/src/test/modules/libpq_pipeline/meson.build
index 727963ee68..4aa732f09d 100644
--- a/src/test/modules/libpq_pipeline/meson.build
+++ b/src/test/modules/libpq_pipeline/meson.build
@@ -12,7 +12,7 @@ endif
libpq_pipeline = executable('libpq_pipeline',
libpq_pipeline_sources,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args + {
'install': false,
},
diff --git a/src/test/regress/meson.build b/src/test/regress/meson.build
index 5a9be73531..a2a9b87e3f 100644
--- a/src/test/regress/meson.build
+++ b/src/test/regress/meson.build
@@ -30,7 +30,7 @@ endif
pg_regress = executable('pg_regress',
regress_sources,
c_args: pg_regress_cflags,
- dependencies: [frontend_code, libpq],
+ dependencies: [frontend_code, libpq, libpgbincommon_dep],
kwargs: default_bin_args + {
'install_dir': dir_pgxs / 'src/test/regress',
},
--
Tristan Partin
Neon (https://neon.tech)
Hi Tristan,
On Tue, Feb 6, 2024 at 11:53 AM Tristan Partin <tristan@neon.tech> wrote:
On Tue Jan 30, 2024 at 3:58 PM CST, Andres Freund wrote:
Hi,
On 2024-01-30 09:59:25 -0600, Tristan Partin wrote:
From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 29 Jan 2024 18:03:39 -0600
Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if
NULLIf this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks
the API contract of memcpy in glibc. The two pointer arguments are
marked as nonnull, even in the event the amount to copy is 0 bytes.It seems pretty odd to call LogLogicalMessage() with a NULL argument. Why is
that something useful?Dropped. Will change on the Neon side. Should we add an assert
somewhere for good measure? Near the memcpy in question?From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Wed, 24 Jan 2024 17:07:01 -0600
Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=addressThe ecpg is parser is extremely leaky, so we need to silence leak
detection.This does stuff beyond epcg...
Dropped.
+if get_option('b_sanitize').contains('address') + cdata.set('USE_ADDRESS_SANITIZER', 1) +endif############################################################### # NLS / Gettext diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index ac409b0006..e18e716d9c 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -338,6 +338,17 @@ do { \ output_failed = true, output_errno = errno; \ } while (0)+#ifdef USE_ADDRESS_SANITIZER
When asan is used __SANITIZE_ADDRESS__ is defined, so we don't need to
implement this ourselves.Thanks!
+const char *__asan_default_options(void); + +const char *__asan_default_options(void) +{ + return "detect_leaks=0"; +} + +#endifWonder if we should move this into some static library and link it into all
binaries that don't want leak detection? It doesn't seem great to have to
adjust this in a bunch of files if we want to adjust the options...See attached patches. Here is what I found to be necessary to get
a -Db_sanitize=address,undefined build to successfully make it through
tests. I do have a few concerns about the patch.1. For whatever reason, __SANITIZE_LEAK__ is not defined when the leak
sanitizer is enabled. So you will see me use this, to make some
include directives work. I don't like this as a final solution
because someone could use -fsanitize=leak.
2. I tracked down what seems to be a valid leak in adt/xml.c. Attached
(test.sql) is a fairly minimal reproduction of what is needed to show
the leak. I didn't spend too much time tracking it down. Might get to
it later, who knows. Below you will find the backtrace, and whoever
wants to try their hand at fixing it will need to comment out
xmlNewNode in the leak.supp file.
3. I don't love the new library name. Maybe it should be name more lsan
specific.
4. Should pg_attribute_no_asan be renamed to
pg_attribute_no_sanitize_address? That would match
pg_attribute_no_sanitize_alignment.I will also attach a Meson test log for good measure. I didn't try
testing anything that requires PG_TEST_EXTRA, but I suspect that
everything will be fine.Alexander, I haven't yet gotten to the things you pointed out in the
sibling thread.==221848==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 120 byte(s) in 1 object(s) allocated from:
#0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c)
#1 0x7fac4a48427d in xmlNewNode (/lib64/libxml2.so.2+0x5d27d) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c)
#2 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754
#3 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020
#4 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537
#5 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881
#6 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355
#7 0x109655d in ExecProject ../src/include/executor/executor.h:389
#8 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136
#9 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464
#10 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273
#11 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670
#12 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365
#13 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309
#14 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924
#15 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768
#16 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273
#17 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653
#18 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464
#19 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140
#20 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776
#21 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475
#22 0x11a5f67 in main ../src/backend/main/main.c:198
#23 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#24 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#25 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b)Indirect leak of 13 byte(s) in 1 object(s) allocated from:
#0 0x7fac4a6d92ef in malloc (/lib64/libasan.so.8+0xd92ef) (BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c)
#1 0x7fac4a4e106f in xmlStrndup (/lib64/libxml2.so.2+0xba06f) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c)
#2 0x7fac4a4842c0 in xmlNewNode (/lib64/libxml2.so.2+0x5d2c0) (BuildId: 3074681c8fa9b17e0cbed09bc61c25ada5c28e7c)
#3 0x22107a6 in xmltotext_with_options ../src/backend/utils/adt/xml.c:754
#4 0xead047 in ExecEvalXmlExpr ../src/backend/executor/execExprInterp.c:4020
#5 0xe8c119 in ExecInterpExpr ../src/backend/executor/execExprInterp.c:1537
#6 0xe91f2c in ExecInterpExprStillValid ../src/backend/executor/execExprInterp.c:1881
#7 0x109632d in ExecEvalExprSwitchContext ../src/include/executor/executor.h:355
#8 0x109655d in ExecProject ../src/include/executor/executor.h:389
#9 0x1097186 in ExecResult ../src/backend/executor/nodeResult.c:136
#10 0xf0f90c in ExecProcNodeFirst ../src/backend/executor/execProcnode.c:464
#11 0xec9bec in ExecProcNode ../src/include/executor/executor.h:273
#12 0xed875c in ExecutePlan ../src/backend/executor/execMain.c:1670
#13 0xecbee0 in standard_ExecutorRun ../src/backend/executor/execMain.c:365
#14 0xecb529 in ExecutorRun ../src/backend/executor/execMain.c:309
#15 0x1ae89f6 in PortalRunSelect ../src/backend/tcop/pquery.c:924
#16 0x1ae7c06 in PortalRun ../src/backend/tcop/pquery.c:768
#17 0x1ad1b43 in exec_simple_query ../src/backend/tcop/postgres.c:1273
#18 0x1adf8de in PostgresMain ../src/backend/tcop/postgres.c:4653
#19 0x170dbce in BackendRun ../src/backend/postmaster/postmaster.c:4464
#20 0x170bf70 in BackendStartup ../src/backend/postmaster/postmaster.c:4140
#21 0x170263f in ServerLoop ../src/backend/postmaster/postmaster.c:1776
#22 0x1701052 in PostmasterMain ../src/backend/postmaster/postmaster.c:1475
#23 0x11a5f67 in main ../src/backend/main/main.c:198
#24 0x7fac48e46149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#25 0x7fac48e4620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756)
#26 0x49c5d4 in _start (/home/tristan957/Projects/work/postgresql/build/tmp_install/home/tristan957/.opt/postgresql/bin/postgres+0x49c5d4) (BuildId: c8ca341e1303be0f9dc0b0271c55c4b9e42af89b)SUMMARY: AddressSanitizer: 133 byte(s) leaked in 2 allocation(s).
--
Tristan Partin
Neon (https://neon.tech)
I tried your v1-0002, it works at compile phase but failed to run initdb
with the following leak detected:
=================================================================
==64983==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 248 byte(s) in 1 object(s) allocated from:
#0 0x7fc7729df9cf in __interceptor_malloc
../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x55bff5480e8b in save_ps_display_args
../postgres/src/backend/utils/misc/ps_status.c:190
#2 0x55bff4a5a298 in main ../postgres/src/backend/main/main.c:90
#3 0x7fc771924249 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
Indirect leak of 19 byte(s) in 1 object(s) allocated from:
#0 0x7fc77299777b in __interceptor_strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
#1 0x55bff5480f41 in save_ps_display_args
../postgres/src/backend/utils/misc/ps_status.c:198
#2 0x55bff4a5a298 in main ../postgres/src/backend/main/main.c:90
#3 0x7fc771924249 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
I worked around by moving *new_environ* as a global variable, I think this is
false positive report so maybe this deserves a patch?
I tried to apply your v2 patch set but v2-0004 seems out of date.
--
Regards
Junwang Zhao
On Mon Sep 16, 2024 at 9:29 AM EDT, Junwang Zhao wrote:
I tried your v1-0002, it works at compile phase but failed to run initdb
with the following leak detected:=================================================================
==64983==ERROR: LeakSanitizer: detected memory leaksDirect leak of 248 byte(s) in 1 object(s) allocated from:
#0 0x7fc7729df9cf in __interceptor_malloc
../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x55bff5480e8b in save_ps_display_args
../postgres/src/backend/utils/misc/ps_status.c:190
#2 0x55bff4a5a298 in main ../postgres/src/backend/main/main.c:90
#3 0x7fc771924249 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58Indirect leak of 19 byte(s) in 1 object(s) allocated from:
#0 0x7fc77299777b in __interceptor_strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
#1 0x55bff5480f41 in save_ps_display_args
../postgres/src/backend/utils/misc/ps_status.c:198
#2 0x55bff4a5a298 in main ../postgres/src/backend/main/main.c:90
#3 0x7fc771924249 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58I worked around by moving *new_environ* as a global variable, I think this is
false positive report so maybe this deserves a patch?I tried to apply your v2 patch set but v2-0004 seems out of date.
Thanks for giving it a try Junwang. I'll rebase this, hopefully tomorrow
and give it another push.
--
Tristan Partin
Neon (https://neon.tech)