Re: ubsan fails on 32bit builds

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
Subject: Re: ubsan fails on 32bit builds
Date: 2022-11-17 19:20:47
Message-ID: CA+TgmoaUWPObBGgTCPD9VHOXAVUSLSLpsVTDaqrjb-GOqbZDLg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 16, 2022 at 8:42 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> Afaict the problem is that
> proc = (PGPROC *) &(waitQueue->links);
>
> is a gross gross hack - this isn't actually a PGPROC, it's pointing to an
> SHM_QUEUE, but *not* one embedded in PGPROC. It kinda works because ->links
> is at offset 0 in PGPROC, which means that
> SHMQueueInsertBefore(&(proc->links), &(MyProc->links));
> will turn &proc->links back into waitQueue->links. Which we then can enqueue
> again.

Not that I object to a targeted fix, but it's been 10 years since
slist and dlist were committed, and we really ought to eliminate
SHM_QUEUE entirely in favor of using those. It's basically an
open-coded implementation of something for which we now have a
toolkit. Not that it's impossible to make this kind of mistake with a
toolkit, but in general open-coding the same logic in multiple places
increases the risk of bugs.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-11-17 19:55:28 Re: [PoC] configurable out of disk space elog level
Previous Message Robert Haas 2022-11-17 19:12:12 Re: when the startup process doesn't (logging startup delays)