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 1n748y-0007wj-1U for pgsql-hackers@arkaria.postgresql.org; Mon, 10 Jan 2022 23:32:04 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1n748w-00037S-Jo for pgsql-hackers@arkaria.postgresql.org; Mon, 10 Jan 2022 23:32:02 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n748w-00037G-8O for pgsql-hackers@lists.postgresql.org; Mon, 10 Jan 2022 23:32:02 +0000 Received: from mail-lf1-x12b.google.com ([2a00:1450:4864:20::12b]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1n748r-0004m2-ET for pgsql-hackers@postgresql.org; Mon, 10 Jan 2022 23:32:01 +0000 Received: by mail-lf1-x12b.google.com with SMTP id x6so50101412lfa.5 for ; Mon, 10 Jan 2022 15:31:57 -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=WKyl2LtSeB9h+JQmJqhx/05pGaYtS6RzQTlppuXfZiE=; b=UlEEdG/mT8NeawHIXVdAzAG4KesU8EP9VaDilWeG2DUFjx6OIVNSYH1EX16OhpSy9z 1FWfutvc5Jg2MdUYZEetaZImCDPSgoSmpLTNzPPzgPgZmwHCqi/X8vTko3C0gBpEOKk/ Tx5uWWzTMk7ylpuCUDNT40C0OrXMi1UZSHnAA= 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=WKyl2LtSeB9h+JQmJqhx/05pGaYtS6RzQTlppuXfZiE=; b=jZjrWQVDEA1GzxHTAghBWF0yH/sAnST8ChSjNPSpZDfoJbDytMUHypVYzgGfYVPwti vF8ibWZj6HE9briGs7aJKsAl0X7nIy7LtVKLRFXNr9hdbwmdXLDqLdBjcDhG08Z8Hgrz kJjum7R+Gs9AeKOgKmu3CW0rBlWxaLQsR/TtXoJk73WIquABoBpHZRTvLaHp/YA4NWKl mzF2URK3r/NtAMHhomOy2tQ6sQ622iD+f2tYp7xMyN0/1RZGQjjt+RrxEr9WfO5Dkapy E7guM84wCPMdfMs+BkQ+pgdFpKN3ej9aGm158wSRL32DRIdx+kyug6F/Ceys0OCtg5Im O5WQ== X-Gm-Message-State: AOAM533kKse48tEGtVOw7wCjRvBM4r6dFZRpwb6t308VteqwS6qK9Cva att0Sqe6GbdrDx9YzBZ0uNZGGrFAlYWB+qIBNw9zXxChawiqrw== X-Google-Smtp-Source: ABdhPJyFy7izp0MLnw8Pku1/l2RuiQVsUhju1c3y7I+QOtL6WJW+yZqjC1bP1wj3D//O7uYZ6c1WKteUsmS1xNB9RIk= X-Received: by 2002:a05:6512:118e:: with SMTP id g14mr1386080lfr.487.1641857515386; Mon, 10 Jan 2022 15:31:55 -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: Mon, 10 Jan 2022 15:34: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="000000000000fa108e05d542bdf6" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000fa108e05d542bdf6 Content-Type: text/plain; charset="UTF-8" On Sun, Jan 9, 2022 at 6:45 PM Zhihong Yu wrote: > > > On Sun, Jan 9, 2022 at 1:27 PM Zhihong Yu wrote: > >> >> >> 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 >> > Hi, > Earlier I was using devtoolset-11 which had an `Illegal instruction` error. > > I compiled / installed gcc-11 from source (which took whole afternoon). > `make check-world` passed with patch v3. > In tmp_install/log/install.log, I saw: > > gcc -Wall -Wmissing-prototypes -Wpointer-arith > -Wdeclaration-after-statement -Werror=vla -Wendif-labels > -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type > -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard > -Wno-format-truncation -Wno-stringop-truncation -fsanitize=undefined > -fsanitize-undefined-trap-on-error -I../../src/port -DFRONTEND > -I../../src/include -D_GNU_SOURCE -c -o path.o path.c > rm -f libpgport.a > Hi, Noah: Patch v3 passes `make check-world` Can you take another look ? --000000000000fa108e05d542bdf6 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sun, Jan 9, 2022 at 6:45 PM Zhihon= g Yu <zyu@yugabyte.com> wrote= :


On Sun, Jan 9, 2022 at 1:27 PM Zhihong Yu <zyu@yugabyte.com> w= rote:


On Sun, Jan 9, 2022 at 12:38 PM Zhihong Yu &l= t;zyu@yugabyte.com> wrote:


On Sun, Jan 9, 2022 at 8:48 AM Noah Misc= h <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
Hi,
Earlier I was using=C2=A0devtoolset-= 11 which had=C2=A0an `Illegal instruction` error.

= I compiled / installed gcc-11 from source (which took whole afternoon).
`make check-world` passed with patch v3.
In=C2=A0tmp_insta= ll/log/install.log, I saw:

gcc -Wall -Wmissing-prototyp= es -Wpointer-arith -Wdeclaration-after-statement -Werror=3Dvla -Wendif-labe= ls -Wmissing-format-attribute -Wimplicit-fallthrough=3D3 -Wcast-function-ty= pe -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=3Dstan= dard -Wno-format-truncation -Wno-stringop-truncation -fsanitize=3Dundefined= -fsanitize-undefined-trap-on-error -I../../src/port -DFRONTEND -I../../src= /include =C2=A0-D_GNU_SOURCE =C2=A0 -c -o path.o path.c
rm -f libpg= port.a=C2=A0

H= i, Noah:=C2=A0
Patch v3 passes `make check-world`

<= /div>
Can you take another look ?
--000000000000fa108e05d542bdf6--