Re: BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit overflow

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Tom Lane <tgl@sss.pgh.pa.us>, Peter Geoghegan <pg@bowt.ie>
Cc: Andres Freund <andres@anarazel.de>, Sergey Koposov <skoposov@cmu.edu>, "pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>
Date: 2017-07-12T16:20:53Z
Lists: pgsql-bugs
On 07/12/2017 07:14 PM, Tom Lane wrote:
> Peter Geoghegan <pg@bowt.ie> writes:
>> On Wed, Jul 12, 2017 at 8:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> Another option to use "unsigned int", on the assumption that UINT_MAX >=
>>>> INT_MAX * 2 + 1.
>
>>> Ah, that seems like a fine idea.
>
>> Works for me.
>
> I'll go make it so, unless Heikki's already on it?

I'm not. Thanks!

>> And to eliminate that assumption, we can use (UINT_MAX
>> - 1) / 2 as the maximum size of the memtuples array, rather than INT_MAX.
>
> Uh ... what assumption?  That's certainly true on any twos-complement
> machine.  Besides, if you're worried about hypothetical portability
> issues, ...

Right, it's a hypothetical portability issue. The assumption we're 
making is that UINT_MAX >= INT_MAX * 2 + 1. I'm not aware of any system 
where it's not true, but I don't know what the C standards say about that.

> ... I'm not sure it's any better to assume that (UINT_MAX - 1) / 2
> fits in a signed int.

Well, you could do Min(INT_MAX, (UINT_MAX - 1 / 2). Or just add a 
StaticAssertion for it. Or just note in a comment that we're making that 
assumption.

- Heikki



Commits

  1. Avoid integer overflow while sifting-up a heap in tuplesort.c.

  2. Implement binary heap replace-top operation in a smarter way.

  3. Permit super-MaxAllocSize allocations with MemoryContextAllocHuge().