From: Noah Misch <noah@leadboat.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Consolidate 'unique array values' logic into a reusable function?
Date: Sun, 29 Dec 2019 02:02:21 -0500
Message-ID: <20191229070221.GA13873@gust.leadboat.com> (raw)
In-Reply-To: <CA+hUKG+7kZ0nVrL8HeVyP+a2K64oLq921K4S4LCubGuw0JWuHw@mail.gmail.com>
References: <CAEepm=2vmFTNpAmwbGGD2WaryM6T3hSDVKQPfUwjdD_5XY6vAA@mail.gmail.com>
<22244.1470585142@sss.pgh.pa.us>
<CA+hUKGK_kwiS+3VCeMMGAKg=27T1v17ABzt+xDa1qeW7W7wruA@mail.gmail.com>
<CA+hUKGLz=6eDPewLb6ToPLcg1R3brH+5eBiWW0WVyn7+fh5y=Q@mail.gmail.com>
<CA+hUKG+7kZ0nVrL8HeVyP+a2K64oLq921K4S4LCubGuw0JWuHw@mail.gmail.com>
On Mon, Nov 04, 2019 at 12:02:21PM +1300, Thomas Munro wrote:
> Rebased. I'm planning to commit this soon.
In each installcheck-parallel run under valgrind-3.14.0, I now see ~1200
reports like this:
==00:00:00:28.322 1527557== Source and destination overlap in memcpy(0x1000104, 0x1000104, 4)
==00:00:00:28.322 1527557== at 0x4C2E74D: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1035)
==00:00:00:28.322 1527557== by 0xA9A57B: qunique (qunique.h:34)
==00:00:00:28.322 1527557== by 0xA9A843: InitCatalogCache (syscache.c:1056)
==00:00:00:28.322 1527557== by 0xAB6B18: InitPostgres (postinit.c:682)
==00:00:00:28.322 1527557== by 0x91F98E: PostgresMain (postgres.c:3909)
==00:00:00:28.322 1527557== by 0x872DE9: BackendRun (postmaster.c:4498)
==00:00:00:28.322 1527557== by 0x8725B3: BackendStartup (postmaster.c:4189)
==00:00:00:28.322 1527557== by 0x86E7F4: ServerLoop (postmaster.c:1727)
==00:00:00:28.322 1527557== by 0x86E0AA: PostmasterMain (postmaster.c:1400)
==00:00:00:28.322 1527557== by 0x77CB56: main (main.c:210)
==00:00:00:28.322 1527557==
{
<insert_a_suppression_name_here>
Memcheck:Overlap
fun:memcpy@@GLIBC_2.14
fun:qunique
fun:InitCatalogCache
fun:InitPostgres
fun:PostgresMain
fun:BackendRun
fun:BackendStartup
fun:ServerLoop
fun:PostmasterMain
fun:main
}
This is like the problem fixed in 9a9473f; the precedent from there would be
to test src!=dst before calling mempcy(), e.g. as attached. I suppose the
alternative would be to add a suppression like the one 9a9473f removed.
I do wonder why the Valgrind buildfarm animals haven't noticed.
Attachments:qunique-valgrind-v1.patch (text/x-diff, 950B) downloaddiff --git a/src/include/lib/qunique.h b/src/include/lib/qunique.hindex 4d620f8..fc539ca 100644--- a/src/include/lib/qunique.h+++ b/src/include/lib/qunique.h@@ -30,8 +30,9 @@ qunique(void *array, size_t elements, size_t width,
for (i = 1, j = 0; i < elements; ++i)
{
- if (compare(bytes + i * width, bytes + j * width) != 0)- memcpy(bytes + ++j * width, bytes + i * width, width);+ if (compare(bytes + i * width, bytes + j * width) != 0 &&+ ++j != i)+ memcpy(bytes + j * width, bytes + i * width, width);
}
return j + 1;
@@ -55,8 +56,9 @@ qunique_arg(void *array, size_t elements, size_t width,
for (i = 1, j = 0; i < elements; ++i)
{
- if (compare(bytes + i * width, bytes + j * width, arg) != 0)- memcpy(bytes + ++j * width, bytes + i * width, width);+ if (compare(bytes + i * width, bytes + j * width, arg) != 0 &&+ ++j != i)+ memcpy(bytes + j * width, bytes + i * width, width);
}
return j + 1;
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-hackers@postgresql.org
Cc: noah@leadboat.com, thomas.munro@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: Consolidate 'unique array values' logic into a reusable function?
In-Reply-To: <20191229070221.GA13873@gust.leadboat.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