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/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 ?? () > >... > > 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
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__ */
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers