make check-world regress failed
Hi,
PostgreSQL check-world regress failed with current GIT HEAD on my Kubuntu
14.10.
uname -a
Linux vlD-kuci 3.16.0-24-generic #32-Ubuntu SMP Tue Oct 28 13:13:18 UTC
2014 i686 athlon i686 GNU/Linux
gdb -d /home/src/postgresql-devel/postgresql-git/postgresql/src -c core
...
Loaded symbols for
/home/src/postgresql-devel/dev-build/src/test/regress/regress.so
(gdb) bt
#0 0xb76ecc7c in __kernel_vsyscall ()
#1 0xb7075577 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#2 0xb7076cf3 in __GI_abort () at abort.c:89
#3 0x084c326a in ?? ()
#4 0x0a56c3b8 in ?? ()
#5 0xb76d232f in pg_atomic_init_u64 (ptr=0xbfa16fd4, val=0) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/include/port/atomics.h:445
#6 0xb76d50e4 in test_atomic_uint64 () at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1022
#7 0xb76d5756 in test_atomic_ops (fcinfo=0xa57c76c) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1114
#8 0x0825bfee in ?? ()
...
My patch solved check-world regress fail.
Best regards
Vladimir Kokovic DP senior
Attachments:
generic-gcc.diffapplication/octet-stream; name=generic-gcc.diffDownload
diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h
index ecabef3..da151a0 100644
--- a/src/include/port/atomics/generic-gcc.h
+++ b/src/include/port/atomics/generic-gcc.h
@@ -92,7 +92,8 @@
/* generic gcc based atomic uint64 implementation */
#if !defined(PG_HAVE_ATOMIC_U64_SUPPORT) \
&& !defined(PG_DISABLE_64_BIT_ATOMICS) \
- && (defined(HAVE_GCC__ATOMIC_INT64_CAS) || defined(HAVE_GCC__SYNC_INT64_CAS))
+ && (defined(HAVE_GCC__ATOMIC_INT64_CAS) || defined(HAVE_GCC__SYNC_INT64_CAS)) \
+ && (SIZEOF_VOID_P != 4)
#define PG_HAVE_ATOMIC_U64_SUPPORT
@@ -194,7 +195,7 @@
#if !defined(PG_DISABLE_64_BIT_ATOMICS)
-#if !defined(PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64) && defined(HAVE_GCC__ATOMIC_INT64_CAS)
+#if !defined(PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64) && defined(HAVE_GCC__ATOMIC_INT64_CAS) && (SIZEOF_VOID_P != 4)
#define PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64
static inline bool
pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
@@ -205,7 +206,7 @@
}
#endif
-#if !defined(PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64) && defined(HAVE_GCC__SYNC_INT64_CAS)
+#if !defined(PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64) && defined(HAVE_GCC__SYNC_INT64_CAS) && (SIZEOF_VOID_P != 4)
#define PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64
static inline bool
pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
@@ -220,7 +221,7 @@
}
#endif
-#if !defined(PG_HAVE_ATOMIC_FETCH_ADD_U64) && defined(HAVE_GCC__SYNC_INT64_CAS)
+#if !defined(PG_HAVE_ATOMIC_FETCH_ADD_U64) && defined(HAVE_GCC__SYNC_INT64_CAS) && (SIZEOF_VOID_P != 4)
#define PG_HAVE_ATOMIC_FETCH_ADD_U64
static inline uint64
pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_)
On 11/23/2014 08:37 PM, Vladimir Koković wrote:
PostgreSQL check-world regress failed with current GIT HEAD on my Kubuntu
14.10.uname -a
Linux vlD-kuci 3.16.0-24-generic #32-Ubuntu SMP Tue Oct 28 13:13:18 UTC
2014 i686 athlon i686 GNU/Linuxgdb -d /home/src/postgresql-devel/postgresql-git/postgresql/src -c core
...
Loaded symbols for
/home/src/postgresql-devel/dev-build/src/test/regress/regress.so
(gdb) bt
#0 0xb76ecc7c in __kernel_vsyscall ()
#1 0xb7075577 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#2 0xb7076cf3 in __GI_abort () at abort.c:89
#3 0x084c326a in ?? ()
#4 0x0a56c3b8 in ?? ()
#5 0xb76d232f in pg_atomic_init_u64 (ptr=0xbfa16fd4, val=0) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/include/port/atomics.h:445
#6 0xb76d50e4 in test_atomic_uint64 () at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1022
#7 0xb76d5756 in test_atomic_ops (fcinfo=0xa57c76c) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1114
#8 0x0825bfee in ?? ()
...
Andres, have you had a chance to look at this?
On 32-bit x86, arch-x86.h leaves PG_HAVE_ATOMIC_U64_SUPPORT undefined.
But generic-gcc.h, which is included later, then defines it.
pg_atomic_init_u64 does AssertPointerAlignment(ptr, 8) on the variable,
but there is no guarantee that it is 8-bytes aligned on x86.
My patch solved check-world regress fail.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-12-04 16:38:45 +0200, Heikki Linnakangas wrote:
On 11/23/2014 08:37 PM, Vladimir Koković wrote:
PostgreSQL check-world regress failed with current GIT HEAD on my Kubuntu
14.10.uname -a
Linux vlD-kuci 3.16.0-24-generic #32-Ubuntu SMP Tue Oct 28 13:13:18 UTC
2014 i686 athlon i686 GNU/Linuxgdb -d /home/src/postgresql-devel/postgresql-git/postgresql/src -c core
...
Loaded symbols for
/home/src/postgresql-devel/dev-build/src/test/regress/regress.so
(gdb) bt
#0 0xb76ecc7c in __kernel_vsyscall ()
#1 0xb7075577 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#2 0xb7076cf3 in __GI_abort () at abort.c:89
#3 0x084c326a in ?? ()
#4 0x0a56c3b8 in ?? ()
#5 0xb76d232f in pg_atomic_init_u64 (ptr=0xbfa16fd4, val=0) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/include/port/atomics.h:445
#6 0xb76d50e4 in test_atomic_uint64 () at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1022
#7 0xb76d5756 in test_atomic_ops (fcinfo=0xa57c76c) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1114
#8 0x0825bfee in ?? ()
...Andres, have you had a chance to look at this?
Nope, missed it somehow.
On 32-bit x86, arch-x86.h leaves PG_HAVE_ATOMIC_U64_SUPPORT undefined. But
generic-gcc.h, which is included later, then defines it.
That's fine. The only reason arch-x86.h implements anything itself is
that that allows older compilers than relying on intrinsics. But
implementing 64bit atomics is too annoying by hand and isn't currently
required.
pg_atomic_init_u64 does AssertPointerAlignment(ptr, 8) on the variable, but
there is no guarantee that it is 8-bytes aligned on x86.
Hrmpf. Annoying. Gcc for a while claimed that was guaranteed, but, if I
understood the tickets correctly, gave up on that.
Unfortunately we have to rely (IIRC) on that for (quite old) x86s and
some other architectures. It doesn't seem to be a problem on any native
64bit platform, because 64bit variables are 8byte aligned natively
there.
I think it can relatively easily be fixed by something like the
attached. Don't have a pure 32bit environment to test though - the
problem isn't reproducable in a 32bit chroot...
Vladimir, if you apply that patch, do things work for you?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
atomic-uint64-alignment.patchtext/x-patch; charset=us-asciiDownload
diff --git a/src/include/port/atomics/arch-x86.h b/src/include/port/atomics/arch-x86.h
index fb5623d..168a49c 100644
--- a/src/include/port/atomics/arch-x86.h
+++ b/src/include/port/atomics/arch-x86.h
@@ -73,6 +73,7 @@ typedef struct pg_atomic_uint32
#define PG_HAVE_ATOMIC_U64_SUPPORT
typedef struct pg_atomic_uint64
{
+ /* alignment guaranteed due to being on a 64bit platform */
volatile uint64 value;
} pg_atomic_uint64;
#endif
diff --git a/src/include/port/atomics/generic-acc.h b/src/include/port/atomics/generic-acc.h
index e16e282..c5639aa 100644
--- a/src/include/port/atomics/generic-acc.h
+++ b/src/include/port/atomics/generic-acc.h
@@ -40,6 +40,12 @@ typedef struct pg_atomic_uint32
#define PG_HAVE_ATOMIC_U64_SUPPORT
typedef struct pg_atomic_uint64
{
+ /*
+ * Alignment is guaranteed to be 64bit. Search for "Well-behaved
+ * application restrictions" => "Data alignment and data sharing" on HP's
+ * website. Unfortunately the URL doesn't seem to stable enough to
+ * include.
+ */
volatile uint64 value;
} pg_atomic_uint64;
diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h
index f19ad34..fea1cb5 100644
--- a/src/include/port/atomics/generic-gcc.h
+++ b/src/include/port/atomics/generic-gcc.h
@@ -98,7 +98,7 @@ typedef struct pg_atomic_uint32
typedef struct pg_atomic_uint64
{
- volatile uint64 value;
+ volatile uint64 value __attribute__((aligned(8)));
} pg_atomic_uint64;
#endif /* defined(HAVE_GCC__ATOMIC_INT64_CAS) || defined(HAVE_GCC__SYNC_INT64_CAS) */
diff --git a/src/include/port/atomics/generic-msvc.h b/src/include/port/atomics/generic-msvc.h
index 1d763ab..d259d6f 100644
--- a/src/include/port/atomics/generic-msvc.h
+++ b/src/include/port/atomics/generic-msvc.h
@@ -41,7 +41,7 @@ typedef struct pg_atomic_uint32
} pg_atomic_uint32;
#define PG_HAVE_ATOMIC_U64_SUPPORT
-typedef struct pg_atomic_uint64
+typedef struct __declspec(align(8)) pg_atomic_uint64
{
volatile uint64 value;
} pg_atomic_uint64;
diff --git a/src/include/port/atomics/generic-sunpro.h b/src/include/port/atomics/generic-sunpro.h
index b756fb9..7a3028e 100644
--- a/src/include/port/atomics/generic-sunpro.h
+++ b/src/include/port/atomics/generic-sunpro.h
@@ -55,7 +55,13 @@ typedef struct pg_atomic_uint32
#define PG_HAVE_ATOMIC_U64_SUPPORT
typedef struct pg_atomic_uint64
{
- volatile uint64 value;
+ /*
+ * Syntax to enforce variable alignment should be supported by versions
+ * supporting atomic.h, but it's hard to find accurate documentation. If
+ * it proves to be a problem, we'll have to add more version checks for 64
+ * bit support.
+ */
+ volatile uint64 value __attribute__((__aligned__(8)));
} pg_atomic_uint64;
#endif /* HAVE_ATOMIC_H */
diff --git a/src/include/port/atomics/generic-xlc.h b/src/include/port/atomics/generic-xlc.h
index 92bba6f..7a4c12a 100644
--- a/src/include/port/atomics/generic-xlc.h
+++ b/src/include/port/atomics/generic-xlc.h
@@ -32,7 +32,7 @@ typedef struct pg_atomic_uint32
#define PG_HAVE_ATOMIC_U64_SUPPORT
typedef struct pg_atomic_uint64
{
- volatile uint64 value;
+ volatile uint64 value __attribute__((__aligned__(8)));
} pg_atomic_uint64;
#endif /* __64BIT__ */
Hi,
Thanks Andres, i686 check-world passed with your
atomic-uint64-alignment.patch.
Vladimir Kokovic
Belgrade Serbia, 9.Jan 2015
------- Forwarded message -------
From: "Andres Freund" <andres@2ndquadrant.com>
To: "Heikki Linnakangas" <hlinnakangas@vmware.com>
Cc: "Vladimir Koković" <vladimir.kokovic@gmail.com>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] make check-world regress failed
Date: Thu, 08 Jan 2015 21:46:35 +0100
On 2014-12-04 16:38:45 +0200, Heikki Linnakangas wrote:
On 11/23/2014 08:37 PM, Vladimir Koković wrote:
PostgreSQL check-world regress failed with current GIT HEAD on my
Kubuntu
14.10.
uname -a
Linux vlD-kuci 3.16.0-24-generic #32-Ubuntu SMP Tue Oct 28 13:13:18 UTC
2014 i686 athlon i686 GNU/Linuxgdb -d /home/src/postgresql-devel/postgresql-git/postgresql/src -c core
...
Loaded symbols for
/home/src/postgresql-devel/dev-build/src/test/regress/regress.so
(gdb) bt
#0 0xb76ecc7c in __kernel_vsyscall ()
#1 0xb7075577 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#2 0xb7076cf3 in __GI_abort () at abort.c:89
#3 0x084c326a in ?? ()
#4 0x0a56c3b8 in ?? ()
#5 0xb76d232f in pg_atomic_init_u64 (ptr=0xbfa16fd4, val=0) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/include/port/atomics.h:445
#6 0xb76d50e4 in test_atomic_uint64 () at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1022
#7 0xb76d5756 in test_atomic_ops (fcinfo=0xa57c76c) at
/home/src/postgresql-devel/postgresql-git/postgresql/src/test/regress/regress.c:1114
#8 0x0825bfee in ?? ()
...Andres, have you had a chance to look at this?
Nope, missed it somehow.
On 32-bit x86, arch-x86.h leaves PG_HAVE_ATOMIC_U64_SUPPORT undefined.
But
generic-gcc.h, which is included later, then defines it.
That's fine. The only reason arch-x86.h implements anything itself is
that that allows older compilers than relying on intrinsics. But
implementing 64bit atomics is too annoying by hand and isn't currently
required.
pg_atomic_init_u64 does AssertPointerAlignment(ptr, 8) on the variable,
but
there is no guarantee that it is 8-bytes aligned on x86.
Hrmpf. Annoying. Gcc for a while claimed that was guaranteed, but, if I
understood the tickets correctly, gave up on that.
Unfortunately we have to rely (IIRC) on that for (quite old) x86s and
some other architectures. It doesn't seem to be a problem on any native
64bit platform, because 64bit variables are 8byte aligned natively
there.
I think it can relatively easily be fixed by something like the
attached. Don't have a pure 32bit environment to test though - the
problem isn't reproducable in a 32bit chroot...
Vladimir, if you apply that patch, do things work for you?
Greetings,
Andres Freund
--
Using Opera's revolutionary email client: http://www.opera.com/mail/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
On 2015-01-09 04:59:56 +0100, Vladimir Koković wrote:
Thanks Andres, i686 check-world passed with your
atomic-uint64-alignment.patch.
Thanks for the report and testing! I've pushed the fix.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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