pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedMake stack depth check work with asan's use-after-return
3+ messages / 2 participants
[nested] [flat]
* Make stack depth check work with asan's use-after-return
@ 2026-05-27 13:23 Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 3+ messages in thread
From: Andres Freund @ 2026-05-27 13:23 UTC (permalink / raw)
To: pgsql-hackers
Hi,
When running asan without further options against the backend, random tests
fail because the stack-depth check fails. One can work around that by putting
detect_stack_use_after_return=0 into ASAN_OPTIONS (at least with gcc, clang
still uses a lot of stack, but i think that's a separate issue).
The reason for that breakage is that with the stack-use-after-return logic,
stack variables are moved to heap allocations, to allow to detect references
to the memory at a later time. That breaks our stack-depth check, which is why
we had to disable detect_stack_use_after_return in CI. Luckily
__builtin_frame_address() works correctly, even under asan, so use that.
I think we should backpatch this. I'd be worried about using
__builtin_frame_address(), but we already do, for the base address of the
stack.
Greetings,
Andres Freund
Attachments:
[text/x-diff] v2-0001-Make-stack-depth-check-work-with-asan-s-use-after.patch (2.6K, ../../2kk4z4odvuyrg7qlwjd7ft4eron4cle4btb33v4qatgsdkayir@gj6e62rgsel4/2-v2-0001-Make-stack-depth-check-work-with-asan-s-use-after.patch)
download | inline diff:
From 86c44c934d4aa0604f82f289a0b6bd6ec0c88e31 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 7 Feb 2026 14:04:06 -0500
Subject: [PATCH v2 1/4] Make stack depth check work with asan's
use-after-return
With address sanitizer's stack-use-after-return check, stack variables are
moved to heap allocations, to allow to detect references to the memory at a
later time. That broke our stack-depth check, which is why we had to disable
detect_stack_use_after_return in CI. Luckily __builtin_frame_address() works
correctly, even under asan, so use that.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
src/backend/utils/misc/stack_depth.c | 17 ++++++++++++++++-
.cirrus.tasks.yml | 2 +-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/misc/stack_depth.c b/src/backend/utils/misc/stack_depth.c
index 61a07cf824e..9955e28c176 100644
--- a/src/backend/utils/misc/stack_depth.c
+++ b/src/backend/utils/misc/stack_depth.c
@@ -108,13 +108,28 @@ check_stack_depth(void)
bool
stack_is_too_deep(void)
{
+#ifndef HAVE__BUILTIN_FRAME_ADDRESS
char stack_top_loc;
+#endif
ssize_t stack_depth;
+ char *stack_address;
+
+ /*
+ * With address sanitizer's stack-use-after-return check, stack variables
+ * are moved to heap allocations, to allow to detect references to the
+ * memory at a later time. That would break our stack-depth check. Luckily
+ * __builtin_frame_address() works correctly, even under asan.
+ */
+#ifndef HAVE__BUILTIN_FRAME_ADDRESS
+ stack_address = &stack_top_loc;
+#else
+ stack_address = (char *) __builtin_frame_address(0);
+#endif
/*
* Compute distance from reference point to my local variables
*/
- stack_depth = (ssize_t) (stack_base_ptr - &stack_top_loc);
+ stack_depth = (ssize_t) (stack_base_ptr - stack_address);
/*
* Take abs value, since stacks grow up on some machines, down on others
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index a22cef063f3..8683d1ae9c7 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -447,7 +447,7 @@ task:
# print_stacktraces=1,verbosity=2, duh
# detect_leaks=0: too many uninteresting leak errors in short-lived binaries
UBSAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:verbosity=2
- ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0:detect_stack_use_after_return=0
+ ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
# SANITIZER_FLAGS is set in the tasks below
CFLAGS: -Og -ggdb -fno-sanitize-recover=all $SANITIZER_FLAGS
--
2.53.0.1.gb2826b52eb
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Make stack depth check work with asan's use-after-return
@ 2026-05-27 13:54 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2026-05-27 13:54 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> The reason for that breakage is that with the stack-use-after-return logic,
> stack variables are moved to heap allocations, to allow to detect references
> to the memory at a later time. That breaks our stack-depth check, which is why
> we had to disable detect_stack_use_after_return in CI. Luckily
> __builtin_frame_address() works correctly, even under asan, so use that.
> I think we should backpatch this. I'd be worried about using
> __builtin_frame_address(), but we already do, for the base address of the
> stack.
+1. It was a little weird that we adopted __builtin_frame_address()
to measure the location of the stack bottom but not the stack top.
So I think this is good cleanup even if ASAN weren't forcing it.
I might write the comments a bit differently, in particular I suggest
changing
- * Compute distance from reference point to my local variables
+ * Compute distance from reference point to my stack frame
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Make stack depth check work with asan's use-after-return
@ 2026-05-28 16:22 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 3+ messages in thread
From: Andres Freund @ 2026-05-28 16:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
Hi,
On 2026-05-27 09:54:51 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > The reason for that breakage is that with the stack-use-after-return logic,
> > stack variables are moved to heap allocations, to allow to detect references
> > to the memory at a later time. That breaks our stack-depth check, which is why
> > we had to disable detect_stack_use_after_return in CI. Luckily
> > __builtin_frame_address() works correctly, even under asan, so use that.
> > I think we should backpatch this. I'd be worried about using
> > __builtin_frame_address(), but we already do, for the base address of the
> > stack.
>
> +1. It was a little weird that we adopted __builtin_frame_address()
> to measure the location of the stack bottom but not the stack top.
> So I think this is good cleanup even if ASAN weren't forcing it.
Agreed.
> I might write the comments a bit differently, in particular I suggest
> changing
>
> - * Compute distance from reference point to my local variables
> + * Compute distance from reference point to my stack frame
Changed that, did some other minor comment polishing, and pushed the changes.
One annoying thing when backpatching is that in the older branches we still
used long. I ended up leaving that aspect as it was, but it didn't feel good
:)
Thanks for reviewing.
- Andres
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-05-28 16:22 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-27 13:23 Make stack depth check work with asan's use-after-return Andres Freund <andres@anarazel.de>
2026-05-27 13:54 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-05-28 16:22 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox