Thread: [HACKERS] valgrind vs. shared typmod registry
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers