Merging BSDPort into HotSpot mainline

Tue Aug 2 14:18:17 PDT 2011

Hello Tom,

I am a co-founding member together with Greg Lewis of the bsd-port project. I have been involved with BSD support for the JDKs for about eight years starting with 1.3 support on OpenBSD. At times I have also been heavily involved with FreeBSD support and contracted with the FreeBSD foundation multiple times to do the enginering work on certifying 1.5 and 1.6 on FreeBSD.

Thank you for the review. I will try to address some of your questions inline below.

Greg, I've cc'ed you for help with answering some questions on changes I was not familiar with.

On Tuesday 02 August 2011 08:47:39 am Tom Rodriguez wrote:
> What are the UseMembar changes about?  They are fine, I'm curious why they are needed.  I believe !UseMembar is more efficient.

In the 1.5 update time-frame Sun was working on changing UseMembar from default true to false. When I intergrated this change into FreeBSD's port we started hitting intermittant segfaults that I debugged and traced back to the UseMembar setting change. Since releasing stable certified binaries quickly was one of the goals, I reverted the UseMembar default back to true instead of taking time to find the root cause. More details can be found in the freebsd-port thread below.

http://markmail.org/message/rigdtb5heiliutec

IIRC, when I worked on porting BSD hotspot support to 1.6 I tried setting UseMembar default to off/false and it still caused intermittant segfaults. Although, I don't recall if I checked this again with OpenJDK7 on FreeBSD SMP systems.

> src/cpu/x86/vm/c1_LIRAssembler_x86.cpp:
> src/cpu/x86/vm/interp_masm_x86_32.cpp:
> 
> Why is the change of the cast needed?  That idiom is used in many other places that haven't been modified.  It's an odd little idiom that could be eliminated by fixing a limitation in the MacroAssembler where it emits larger assembly than it needs to.  Mainly I'm just unclear why it was changed in some places and not others.

On OpenBSD/i386 and I believe also OS/X/x86_32 intptr_t is typedef to long not int which leads to the following build errors:

g++ -D_ALLBSD_SOURCE -D_GNU_SOURCE -DIA32 -DPRODUCT -I. -I/home/truk/jdk/bsd-port/hotspot/src/share/vm/prims -I/home/truk/jdk/bsd-port/hotspot/src/share/vm -I/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm -I/home/truk/jdk/bsd-port/hotspot/src/os_cpu/bsd_x86/vm -I/home/truk/jdk/bsd-port/hotspot/src/os/bsd/vm -I/home/truk/jdk/bsd-port/hotspot/src/os/posix/vm -I../generated -DHOTSPOT_RELEASE_VERSION="\"21.0-b17\"" -DHOTSPOT_BUILD_TARGET="\"product\"" -DHOTSPOT_BUILD_USER="\"truk\"" -DHOTSPOT_LIB_ARCH=\"i386\" -DJRE_RELEASE_VERSION="\"1.7.0-internal-truk_2011_08_02_14_13-b00\"" -DHOTSPOT_VM_DISTRO="\"OpenJDK\"" -DDEFAULT_LIBPATH="\"/usr/lib:/usr/X11R6/lib:/usr/local/lib\"" -DTARGET_OS_FAMILY_bsd -DTARGET_ARCH_x86 -DTARGET_ARCH_MODEL_x86_32 -DTARGET_OS_ARCH_bsd_x86 -DTARGET_OS_ARCH_MODEL_bsd_x86_32 -DTARGET_COMPILER_gcc -DCOMPILER2 -DCOMPILER1 -fPIC -fno-rtti -fno-exceptions -pthread -fcheck-new -m32 -march=i586 -pipe -O3 -fno-strict-aliasing -DVM_LITTLE_ENDIAN -Werror -Wpointer-arith -Wconversion -Wsign-compare    -c -MMD -MP -MF ../generated/dependencies/c1_LIRAssembler_x86.o.d -o c1_LIRAssembler_x86.o /home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp: In member function 'int LIR_Assembler::emit_unwind_handler()':
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp:483: error: call of overloaded 'movptr(Address, int32_t)' is ambiguous
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/assembler_x86.hpp:2317: note: candidates are: void MacroAssembler::movptr(Address, intptr_t)
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/assembler_x86.hpp:2319: note:                 void MacroAssembler::movptr(Address, RegisterImpl*)
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp:484: error: call of overloaded 'movptr(Address, int32_t)' is ambiguous
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/assembler_x86.hpp:2317: note: candidates are: void MacroAssembler::movptr(Address, intptr_t)
/home/truk/jdk/bsd-port/hotspot/src/cpu/x86/vm/assembler_x86.hpp:2319: note:                 void MacroAssembler::movptr(Address, RegisterImpl*)

Hmm, reverting the src/cpu/x86/vm/interp_masm_x86_32.cpp change didn't cause a build issue on OpenBSD. We need an Apple developer to check if this one is needed on OS/X x86_32 still (note this is for 32bit build on OS/X not the 64bit build).

> 
> src/share/vm/interpreter/bytecodeTracer.cpp:
> 
> Should this just be converted to INTPTR_FORMAT and intptr_t?

That would work ok on OpenBSD, FreeBSD and OS/X where pthread_t is defined as a pointer. 

> src/share/vm/runtime/os.cpp:
> 
> This is an odd little hack.  Why is this in the JVM?

Which one?

+#if defined(_ALLBSD_SOURCE)
+  const time_t zone = (time_t) time_struct.tm_gmtoff;
+#else
   const time_t zone = timezone;
+#endif

This one is because timezone is a function on BSD.

+
+#if defined(__OpenBSD__)
+    // Work-around OpenBSD's lack of $ORIGIN support by pre-loading libnet.so
+    // ignore errors
+    dll_build_name(buffer, sizeof(buffer), Arguments::get_dll_dir(), "net");
+    dll_load(buffer, ebuf, sizeof(ebuf));
+#endif

This one is due to OpenBSD's lack of $ORIGIN support in our runtime linker. I've been meaning to add support for it, but have been short on time. Clearly it is a hack but it is needed for OpenBSD until $ORIGIN is supported.

> globalDefinitions.hpp:
> 
> INTPTR_FORMAT has been changed to a decimal format when it's supposed to be hexadecimal.  I think it should be PRIxPTR instead.
> 
> These changes might cause compilation problems on earlier releases since for instance Solaris 8 doesn't have PRI*PTR.  We might want to add something like:
> 
> #ifndef PRIxPTR
> #ifdef _LP64
> #define PRIxPTR PRIx64
> #else
> #define PRIxPTR PRIx32
> #endif

Greg added these changes. Greg, can you comment on them?

> 
> I see a full copy of elf.h in there as well which I doubt we can accept.  It seems to be apple specfic but doesn't mac use MachO?  
> 

This was brought in for Apple support. We need an Apple developer to remove it and find the places where the build is missing defines. I don't have an x86 OS/X box at the moment to do this work.

> test/Makefile:
> 
> What's the point of the JTREG_TESTDIRS change?

Not sure. Greg can you comment on this one?

Thank you for the review.

Regards,
-Kurt