Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Date: 2023-09-04T06:35:41Z
Lists: pgsql-bugs
On Sun, Aug 13, 2023 at 09:00:01PM +0300, Alexander Lakhin wrote:
> I've tried to make that addition as valuable as possible in the context of
> this issue, but as it is too expensive (to be honest, I was bolstered by
> the gin test duration (~10 secs too)), then let's leave it aside.

I have applied 0001 down to 11 to get the basic fix in place, but the
regression tests are really too expensive compared to the value they
bring. 

Regarding the changes in gtsvectorout(), the output produced is indeed
confusing when ISALLTRUE is set.

-        int            siglen = GETSIGLEN(key);
-        int            cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);
+        if (ISALLTRUE(key))
+            sprintf(outbuf, "all true bits");
+        else
+        {
+            int            siglen = GETSIGLEN(key);
+            int            cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);
 
-        sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);
+            sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);
+        }

In the false branch of ISALLTRUE(key), why isn't cnttrue always
calculated with sizebitvec()?  It's also not something I would
backpatch.  That's confusing, for sure, but there is also the argument
of keeping a consistent output in the stable branches.
--
Michael

Commits

  1. Improve description of keys in tsvector

  2. Fix out-of-bound read in gtsvector_picksplit()