Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n6fgX-0006PO-9Z for pgsql-hackers@arkaria.postgresql.org; Sun, 09 Jan 2022 21:25:05 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1n6fgW-0002z7-3W for pgsql-hackers@arkaria.postgresql.org; Sun, 09 Jan 2022 21:25:04 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n6fgV-0002yy-Lp for pgsql-hackers@lists.postgresql.org; Sun, 09 Jan 2022 21:25:03 +0000 Received: from mail-lf1-x133.google.com ([2a00:1450:4864:20::133]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1n6fgQ-0005n2-Q4 for pgsql-hackers@postgresql.org; Sun, 09 Jan 2022 21:25:02 +0000 Received: by mail-lf1-x133.google.com with SMTP id o12so37638949lfk.1 for ; Sun, 09 Jan 2022 13:24:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yugabyte.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=lI1bMPif6esCHY9MjQul2yXMfIjsvXOEnhXZkVldGKs=; b=aaZJod6hqxgXvkvme4HWV3mbLHxf7Z9EoPvXC1kHSgnimYJczw4MNNZimDLeLVUslC OqqQSMkOt5gwBoYEY31et099/mdZVrmJeft1dT1DvDreDzTMUUfE/dV6mt1Flc683rmu KJCdNoNb+hIVu7myqmVSUkFjmpAOOjy60rYZw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=lI1bMPif6esCHY9MjQul2yXMfIjsvXOEnhXZkVldGKs=; b=ouInfsUOZ36bQq8HMmoXprTDIbI38g4tHG5yxAiXNvrFgBin6zSdF1YkqTYt+Oanz+ fhAJzfvWLSU2o+SwDzfLmRi3/fXW+sp/7c9ihMlmBKfgEIzrBucM6oZiDseWgGL4qomO qXAdLR2aDKWEiI7tilb6m63smHlyDvI2BHudI3U/6+9Weqm1eKC0jsepHXyibfPiUv/s zpJ6T7MvVtsqt1fxSPO11ennfuIxGjTpdyC/G4R0reKlFbubw+SXAf6J9lJgo0UwVaJP nqIrGR8WwQ4TZ3Gh/1tgk5rgdHKYRCNBU05fNtuOGnXLtvH3HbMtes8O9ZAgsprzfgcy pnRQ== X-Gm-Message-State: AOAM533TYEZz0o/1IONf5pYOwYvY5nTo7Iyza6w+9JX19ksiNtx3IZKv 8/z9/PzUsId1uQaO7hJ6ZYPYfLplYA+XerQ23Kyfug== X-Google-Smtp-Source: ABdhPJzj6QBkdMjP7iH12oYt3zD5hpQV5fNRNIo0gFfLkVCGG6FLMTcarSFXk6mnyg71Kl/45daL/gGDV0ejYwa8+40= X-Received: by 2002:a05:651c:1686:: with SMTP id bd6mr49548746ljb.144.1641763497138; Sun, 09 Jan 2022 13:24:57 -0800 (PST) MIME-Version: 1.0 References: <20220109015202.GB283924@rfd.leadboat.com> <20220109031100.GC283924@rfd.leadboat.com> <1643134.1641713553@sss.pgh.pa.us> <20220109164854.GA309234@rfd.leadboat.com> In-Reply-To: From: Zhihong Yu Date: Sun, 9 Jan 2022 13:27:27 -0800 Message-ID: Subject: Re: null iv parameter passed to combo_init() To: Noah Misch Cc: Tom Lane , PostgreSQL-development Content-Type: multipart/alternative; boundary="0000000000000d6cdc05d52cda8d" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000000d6cdc05d52cda8d Content-Type: text/plain; charset="UTF-8" On Sun, Jan 9, 2022 at 12:38 PM Zhihong Yu wrote: > > > On Sun, Jan 9, 2022 at 8:48 AM Noah Misch wrote: > >> On Sun, Jan 09, 2022 at 04:37:32AM -0800, Zhihong Yu wrote: >> > On Sat, Jan 8, 2022 at 11:32 PM Tom Lane wrote: >> > > Noah Misch writes: >> > > > On further thought, I would write it this way: >> > > >> > > > - else >> > > > + else if (ivlen != 0) >> > > > memcpy(ivbuf, iv, ivlen); >> > > >> > > FWIW, I liked the "ivlen > 0" formulation better. They should be >> > > equivalent, because ivlen is unsigned, but it just seems like "> 0" >> > > is more natural. >> >> If I were considering the one code site in isolation, I'd pick "ivlen > >> 0". >> But of the four sites identified so far, three have signed length >> variables. >> Since we're likely to get more examples of this pattern, some signed and >> some >> unsigned, I'd rather use a style that does the optimal thing whether or >> not >> the variable is signed. What do you think? >> >> > Patch v4 is attached. >> >> Does this pass the test procedure shown upthread? >> > Hi, > I installed gcc 4.9.3 > > When I ran: > ./configure CFLAGS='-fsanitize=undefined > -fsanitize-undefined-trap-on-error' > > I saw: > > configure:3977: $? = 0 > configure:3966: gcc -V >&5 > gcc: error: unrecognized command line option '-V' > gcc: fatal error: no input files > compilation terminated. > configure:3977: $? = 1 > configure:3966: gcc -qversion >&5 > gcc: error: unrecognized command line option '-qversion' > gcc: fatal error: no input files > compilation terminated. > configure:3977: $? = 1 > configure:3997: checking whether the C compiler works > configure:4019: gcc -fsanitize=undefined > -fsanitize-undefined-trap-on-error conftest.c >&5 > gcc: error: unrecognized command line option > '-fsanitize-undefined-trap-on-error' > configure:4023: $? = 1 > configure:4061: result: no > > I wonder if a higher version gcc is needed. > > FYI > After installing gcc-11, ./configure passed (with 0003-memcpy-null.patch). In the output of `make check-world`, I don't see `runtime error`. Though there was a crash (maybe specific to my machine): Core was generated by `/nfusr/dev-server/zyu/postgres/tmp_install/usr/local/pgsql/bin/postgres --singl'. Program terminated with signal SIGILL, Illegal instruction. #0 0x000000000050642d in write_item.cold () Missing separate debuginfos, use: debuginfo-install glibc-2.17-325.el7_9.x86_64 nss-pam-ldapd-0.8.13-25.el7.x86_64 sssd-client-1.16.5-10.el7_9.10.x86_64 (gdb) bt #0 0x000000000050642d in write_item.cold () #1 0x0000000000ba9d1b in write_relcache_init_file () #2 0x0000000000bb58f7 in RelationCacheInitializePhase3 () #3 0x0000000000bd5cb5 in InitPostgres () #4 0x0000000000a0a9ea in PostgresMain () FYI --0000000000000d6cdc05d52cda8d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sun, Jan 9, 2022 at 12:38 PM Zhiho= ng Yu <zyu@yugabyte.com> wrot= e:


On Sun, Jan 9, 2022 at 8:48 AM Noah Misch <noah@leadboat.com>= ; wrote:
On Sun,= Jan 09, 2022 at 04:37:32AM -0800, Zhihong Yu wrote:
> On Sat, Jan 8, 2022 at 11:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Noah Misch <noah@leadboat.com> writes:
> > > On further thought, I would write it this way:
> >
> > > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else if (iv= len !=3D 0)
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0memcpy(ivbuf, iv, ivlen);
> >
> > FWIW, I liked the "ivlen > 0" formulation better.=C2= =A0 They should be
> > equivalent, because ivlen is unsigned, but it just seems like &qu= ot;> 0"
> > is more natural.

If I were considering the one code site in isolation, I'd pick "iv= len > 0".
But of the four sites identified so far, three have signed length variables= .
Since we're likely to get more examples of this pattern, some signed an= d some
unsigned, I'd rather use a style that does the optimal thing whether or= not
the variable is signed.=C2=A0 What do you think?

> Patch v4 is attached.

Does this pass the test procedure shown upthread?
Hi,<= /div>
I installed gcc 4.9.3

When I ran:
<= div>./configure CFLAGS=3D'-fsanitize=3Dundefined -fsanitize-undefined-t= rap-on-error'=C2=A0

I saw:

configure:3977: $? =3D 0
configure:3966: gcc -V >&5
gcc:= error: unrecognized command line option '-V'
gcc: fatal error: = no input files
compilation terminated.
configure:3977: $? =3D 1
co= nfigure:3966: gcc -qversion >&5
gcc: error: unrecognized command = line option '-qversion'
gcc: fatal error: no input files
comp= ilation terminated.
configure:3977: $? =3D 1
configure:3997: checking= whether the C compiler works
configure:4019: gcc -fsanitize=3Dundefined= -fsanitize-undefined-trap-on-error =C2=A0 conftest.c =C2=A0>&5
g= cc: error: unrecognized command line option '-fsanitize-undefined-trap-= on-error'
configure:4023: $? =3D 1
configure:4061: result: no
=

I wonder if a higher version gcc is needed.
=

FYI
=C2=A0
After installing gcc-11, ./configure passed (with 0003-memcpy-null.patch).=
In the output of `make check-world`, I don't see `runtime error`.
Though there was a crash (maybe specific to my machine):

Core was generated by `/nfusr/dev-server/zyu/postg= res/tmp_install/usr/local/pgsql/bin/postgres --singl'.
Program termi= nated with signal SIGILL, Illegal instruction.
#0 =C2=A00x00000000005064= 2d in write_item.cold ()
Missing separate debuginfos, use: debuginfo-ins= tall glibc-2.17-325.el7_9.x86_64 nss-pam-ldapd-0.8.13-25.el7.x86_64 sssd-cl= ient-1.16.5-10.el7_9.10.x86_64
(gdb) bt
#0 =C2=A00x000000000050642d i= n write_item.cold ()
#1 =C2=A00x0000000000ba9d1b in write_relcache_init_= file ()
#2 =C2=A00x0000000000bb58f7 in RelationCacheInitializePhase3 ()<= br>#3 =C2=A00x0000000000bd5cb5 in InitPostgres ()
#4 =C2=A00x000000= 0000a0a9ea in PostgresMain ()=C2=A0

FYI
--0000000000000d6cdc05d52cda8d--