From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: a.kozhemyakin@postgrespro.ru
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18483: Segmentation fault in tests modules
Date: Tue, 11 Jun 2024 11:00:01 +0300
Message-ID: <53085e53-7148-1707-1643-71a4969728cf@gmail.com> (raw)
In-Reply-To: <ZmeH9lDIUWTsJ-Uk@paquier.xyz>
References: <18483-30bfff42de238000@postgresql.org>
<Zl5GS6SHBXRAkl_c@paquier.xyz>
<219049.1717792376@sss.pgh.pa.us>
<ZmOJ3jfeltZQnN_M@paquier.xyz>
<439c9310-1d71-f2c6-b213-a9a2ea732ba9@gmail.com>
<336833.1717859243@sss.pgh.pa.us>
<250a21e5-d677-6b2a-2692-cd4233785e37@gmail.com>
<ZmaUKZOmjv-L9EvK@paquier.xyz>
<d959bd09-3bce-6a5e-7ae0-dff761f8ad81@gmail.com>
<ZmeH9lDIUWTsJ-Uk@paquier.xyz>
11.06.2024 02:10, Michael Paquier wrote:
> So, valgrind is noisy here because the incomplete byte sequence
> \x8b passed as input of the function after a valid byte sequence
> causes pg_encoding_mblen() to return 2, which is 1 byte passed the end
> of the actual string. report_invalid_encoding() is careful to not
> show any data passed in its input. I am not really sure that this is
> worth bothering with here, TBH. As far as I know this is related to
> the fact that we want test_enc_conversion() to be flexible enough to
> accept any kind of input, working as a cheap shortcut to call these
> internal encoding routines. What do you think?
I think that maybe this case illustrates an API shortcoming. The
report_invalid_encoding function takes len as an argument, but then passes
mbstr as a null-terminated string to pg_encoding_mblen():
/*
* report_invalid_encoding: complain about invalid multibyte character
*
* note: len is remaining length of string, not length of character;
* len must be greater than zero, as we always examine the first byte.
*/
void
report_invalid_encoding(int encoding, const char *mbstr, int len)
{
int l = pg_encoding_mblen(encoding, mbstr);
...
Maybe the comment should say something about null-termination expected?
I could not find other report_invalid_encoding() callers that pass not
null-terminated strings to it, so maybe leave test_enc_conversion() as-is,
just keep in mind that it is an imperfect example of
report_invalid_encoding() usage and it might require some modifications
for fuzz testing.
Best regards,
Alexander
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, michael@paquier.xyz, tgl@sss.pgh.pa.us, a.kozhemyakin@postgrespro.ru, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18483: Segmentation fault in tests modules
In-Reply-To: <53085e53-7148-1707-1643-71a4969728cf@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