pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alexander Lakhin <exclusion@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Justin Pryzby <pryzby@telsasoft.com>
To: gkokolatos@pm.me
Cc: shiy.fnst@fujitsu.com, Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
Subject: Re: Add LZ4 compression in pg_dump
Date: Fri, 5 May 2023 09:00:01 +0300
Message-ID: <94ae9bca-5ebb-1e68-bb7b-4f32e89fefbe@gmail.com> (raw)
In-Reply-To: <3beb0dd7-c927-3eca-689f-716e255d5b27@enterprisedb.com>
References: <7zu6GYFi72jByVk2SEe_nd8r1DHhwA35vSntAE9sAfiMX9Lfk6bwimkwmu7GDgYaEvL27SE4qF1uK4_LHWQM4H-SXOwkMAI-GV8PKGmED_E=@pm.me>
	<8421f53d-c9f7-c02e-3278-9f9522a8e8f3@enterprisedb.com>
	<33496f7c-3449-1426-d568-63f6bca2ac1f@gmail.com>
	<1aBKUlb8BGVUUTkRJwEmU3ud1NyeHVCoC7bpOKAuRYlHHJ7hRiDVXYgad-wmjx1K3pykd_ctyY4Lqy9xA7JGLXOm1Eul-imdZ_7TVr5LY8U=@pm.me>
	<3da84c90-7e12-75ee-a582-025bc68dd098@enterprisedb.com>
	<5qAu7vrMV3hEodlIC82b0JICWTwEUSRoIDWlIaczNBIEVVfDH2lccxHNIQ726e4nX_Qqjc4lPc5_z9RtV28NMdVv7BNiADF4JB-FDahmlVk=@pm.me>
	<d8315656-3b35-7d9a-c799-8d48fdccfe45@enterprisedb.com>
	<JduQzRIUMhF5NDvC_klfyHTDhzb5Py60xH9ICRUtluHQ4dD02nw-BH7WfX7DQG6gaFWl1Q919RvX3Xvm_5mau_0YKRQeUs6ezUSFDlFVPsE=@pm.me>
	<32ba9612-1982-4b2d-135a-5d1222987082@enterprisedb.com>
	<XPMIIorJ5munrlLRZGquartiN5tuapdjiBhaPZ6iEmJccRiKWCIHSowSLPhaGj4gLpqvaS1pQn7PGkA_WNwvuUTVdl-lrU_TzHA81LqujJ4=@pm.me>
	<ZBjg4D/X05X+JbcJ@telsasoft.com>
	<3beb0dd7-c927-3eca-689f-716e255d5b27@enterprisedb.com>

23.03.2023 20:10, Tomas Vondra wrote:
> So pushed all three parts, after updating the commit messages a bit.
>
> This leaves the empty-data issue (which we have a fix for) and the
> switch to LZ4F. And then the zstd part.
>

I'm sorry that I haven't noticed/checked that before, but when trying to
perform check-world with Valgrind I've discovered another issue presumably
related to LZ4File_gets().
When running under Valgrind:
PROVE_TESTS=t/002_pg_dump.pl make check -C src/bin/pg_dump/
I get:
...
[07:07:11.683](0.000s) ok 1939 - compression_lz4_dir: glob check for 
.../src/bin/pg_dump/tmp_check/tmp_test_HB6A/compression_lz4_dir/*.dat.lz4
# Running: pg_restore --jobs=2 --file=.../src/bin/pg_dump/tmp_check/tmp_test_HB6A/compression_lz4_dir.sql 
.../src/bin/pg_dump/tmp_check/tmp_test_HB6A/compression_lz4_dir

==00:00:00:00.579 2811926== Conditional jump or move depends on uninitialised value(s)
==00:00:00:00.579 2811926==    at 0x4853376: rawmemchr (vg_replace_strmem.c:1548)
==00:00:00:00.579 2811926==    by 0x4C96A67: _IO_str_init_static_internal (strops.c:41)
==00:00:00:00.579 2811926==    by 0x4C693A2: _IO_strfile_read (strfile.h:95)
==00:00:00:00.579 2811926==    by 0x4C693A2: __isoc99_sscanf (isoc99_sscanf.c:28)
==00:00:00:00.579 2811926==    by 0x11DB6F: _LoadLOs (pg_backup_directory.c:458)
==00:00:00:00.579 2811926==    by 0x11DD1E: _PrintTocData (pg_backup_directory.c:422)
==00:00:00:00.579 2811926==    by 0x118484: restore_toc_entry (pg_backup_archiver.c:882)
==00:00:00:00.579 2811926==    by 0x1190CC: RestoreArchive (pg_backup_archiver.c:699)
==00:00:00:00.579 2811926==    by 0x10F25D: main (pg_restore.c:414)
==00:00:00:00.579 2811926==
...

It looks like the line variable returned by gets_func() here is not
null-terminated:
     while ((CFH->gets_func(line, MAXPGPATH, CFH)) != NULL)
     {
...
         if (sscanf(line, "%u %" CppAsString2(MAXPGPATH) "s\n", &oid, lofname) != 2)
...
And Valgrind doesn't like it.

Best regards,
Alexander





view thread (205+ messages)  latest in thread

Message-ID: <94ae9bca-5ebb-1e68-bb7b-4f32e89fefbe@gmail.com>
Permalink:  ../94ae9bca-5ebb-1e68-bb7b-4f32e89fefbe@gmail.com/
Also on:    postgresql.org/message-id/94ae9bca-5ebb-1e68-bb7b-4f32e89fefbe@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-hackers@postgresql.org
  Cc: exclusion@gmail.com, tomas.vondra@enterprisedb.com, pryzby@telsasoft.com, gkokolatos@pm.me, michael@paquier.xyz, rachelmheaton@gmail.com
  Subject: Re: Add LZ4 compression in pg_dump
  In-Reply-To: <94ae9bca-5ebb-1e68-bb7b-4f32e89fefbe@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