Re: BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit overflow
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Sergey Koposov <skoposov@cmu.edu>, "pg@bowt.ie" <pg@bowt.ie>,
"pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>
Date: 2017-07-12T13:15:10Z
Lists: pgsql-bugs
On 07/06/2017 01:14 AM, Andres Freund wrote: > On 2017-07-05 18:03:56 -0400, Tom Lane wrote: >> I don't like s/int/int64/g as a fix for this. That loop is probably >> a hot spot, and this fix is going to be expensive on any machine where >> int64 isn't the native word width. How about something like this instead: >> >> - int j = 2 * i + 1; >> + int j; >> >> + if (unlikely(i > INT_MAX / 2)) >> + break; /* if j would overflow, we're done */ >> + j = 2 * i + 1; >> if (j >= n) >> break; > > Isn't an added conditional likely going to be more costly than the > s/32/64/ bit calculations on the majority of machines pg runs on? I'm > quite doubtful that it's worth catering for the few cases where that's > really slow. Another option to use "unsigned int", on the assumption that UINT_MAX >= INT_MAX * 2 + 1. And to eliminate that assumption, we can use (UINT_MAX - 1) / 2 as the maximum size of the memtuples array, rather than INT_MAX. - Heikki
Commits
-
Avoid integer overflow while sifting-up a heap in tuplesort.c.
- e439bbe9996f 9.4.13 landed
- e7213fe2bda8 9.5.8 landed
- 512f67c8d02c 10.0 landed
- 09c598898166 9.6.4 landed
-
Implement binary heap replace-top operation in a smarter way.
- 24598337c8d2 10.0 cited
-
Permit super-MaxAllocSize allocations with MemoryContextAllocHuge().
- 263865a48973 9.4.0 cited