openjdk team mailing list archive

[John Paul Adrian Glaubitz <glaubitz@xxxxxxxxxxxxxxxxxxx>]

Source: openjdk-9
Version: 9~b181-1
Severity: normal
Tags: patch
User: debian-arm@xxxxxxxxxxxxxxxx
Usertags: armel armhf m68k

Hi!

The patch jdk-8067331.diff is obviously broken since it patches the
functions Atomic::xchg and Atomic::xchg_ptr to not return the values
of arm_lock_test_and_set and m68k_lock_test_and_set but to store
them into the result variable which is then never returned because
the "__sync_synchronize();" and "return result" statements are
guarded by the #ifdef macros.

This is fixed by the attached version of the patch. This should
fix the Zero builds on armhf, armel and m68k. Currently, openjdk-9
fails to build from source on armel, doesn't build on m68k without
passing "--with-debug-level=slowdebug" and on armhf, Zero isn't
enabled at all.

I am currently testing the updated patch on armel, armhf and
m68k. If it succeeds, it should be included for the next upload
and the workaround for m68k from #868255 [1].

Thanks,
Adrian

> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=868255

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

--- a/hotspot/src/os_cpu/linux_zero/vm/atomic_linux_zero.hpp	2017-08-01 11:03:05.000000000 +0200
+++ b/hotspot/src/os_cpu/linux_zero/vm/atomic_linux_zero.hpp	2017-08-06 13:53:29.764594490 +0200
@@ -221,39 +221,39 @@
 
 inline jint Atomic::xchg(jint exchange_value, volatile jint* dest) {
 #ifdef ARM
-  return arm_lock_test_and_set(dest, exchange_value);
+  jint result = arm_lock_test_and_set(dest, exchange_value);
 #else
 #ifdef M68K
-  return m68k_lock_test_and_set(dest, exchange_value);
+  jint result = m68k_lock_test_and_set(dest, exchange_value);
 #else
   // __sync_lock_test_and_set is a bizarrely named atomic exchange
   // operation.  Note that some platforms only support this with the
   // limitation that the only valid value to store is the immediate
   // constant 1.  There is a test for this in JNI_CreateJavaVM().
   jint result = __sync_lock_test_and_set (dest, exchange_value);
+#endif // M68K
+#endif // ARM
   // All atomic operations are expected to be full memory barriers
   // (see atomic.hpp). However, __sync_lock_test_and_set is not
   // a full memory barrier, but an acquire barrier. Hence, this added
   // barrier.
   __sync_synchronize();
   return result;
-#endif // M68K
-#endif // ARM
 }
 
 inline intptr_t Atomic::xchg_ptr(intptr_t exchange_value,
                                  volatile intptr_t* dest) {
 #ifdef ARM
-  return arm_lock_test_and_set(dest, exchange_value);
+  intptr_t result = arm_lock_test_and_set(dest, exchange_value);
 #else
 #ifdef M68K
-  return m68k_lock_test_and_set(dest, exchange_value);
+  intptr_t result = m68k_lock_test_and_set(dest, exchange_value);
 #else
   intptr_t result = __sync_lock_test_and_set (dest, exchange_value);
-  __sync_synchronize();
-  return result;
 #endif // M68K
 #endif // ARM
+  __sync_synchronize();
+  return result;
 }
 
 inline void* Atomic::xchg_ptr(void* exchange_value, volatile void* dest) {