On 2013-07-05 14:03:56 +0200, Andres Freund wrote:
> On 2013-06-27 21:52:03 -0700, Kevin Grittner wrote:
> > Andres Freund <and...@2ndquadrant.com> wrote:
> >
> > > Hm. There were some issues with the test_logical_decoding
> > > Makefile not cleaning up the regression installation properly.
> > > Which might have caused the issue.
> > >
> > > Could you try after applying the patches and executing a clean
> > > and then rebuild?
> >
> > Tried, and problem persists.
> >
> > > Otherwise, could you try applying my git tree so we are sure we
> > > test the same thing?
> > >
> > > $ git remote add af
> > > git://git.postgresql.org/git/users/andresfreund/postgres.git
> > > $ git fetch af
> > > $ git checkout -b xlog-decoding af/xlog-decoding-rebasing-cf4
> > > $ ./configure ...
> > > $ make
> >
> > Tried that, too, and problem persists. The log shows the last
> > commit on your branch as 022c2da1873de2fbc93ae524819932719ca41bdb.
>
> Ok. I think I have a slight idea what's going on. Could you check
> whether recompiling with -O0 "fixes" the issue?
>
> There's something strange going on here, not sure whether it's just a
> bug that's hidden, by either not doing optimizations or by adding more
> elog()s, or wheter it's a compiler bug.
Ok. It was supreme stupidity on my end. Sorry for the time you spent on
it.
Some versions of gcc (and probably other compilers) were removing
sections of code when optimizing because the code was doing undefined
things. Parts of the rdata chain were allocated locally in an
if (needs_key). Which obviously is utterly bogus... A warning would have
been nice though.
Fix pushed and attached.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From ddbaa1dbf8e0283b41098f5a08a8d21d809b9a63 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Fri, 5 Jul 2013 15:07:19 +0200
Subject: [PATCH] wal_decoding: mergme: Don't use out-of-scope local variables
as part of the rdata chain
Depending on optimization level and other configuration flags removed the
sections of code doing that sinced doing so invokes undefined behaviour making
it legal for the compiler to do so.
---
src/backend/access/heap/heapam.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index f51b73f..f9f1705 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5987,9 +5987,10 @@ log_heap_update(Relation reln, Buffer oldbuf,
{
xl_heap_update xlrec;
xl_heap_header_len xlhdr;
+ xl_heap_header_len xlhdr_idx;
uint8 info;
XLogRecPtr recptr;
- XLogRecData rdata[4];
+ XLogRecData rdata[7];
Page page = BufferGetPage(newbuf);
/*
@@ -6054,40 +6055,38 @@ log_heap_update(Relation reln, Buffer oldbuf,
*/
if(need_tuple_data)
{
- XLogRecData rdata_logical[4];
-
- rdata[3].next = &(rdata_logical[0]);
+ rdata[3].next = &(rdata[4]);
- rdata_logical[0].data = NULL,
- rdata_logical[0].len = 0;
- rdata_logical[0].buffer = newbuf;
- rdata_logical[0].buffer_std = true;
- rdata_logical[0].next = NULL;
+ rdata[4].data = NULL,
+ rdata[4].len = 0;
+ rdata[4].buffer = newbuf;
+ rdata[4].buffer_std = true;
+ rdata[4].next = NULL;
xlrec.flags |= XLOG_HEAP_CONTAINS_NEW_TUPLE;
/* candidate key changed and we have a candidate key */
if (idx_tuple)
{
/* don't really need this, but its more comfy */
- xl_heap_header_len xlhdr_idx;
xlhdr_idx.header.t_infomask2 = idx_tuple->t_data->t_infomask2;
xlhdr_idx.header.t_infomask = idx_tuple->t_data->t_infomask;
xlhdr_idx.header.t_hoff = idx_tuple->t_data->t_hoff;
xlhdr_idx.t_len = idx_tuple->t_len;
- rdata_logical[0].next = &(rdata_logical[1]);
- rdata_logical[1].data = (char *) &xlhdr_idx;
- rdata_logical[1].len = SizeOfHeapHeaderLen;
- rdata_logical[1].buffer = InvalidBuffer;
- rdata_logical[1].next = &(rdata_logical[2]);
+ rdata[4].next = &(rdata[5]);
+ rdata[5].data = (char *) &xlhdr_idx;
+ rdata[5].len = SizeOfHeapHeaderLen;
+ rdata[5].buffer = InvalidBuffer;
+ rdata[5].next = &(rdata[6]);
/* PG73FORMAT: write bitmap [+ padding] [+ oid] + data */
- rdata_logical[2].data = (char *) idx_tuple->t_data
+ rdata[6].data = (char *) idx_tuple->t_data
+ offsetof(HeapTupleHeaderData, t_bits);
- rdata_logical[2].len = idx_tuple->t_len
+ rdata[6].len = idx_tuple->t_len
- offsetof(HeapTupleHeaderData, t_bits);
- rdata_logical[2].buffer = InvalidBuffer;
- rdata_logical[2].next = NULL;
+ rdata[6].buffer = InvalidBuffer;
+ rdata[6].next = NULL;
+
xlrec.flags |= XLOG_HEAP_CONTAINS_OLD_KEY;
}
}
--
1.8.2.rc2.4.g7799588.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
