pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alexander Lakhin <exclusion@gmail.com>
To: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering
Date: Mon, 6 Apr 2020 09:00:00 +0300
Message-ID: <e91e45fd-a614-bb87-e9fc-c556c2b95fe5@gmail.com> (raw)
In-Reply-To: <ca1f896b-e5d1-53a7-5bc3-9451ab9aca22@gmail.com>
References: <16329-7a6aa9b6fa1118a1@postgresql.org>
	<ca1f896b-e5d1-53a7-5bc3-9451ab9aca22@gmail.com>

Hello hackers,
30.03.2020 08:00, Alexander Lakhin wrote:
> 30.03.2020 00:00, PG Bug reporting form wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference:      16329
>> Logged by:          Alexander Lakhin
>> Email address:      exclusion@gmail.com
>> PostgreSQL version: 12.2
>> Operating system:   Ubuntu 18.04
>> Description:        
>>
>> Running the following query under valgrind:
>> create table gist_point_tbl(id int4, p point);
>> insert into gist_point_tbl (id, p)
>> select g, point(g*10+1, g*10+1) from generate_series(1, 30000) g;
>> create index gist_pointidx2 on gist_point_tbl using gist(p) with (buffering
>> = on, fillfactor=50);
>>
>> leads to a memory access error:
>> ==00:00:00:08.755 18585== Invalid read of size 2
>> ==00:00:00:08.756 18585==    at 0x1F94CA: gistBuildCallback
>> (gistbuild.c:495)
>>
Please look at the patch that modifies the gist regression test to make
the issue visible and fixes it by avoiding access to the memory context
that can be reset in gistProcessEmptyingQueue().

Best regards,
Alexander

Attachments:

fix-gistbuild.patch (text/x-patch, 2.8K) download diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index 671b5e9186..cee81d58ad 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -456,6 +456,10 @@ gistBuildCallback(Relation index, itup = gistFormTuple(buildstate->giststate, index, values, isnull, true); itup->t_tid = *tid; + /* Update tuple count and total size. */ + buildstate->indtuples += 1; + buildstate->indtuplesSize += IndexTupleSize(itup); + if (buildstate->bufferingMode == GIST_BUFFERING_ACTIVE) { /* We have buffers, so use them. */ @@ -471,10 +475,6 @@ gistBuildCallback(Relation index, buildstate->giststate, buildstate->heaprel, true); } - /* Update tuple count and total size. */ - buildstate->indtuples += 1; - buildstate->indtuplesSize += IndexTupleSize(itup); - MemoryContextSwitchTo(oldCtx); MemoryContextReset(buildstate->giststate->tempCxt); diff --git a/src/test/regress/expected/gist.out b/src/test/regress/expected/gist.out index 90edb4061d..8515a63c07 100644 --- a/src/test/regress/expected/gist.out +++ b/src/test/regress/expected/gist.out @@ -22,9 +22,12 @@ ERROR: value 101 out of bounds for option "fillfactor" DETAIL: Valid values are between "10" and "100". -- Insert enough data to create a tree that's a couple of levels deep. insert into gist_point_tbl (id, p) -select g, point(g*10, g*10) from generate_series(1, 10000) g; +select g, point(g*10, g*10) from generate_series(1, 20000) g; insert into gist_point_tbl (id, p) select g+100000, point(g*10+1, g*10+1) from generate_series(1, 10000) g; +-- Test buffering index build with data +create index gist_pointidx6 on gist_point_tbl using gist(p) with (buffering = on, fillfactor=50); +drop index gist_pointidx6; -- To test vacuum, delete some entries from all over the index. delete from gist_point_tbl where id % 2 = 1; -- And also delete some concentration of values. diff --git a/src/test/regress/sql/gist.sql b/src/test/regress/sql/gist.sql index b9d398ea94..0efb265ec3 100644 --- a/src/test/regress/sql/gist.sql +++ b/src/test/regress/sql/gist.sql @@ -20,11 +20,15 @@ create index gist_pointidx5 on gist_point_tbl using gist(p) with (fillfactor=101 -- Insert enough data to create a tree that's a couple of levels deep. insert into gist_point_tbl (id, p) -select g, point(g*10, g*10) from generate_series(1, 10000) g; +select g, point(g*10, g*10) from generate_series(1, 20000) g; insert into gist_point_tbl (id, p) select g+100000, point(g*10+1, g*10+1) from generate_series(1, 10000) g; +-- Test buffering index build with data +create index gist_pointidx6 on gist_point_tbl using gist(p) with (buffering = on, fillfactor=50); +drop index gist_pointidx6; + -- To test vacuum, delete some entries from all over the index. delete from gist_point_tbl where id % 2 = 1;

view thread (20+ messages)  latest in thread

Message-ID: <e91e45fd-a614-bb87-e9fc-c556c2b95fe5@gmail.com>
Permalink:  ../e91e45fd-a614-bb87-e9fc-c556c2b95fe5@gmail.com/
Also on:    postgresql.org/message-id/e91e45fd-a614-bb87-e9fc-c556c2b95fe5@gmail.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering
  In-Reply-To: <e91e45fd-a614-bb87-e9fc-c556c2b95fe5@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox