Re: null iv parameter passed to combo_init()

From: Zhihong Yu <zyu(at)yugabyte(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: null iv parameter passed to combo_init()
Date: 2022-01-10 23:34:27
Message-ID: CALNJ-vQipwyw=WN-zvRgFnOc6PhuLmTWS-d2zmwXUS-FmcyfeA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Jan 9, 2022 at 6:45 PM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:

>
>
> On Sun, Jan 9, 2022 at 1:27 PM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:
>
>>
>>
>> On Sun, Jan 9, 2022 at 12:38 PM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:
>>
>>>
>>>
>>> On Sun, Jan 9, 2022 at 8:48 AM Noah Misch <noah(at)leadboat(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> > > Noah Misch <noah(at)leadboat(dot)com> 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 ?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-01-11 00:01:36 Re: Use -fvisibility=hidden for shared libraries
Previous Message Melanie Plageman 2022-01-10 22:50:40 Re: Avoiding smgrimmedsync() during nbtree index builds