openjdk team mailing list archive

[John Paul Adrian Glaubitz <glaubitz@xxxxxxxxxxxxxxxxxxx>]

On Tue, Aug 01, 2017 at 01:57:02PM +0100, Andrew Haley wrote:
> So I found two bugs in the package which stop it from building, one
> yours and one ours.  The first one is
> debian/patches/8073754-stack-overflow-9-build.diff, which sets the
> thread stack size to 2240: this is too small, and the build aborts.  I
> think this problem may be due to the use of 64k pages.

Interesting.

> NOTE THAT you should not increase the thread sizes in
> os_linux_zero.cpp: these are minimums.  Change the values in
> hotspot/src/os_cpu/linux_zero/vm/globals_linux_zero.hpp and
> common/autoconf/boot-jdk.m4 .

Ok, I will test that.

> The second one is more subtle.  Zero is so called because it uses zero
> assembly language, but this is not quite true: there is a tiny bit of
> assembly language, and it is wrong.

Yeah, I already assumed that because of the fact that the Zero build
fails on powerpc with --with-debug-level=release but not on sh4, for
example.

> Here is the PPC32 definition of
> atomic_copy64.  It uses a floating-point register to copy a 64-bit
> doubleword atomically:
> 
>   // Atomically copy 64 bits of data
>   static void atomic_copy64(volatile void *src, volatile void *dst) {
> #if defined(PPC32) && !defined(__NO_FPRS__)
>     double tmp;
>     asm volatile ("lfd  %0, 0(%1)\n"
>                   "stfd %0, 0(%2)\n"
>                   : "=f"(tmp)
>                   : "b"(src), "b"(dst));
> 
> The eagle-eyed among you might have noticed the bug: this asm has no
> memory effect.  It has no memory inputs, no memory outputs, and no
> memory clobber.  So, as far as GCC is concerned atomic_copy64 does not
> touch memory at all, and there is no need to store the source operand
> into memory.  For all GCC knows, the asm might just be doing some
> arithmetic on the pointers.  We need a better definition of
> atomic_copy64, and this is mine:
> 
>   // Atomically copy 64 bits of data
> static void atomic_copy64(volatile void *src, volatile void *dst) {
> #if defined(PPC32) && !defined(__NO_FPRS__)
>     double tmp;
>     asm volatile ("lfd  %0, %2\n"
>                   "stfd %0, %1\n"
>                   : "=&f"(tmp), "=Q"(*(volatile double*)dst)
>                   : "Q"(*(volatile double*)src));

Wow, that's indeed very subtle.

> Note that we dereference src and dst and pass the actual memory
> operands to the asm, not just pointers to them.
> 
> (This might be more detail than you need, and I'm sorry this isn't a
> real patch, but if you base a patch on what I've said here, it should
> build.  Let me know.)

Ok, I'll give it a try. Thanks a lot for digging this out!

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@xxxxxxxxxx
`. `'   Freie Universitaet Berlin - glaubitz@xxxxxxxxxxxxxxxxxxx
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913