| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Make stack depth check work with asan's use-after-return |
| Date: | 2026-05-28 16:22:35 |
| Message-ID: | wm6snf5vh5c3hhduzct3lx4p4zhxps2ec4u3qthooduzqbrm3s@jcjsx2kss6qt |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On 2026-05-27 09:54:51 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Khoa Nguyen | 2026-05-28 16:42:54 | Re: Bound memory usage during manual slot sync retries |
| Previous Message | Andres Freund | 2026-05-28 16:19:15 | Re: Heads Up: cirrus-ci is shutting down June 1st |