Horton

Thread

Commits

  1. Fix uninitialized variable in dshash.c.

  1. valgrind vs. shared typmod registry

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2017-09-16T12:30:19Z

    Hi,

I've been running some regression tests under valgrind, and it seems
select_parallel triggers some uses of uninitialized values in dshash. If
I'm reading the reports right, it complains about hashtable->size_log2
being not being initialized in ensure_valid_bucket_pointers.

I've been running tests under valgrind not too long ago and I don't
recall such failures, so perhaps something broke it in the past few days.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  2. Re: valgrind vs. shared typmod registry

    Thomas Munro <thomas.munro@enterprisedb.com> — 2017-09-16T19:42:55Z

    On Sun, Sep 17, 2017 at 12:30 AM, Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> I've been running some regression tests under valgrind, and it seems
> select_parallel triggers some uses of uninitialized values in dshash. If
> I'm reading the reports right, it complains about hashtable->size_log2
> being not being initialized in ensure_valid_bucket_pointers.

Thanks.  Will investigate.

-- 
Thomas Munro
http://www.enterprisedb.com

  3. Re: valgrind vs. shared typmod registry

    Peter Geoghegan <pg@bowt.ie> — 2017-09-16T19:47:21Z

    On Sat, Sep 16, 2017 at 5:30 AM, Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> I've been running tests under valgrind not too long ago and I don't
> recall such failures, so perhaps something broke it in the past few days.

That's what we have the buildfarm animal Skink for. It has indeed been
failing within select_parallel only following the commit that you
mentioned:

https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=skink&br=HEAD

-- 
Peter Geoghegan

  4. Re: valgrind vs. shared typmod registry

    Thomas Munro <thomas.munro@enterprisedb.com> — 2017-09-16T20:49:22Z

    On Sun, Sep 17, 2017 at 7:42 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Sun, Sep 17, 2017 at 12:30 AM, Tomas Vondra
> <tomas.vondra@2ndquadrant.com> wrote:
>> I've been running some regression tests under valgrind, and it seems
>> select_parallel triggers some uses of uninitialized values in dshash. If
>> I'm reading the reports right, it complains about hashtable->size_log2
>> being not being initialized in ensure_valid_bucket_pointers.
>
> Thanks.  Will investigate.

Yeah, it's a bug, I simply failed to initialise it.
ensure_valid_bucket_pointers() immediately fixes the problem (unless
the uninitialised memory had an unlikely value), explaining why it
works anyway.  I'm a bit tied up today but will test and post a patch
tomorrow.

-- 
Thomas Munro
http://www.enterprisedb.com

  5. Re: valgrind vs. shared typmod registry

    Thomas Munro <thomas.munro@enterprisedb.com> — 2017-09-18T05:39:32Z

    On Sun, Sep 17, 2017 at 8:49 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Sun, Sep 17, 2017 at 7:42 AM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>> On Sun, Sep 17, 2017 at 12:30 AM, Tomas Vondra
>> <tomas.vondra@2ndquadrant.com> wrote:
>>> I've been running some regression tests under valgrind, and it seems
>>> select_parallel triggers some uses of uninitialized values in dshash. If
>>> I'm reading the reports right, it complains about hashtable->size_log2
>>> being not being initialized in ensure_valid_bucket_pointers.
>>
>> Thanks.  Will investigate.
>
> Yeah, it's a bug, I simply failed to initialise it.
> ensure_valid_bucket_pointers() immediately fixes the problem (unless
> the uninitialised memory had an unlikely value), explaining why it
> works anyway.  I'm a bit tied up today but will test and post a patch
> tomorrow.

Here is a patch to fix that.

-- 
Thomas Munro
http://www.enterprisedb.com

  6. Re: valgrind vs. shared typmod registry

    Thomas Munro <thomas.munro@enterprisedb.com> — 2017-09-18T06:04:36Z

    On Mon, Sep 18, 2017 at 5:39 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> Here is a patch to fix that.

Here's a better one (same code, corrected commit message).

-- 
Thomas Munro
http://www.enterprisedb.com

  7. Re: valgrind vs. shared typmod registry

    Andres Freund <andres@anarazel.de> — 2017-09-19T00:56:07Z

    Hi,

On 2017-09-18 18:04:36 +1200, Thomas Munro wrote:
> On Mon, Sep 18, 2017 at 5:39 PM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
> > Here is a patch to fix that.
> 
> Here's a better one (same code, corrected commit message).

Pushed. For a second I was tempted to also replace the
palloc(sizeof(dshash_table)) with a palloc0 - but in the end it seems
actually not too bad either to be able to catch bugs like this with some
help. If you have a strong opinion either way...

Thanks,

Andres