Discussion:
Bug#1068873: openjdk-21: more m68k patches
(too old to reply)
Thorsten Glaser
2024-04-12 16:40:01 UTC
Permalink
Source: openjdk-21
X-Debbugs-Cc: ***@mirbsd.de, debian-***@lists.debian.org

Please add the following patch e.g. to debian/patches/m68k-support.diff
for more making implicit alignment assumptions (here by the futex
syscall) explicit:

--- src/hotspot/os/linux/waitBarrier_linux.hpp~=092024-04-12 18:24:38.58468=
6322 +0200
+++ src/hotspot/os/linux/waitBarrier_linux.hpp=092024-04-12 18:24:46.768716=
977 +0200
@@ -29,7 +29,7 @@
#include "utilities/globalDefinitions.hpp"
=20
class LinuxWaitBarrier : public CHeapObj<mtInternal> {
- volatile int _futex_barrier;
+ volatile int _futex_barrier __attribute__((__aligned__(4)));
=20
NONCOPYABLE(LinuxWaitBarrier);
=20

Thanks!

(This is what I found trying to build openjdk-20, but it=E2=80=99ll be
needed in 21 as well. Even getting to this point took 13=C2=BD days
already=E2=80=A6)
Thorsten Glaser
2024-04-12 22:50:01 UTC
Permalink
Dixi quod=E2=80=A6
Post by Thorsten Glaser
(This is what I found trying to build openjdk-20, but it=E2=80=99ll be
needed in 21 as well. Even getting to this point took 13=C2=BD days
already=E2=80=A6)
And turns out that this isn=E2=80=99t the cause.

In 17, we=E2=80=99ve got src/hotspot/share/memory/allocation.hpp to
align all CHeapObj, StackObj, MetaspaceObj, etc. classes; this
is gone in 21. So this needs to be brought back instead.
Thorsten Glaser
2024-04-13 00:00:01 UTC
Permalink
Dixi quod=E2=80=A6
Post by Thorsten Glaser
Post by Thorsten Glaser
(This is what I found trying to build openjdk-20, but it=E2=80=99ll be
needed in 21 as well. Even getting to this point took 13=C2=BD days
already=E2=80=A6)
And turns out that this isn=E2=80=99t the cause.
In 17, we=E2=80=99ve got src/hotspot/share/memory/allocation.hpp to
align all CHeapObj, StackObj, MetaspaceObj, etc. classes; this
is gone in 21. So this needs to be brought back instead.
Hmmhmm. Since I=E2=80=99m having to build/debug 20 first=E2=80=A6

=E2=80=A6 in 20, StackObj has its alignment bumped manually,
but CHeapObj doesn=E2=80=99t. MetaspaceObj does, ResourceObj
and ArenaObj don=E2=80=99t, AnyObj does.

So I=E2=80=99m guessing we will want to fix up the allocators instead?
(Though raising the alignment for cases where people allocate
them on the stack may still be useful=E2=80=A6)

ArenaObj=E2=80=A6 is not allocated=E2=80=BD

resource_allocate_bytes uses Thread::current()->resource_area()->allocate_b=
ytes
which uses Amalloc which seems to align well.

AllocateHeap uses os::malloc which uses ::malloc (C function?)
in NMT and normal cases. Huh. MallocHeader is 16 bytes, also okay.
The glibc texinfo docs say=E2=80=A6
| The address of a block returned by =E2=80=98malloc=E2=80=99 or =E2=80=98r=
ealloc=E2=80=99 in GNU systems
| is always a multiple of eight (or sixteen on 64-bit systems). If you
=E2=80=A6 so that should *also* be okay?! Unless that=E2=80=99s not true, a=
nyway=E2=80=A6

#define SIZE_SZ (sizeof (INTERNAL_SIZE_T))
#define MALLOC_ALIGNMENT (2 * SIZE_SZ < __alignof__ (long double) \
? __alignof__ (long double) : 2 * SIZE_SZ)

=E2=80=A6 it should.

So where does the unaligned _futex_barrier member in the
class LinuxWaitBarrier : public CHeapObj<mtInternal> come from?

AFAICS, the caller is:

WaitBarrier* SafepointSynchronize::_wait_barrier;
_wait_barrier =3D new WaitBarrier(vmthread);

With:

typedef LinuxWaitBarrier WaitBarrierDefault;
template <typename WaitBarrierImpl>
class WaitBarrierType : public CHeapObj<mtInternal> {
WaitBarrierImpl _impl;
=E2=80=A6
}
typedef WaitBarrierType<WaitBarrierDefault> WaitBarrier;

So the =E2=80=9Cnew WaitBarrier=E2=80=9D should call CHeapObj::operator new=
(size_t size)
TTBOMK (IANAC++Programmer) which calls CHeapObjBase::operator new(size, mtI=
nternal)
=3D AllocateHeap(size, mtInternal)=E2=80=A6

Hmmm. But, oops, I see something more:

src/hotspot/share/services/mallocTracker.hpp: static const size_t overhead=
_per_malloc =3D sizeof(MallocHeader) + sizeof(uint16_t);

That would dealign things=E2=80=A6 but MallocTracker::record_malloc
only adds sizeof(MallocHeader) and has an assert (unsure if
NDEBUG though) that checks alignment=E2=80=A6

I am lost. I can *see* an under-aligned futex barrier in strace.

19270 futex(0xcf80078a, FUTEX_WAKE_PRIVATE, 2147483647) =3D -1 EINVAL (Inva=
lid argument)

I cannot see how, though.

FWIW, /tmp/buildd/openjdk-20-20.0.2+9/build/jdk/bin/java \
-XX:NativeMemoryTracking=3Dsummary -version also crashes, same
with an explicit -XX:NativeMemoryTracking=3Doff :/

I=E2=80=99ll recompile with re-enabled alignment on the missing base
classes like we have in 17. But if someone has ideas =E2=80=99til then=E2=
=80=A6

Mraw,
//mirabilos
--=20
<igli> exceptions: a truly awful implementation of quite a nice idea.
<igli> just about the worst way you could do something like that, afaic.
<igli> it's like anti-design. <mirabilos> that too=E2=80=A6 may I quote yo=
u on that?
<igli> sure, tho i doubt anyone will listen ;)
Thorsten Glaser
2024-04-13 00:00:01 UTC
Permalink
Dixi quod=E2=80=A6
Post by Thorsten Glaser
I=E2=80=99ll recompile with re-enabled alignment on the missing base
Seems to be only one.

--- src/hotspot/share/memory/allocation.hpp~=092024-04-12 23:52:54.00000000=
0 +0000
+++ src/hotspot/share/memory/allocation.hpp=092024-04-12 23:52:56.000000000=
+0000
@@ -276,7 +276,7 @@ class CHeapObj {
void operator delete [] (void* p) {
CHeapObjBase::operator delete[](p);
}
-};
+} __attribute__ ((aligned (4)));
=20
// Base class for objects allocated on the stack only.
// Calling new or delete will result in fatal error.
Post by Thorsten Glaser
classes like we have in 17. But if someone has ideas =E2=80=99til then=E2=
=80=A6

gn8,
//mirabilos
--=20
This space for rent.
Thorsten Glaser
2024-04-15 10:50:01 UTC
Permalink
Dixi quod=E2=80=A6
Post by Thorsten Glaser
Post by Thorsten Glaser
I=E2=80=99ll recompile with re-enabled alignment on the missing base
Seems to be only one.
--- src/hotspot/share/memory/allocation.hpp~=092024-04-12 23:52:54.0000000=
00 +0000
Post by Thorsten Glaser
+++ src/hotspot/share/memory/allocation.hpp=092024-04-12 23:52:56.00000000=
0 +0000
Post by Thorsten Glaser
@@ -276,7 +276,7 @@ class CHeapObj {
void operator delete [] (void* p) {
CHeapObjBase::operator delete[](p);
}
-};
+} __attribute__ ((aligned (4)));
=20
// Base class for objects allocated on the stack only.
// Calling new or delete will result in fatal error.
Post by Thorsten Glaser
classes like we have in 17. But if someone has ideas =E2=80=99til then=E2=
=80=A6
Yes, with this, the build gets much further, so I=E2=80=99d be glad if you
could include this in the next -21 upload (and -20 if you do any
more of these, but probably not, and not necessary on my account).

Thanks,
//mirabilos
--=20
Solange man keine schmutzigen Tricks macht, und ich meine *wirklich*
schmutzige Tricks, wie bei einer doppelt verketteten Liste beide
Pointer XORen und in nur einem Word speichern, funktioniert Boehm ganz
hervorragend.=09=09-- Andreas Bogk =C3=BCber boehm-gc in d.a.s.r

Loading...