search

battleblow / openjdk-jdk11u

BSD port of OpenJDK 11
GNU General Public License v2.0
9 stars 8 forks source link

Change type of mem_val to size_t #79

Closed MikaelUrankar closed 5 years ago

MikaelUrankar commented 5 years ago

it fixes https://github.com/battleblow/openjdk-jdk11u/issues/77

bsdkurt commented 5 years ago

Sorry for the late comment.

I believe this change would break OpenBSD & NetBSD on i386. Perhaps this line should be moved into the the #define block where it can be left at julong for Apple and Open and NetBSD.

battleblow commented 5 years ago

My bad for merging it without checking them :(. I don't think I have an i386 VM for either OpenBSD or NetBSD. Let me create one and see if I can verify this.

bsdkurt commented 5 years ago

I have one handy and can test. HW_PHYSMEM64 used on OpenBSD and NetBSD is documented as a 64 bit integer so it's pretty clear this change will be an issue with that sysctl MIB on 32 bit platforms.

battleblow commented 5 years ago

Note that a FreeBSD12/i386 fastdebug build also fails, but on a different assert:

#  Internal Error (/home/glewis/jdk11/openjdk-jdk11u/src/hotspot/cpu/x86/assembler_x86.cpp:1553), pid=42456, tid=100788
#  assert(entry == __null || is_simm32(disp)) failed: disp=0xff719c08 must be 32bit offset (call2)
bsdkurt commented 5 years ago

I confirmed that the sysctl goes through the error path on OpenBSD/i386. Note that ./src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c will also need to be adjusted to match our solution for this. I'll submit a pull request shortly with proposed changes.

MikaelUrankar commented 5 years ago

Sorry for the breakage :(

MikaelUrankar commented 5 years ago

I confirmed that the sysctl goes through the error path on OpenBSD/i386. Note that ./src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c will also need to be adjusted to match our solution for this. I'll submit a pull request shortly with proposed changes.

If you work on OperatingSystemImpl.c, can you fix this conditional please?: https://github.com/battleblow/openjdk-jdk11u/blob/bsd-port/src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c#L263

edit: should be #elif defined...

bsdkurt commented 5 years ago

No worries. Thanks for pointing out the typo. Proposed changes in #80

krytarowski commented 5 years ago

NetBSD must have uint64_t.