Re: Remove dependence on integer wrapping - Mailing list pgsql-hackers

From Joseph Koshakow
Subject Re: Remove dependence on integer wrapping
Date
Msg-id CAAvxfHeWy-rQpqAkc60WmcaMefi1_tMZptrzCutW8xY5Uc1LUA@mail.gmail.com
Whole thread Raw
In response to Re: Remove dependence on integer wrapping  (Alexander Lakhin <exclusion@gmail.com>)
List pgsql-hackers


On Fri, Jun 14, 2024 at 8:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
>
>    And the most interesting case to me:
>    SET temp_buffers TO 1000000000;
>
>    CREATE TEMP TABLE t(i int PRIMARY KEY);
>    INSERT INTO t VALUES(1);
>
>    #4  0x00007f385cdd37f3 in __GI_abort () at ./stdlib/abort.c:79
>    #5  0x00005620071c4f51 in __addvsi3 ()
>    #6  0x0000562007143f3c in init_htab (hashp=0x562008facb20, nelem=610070812) at dynahash.c:720
>
>    (gdb) f 6
>    #6  0x0000560915207f3c in init_htab (hashp=0x560916039930, nelem=1000000000) at dynahash.c:720
>    720             hctl->high_mask = (nbuckets << 1) - 1;
>    (gdb) p nbuckets
>    $1 = 1073741824

Alex, are you able to get a full stack trace for this panic? I'm unable
to reproduce this because I don't have enough memory in my system. I've
tried reducing `BLCKSZ` to 1024, which is the lowest value allowed per
my understanding, and I still don't have enough memory.

Here's what it looks like is happening:

1. When inserting into the table, we create a new dynamic hash table
and set `nelem` equal to `temp_buffers`, which is 1000000000.

2. `nbuckets` is then set to the the next highest power of 2 from
   `nelem`, which is 1073741824.

    /*
     * Allocate space for the next greater power of two number of buckets,
     * assuming a desired maximum load factor of 1.
     */
    nbuckets = next_pow2_int(nelem);

3. Shift `nbuckets` to the left by 1. This would equal 2147483648,
which is larger than `INT_MAX`, which causes an overflow.

    hctl->high_mask = (nbuckets << 1) - 1;

The max value allowed for `temp_buffers` is `INT_MAX / 2` (1073741823),
So any value of `temp_buffers` in the range (536870912, 1073741823]
would cause this overflow. Without `-ftrapv`, `nbuckets` would wrap
around to -2147483648, which is likely to cause all sorts of havoc, I'm
just not sure what exactly.

Also, `nbuckets = next_pow2_int(nelem);`, by itself is a bit sketchy
considering that `nelem` is a `long` and `nbuckets` is an `int`.
Potentially, the fix here is to just convert `nbuckets` to a `long`. I
plan on checking if that's feasible.

I also found this commit [0] that increased the max of `nbuckets` from
`INT_MAX / BLCKSZ` to `INT_MAX / 2`, which introduced the possibility
of this overflow. So I plan on reading through that as well.

Thanks,
Joseph Koshakow

[0] https://github.com/postgres/postgres/commit/0007490e0964d194a606ba79bb11ae1642da3372

pgsql-hackers by date:

Previous
From: Sutou Kouhei
Date:
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Next
From: Alexander Korotkov
Date:
Subject: Re: type cache cleanup improvements