RE: code review round 0 for minor FDS makefile fix (8033714)
Dan The changes look good Ron -Original Message- From: Daniel D. Daugherty Sent: Wednesday, February 05, 2014 4:21 PM To: hotspot-runtime-...@openjdk.java.net; serviceability-...@openjdk.java.net; build-dev; Doug Simon; Tom Rodriguez Subject: code review round 0 for minor FDS makefile fix (8033714) This code review request is going to three different aliases. Don't use Thunderbird's reply to list option since it will pick just _one_ of the _three_ lists. Greetings, Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix our way. Here are the bug and webrev URLs: http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 8033714 hotspot 'install_jvm' bld target broken with ZIP_DEBUGINFO_FILES=0 https://bugs.openjdk.java.net/browse/JDK-8033714 As you might guess from the bug synopsis, this fix is needed when building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0). Based on the Graal project fix, I've updated a few other places where building with FDS disabled is affected. As always, comments and suggestions are welcome. Dan
RE: code review round 0 for Full Debug Symbols on MacOS X hotspot (7165611)
Dan
You have my thumbs up changes look good
-Original Message-
From: David Holmes
Sent: Tuesday, October 01, 2013 8:53 PM
To: Daniel Daugherty
Cc: serviceability-...@openjdk.java.net; build-dev;
hotspot-runtime-...@openjdk.java.net
Subject: Re: code review round 0 for Full Debug Symbols on MacOS X hotspot
(7165611)
Hi Dan,
Overall thumbs up. A couple of minor issues that need fixing. A few
meta-comments (I hate
seeing all this stuff duplicated again and again.
David
-
- common/autoconf/hotspot-spec.gmk.in
Seems a good simplification.
- common/autoconf/jdk-options.m4
No comment.
---
- common/makefiles/NativeCompilation.gmk
So JDK .diz support is phase 2? :)
The Windows changes here seem okay given that on windows a .debuginfo file
should never be
in the target list.
---
- hotspot/make/Makefile
+ $(EXPORT_CLIENT_DIR)/%.dSYM:
$(MINIMAL1_BUILD_DIR)/%.dSYM
EXPORT_CLIENT_DIR should be EXPORT_MINIMAL_DIR.
For fun you can try building minimal on OSX, but I don't know how far you
will get:
--with-jvm-variants=client,server,minimal1
I'll point out obvious issues with minimal VM support anyway.
---
- hotspot/make/bsd/Makefile
No comment.
- hotspot/make/bsd/makefiles/buildtree.make
No comment.
- make/bsd/makefiles/defs.make
Note that the whole jdk6_or_earlier logic is defunct as this will never be
used for jdk6 or
earlier. But best to clean all that up at the one time.
Sadly I never found a satisfactory solution to the multiplicity and verbosity
of the FDS
messages, so OSX builds will now be afflicted by them too.
328 else
329 EXPORT_LIST += $(EXPORT_MINIMAL_DIR)/libjvm.debuginfo
330 endif
This pre-existing minimal VM code needs to be modified to reference the dSYM
directory on
OSX as per the client/server cases.
---
- hotspot/make/bsd/makefiles/dtrace.make
Note related to your changes but I don't think any of the 64 directory
stuff has any
meaning outside of Solaris.
102 dsymutil $@
I think dsymutil should be assigned to a variable in the platform defs.make
as with other
tools.
Would be nice if objcopy/dsymutil could be hidden behind a single SYM_TOOL
variables as well
so there wasn't so much duplication of the process. You could have a single
set of
instructions to invoke SYM_TOOL, STRIP and ZIP (strip would be skipped of
course because
STRIP_POLICY would never be set on osx). Just wishful thinking ...
---
- hotspot/make/bsd/makefiles/gcc.make
+ FASTDEBUG_CFLAGS += $(DEBUG_CFLAGS/$(BUILDARCH))
should be
+ FASTDEBUG_CFLAGS += $(FASTDEBUG_CFLAGS/$(BUILDARCH))
Don't we need the USE_CLANG variations here as for linux?
---
- hotspot/make/bsd/makefiles/jsig.make
- hotspot/make/bsd/makefiles/saproc.make
Similar comments to dtrace.make
---
- make/bsd/makefiles/universal.gmk
I did not understand the additional logic here:
63 # Copy built non-universal binaries in place
64 $(UNIVERSAL_COPY_LIST):
65 BUILT_COPY_FILES=`find
$(EXPORT_JRE_LIB_DIR)/{i386,amd64}/$(subst $(EXPORT_JRE_LIB_DIR)/,,$@)
2/dev/null`; \
66 if [ -n $${BUILT_COPY_FILES} ]; then \
67 for i in $${BUILT_COPY_FILES}; do \
68 if [ -f $${i} ]; then \
69 $(MKDIR) -p $(shell dirname $@); \
70 $(CP) -R $${i} $@; \
71 fi; \
72 if [ -d $${i} ]; then \
73 $(MKDIR) -p $@; \
74 fi; \
75 done; \
76 fi
until I realized that foo.dSYM is a directory not a file! Even so don't
you want to copy the contents of the directory across ?
BTW: UNIVERSAL_COPY_LIST doesn't handle minimal VM.
---
- make/bsd/makefiles/vm.make
Similar comments to dtrace.make ref dsymutil.
---
- hotspot/make/defs.make
No comment.
---
- jdk/make/common/Defs-macosx.gmk
No comment
- jdk/makefiles/Tools.gmk
Not sure about this one. Logically is seems correct but up to now this
has been defined for everything without it seeming to cause a problem.
So why do we need to change it and what impact will it have?
David
-
On 21/09/2013 1:36 PM, Daniel D. Daugherty wrote:
Greetings,
I have the initial support for Full Debug Symbols (FDS) on MacOS X done
and ready for review:
7165611 implement Full Debug Symbols on MacOS X hotspot
https://bugs.openjdk.java.net/browse/JDK-7165611
Here is the JDK8/HSX-25 webrev URL:
OpenJDK:
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/0-jdk8/
Internal:
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/0-jdk8/
This webrev includes changes for the follow repos:
jdk8
jdk8/hotspot
jdk8/jdk
jdk8/jdk/make/closed
Once these changes are approved, I'm planning to push them to
RT_Baseline. From there,
RE: RFR (L) JDK-7195622: CheckUnhandledOops has limited usefulness now
I am not an official open source code reviewer, but I did review the code and it looks good. Ron From: Lois Foltan Sent: Thursday, September 19, 2013 9:11 AM To: hotspot-runtime-...@openjdk.java.net; build-dev@openjdk.java.net Subject: RFR (L) JDK-7195622: CheckUnhandledOops has limited usefulness now Please review the following fix: Webrev: HYPERLINK http://cr.openjdk.java.net/%7Ehseigel/bug_jdk7195622.0/http://cr.openjdk.java.net/~hseigel/bug_jdk7195622.0/ Bug: JDK8 b44 hotspot:src/share/vm/oops/klass.hpp: Error:Initializing const volatile oop requires ... CheckUnhandledOops has limited usefulness now bug links at: https://bugs.openjdk.java.net/browse/JDK-7180556 https://bugs.openjdk.java.net/browse/JDK-7195622 Summary of fix: Update the C++ oop structure definition in oopsHierarchy.hpp to solve several problems with the current definition when compiled with various C++ compilers across supported platforms. These changes initially address the problem reported in JDK-7180556 and continue with additional improvements to allow CHECK_UNHANDLED_OOPS to be defined in all fastdebug builds on all platforms as suggested in JDK-7195622. Several notes concerning this fix: 1. A review should start at understanding the changes made to oopsHierarchy.hpp a. Addition of a non-volatile copy constructor to address compile time errors reported in JDK-7180556 and also currently by g++ compilers on Linux. b. Addition of member wise assignment operators to handle many instances of [non]volatile to [non]volatile oops within the JVM. Note: Solaris compilers would not allow for the member wise assignment operators of every flavor of non-volatile to volatile and vice versa. However, unlike g++ compilers, Solaris compilers had no issue passing a volatile this pointer to a non-volatile assignment operator. So the g++ compilers needed these different flavors of the assignment operator and Solaris did not. d. For similar reasons as 1b, addition of a volatile explicit conversion from oop - void *. g++ specifically complained when trying to pass a volatile this pointer. e. Removal of the ambiguous behavior of having overloaded copy constructor and explicit user conversion member functions defined of both integral and void *. All C++ compilers, except Solaris, issue a compile time error concerning this ambiguity. 2. Change #1e had the consequence of C++ compilers now generating compile time errors where the practice has been to cast an oop to and from an integral type such as intptr_t. To allow for this practice to continue, when oop is a structure and not a OopDesc *, as is the case when CHECK_UNHANDLED_OOPS is defined, two new macros were introduced within globalDefinitions.hpp, CAST_TO_OOP and CAST_FROM_OOP. 3. Due to the change in #1a #1b, the oop structure in no longer considered a POD (plain old data) by the g++ compilers on Linux and MacOS. This caused several changes to be needed throughout the JVM to add an (void *) cast of an oop when invoking print_cr(). 4. Many instances of an assignment to a volatile oop required a const_castoop to cast away the volatileness of the lvalue. There is already precedence for this type of change within utilities/taskqueue.hpp. The following comment was in place: // g++ complains if the volatile result of the assignment is // unused, so we cast the volatile away. We cannot cast directly // to void, because gcc treats that as not using the result of the // assignment. However, casting to E means that we trigger an // unused-value warning. So, we cast the E to void. 5. The addition of the volatile keyword to the GenericTaskQueue's pop_local() pop_global() member functions was to accommodate the Solaris C++ compiler's complaint about the assignment of the volatile elements of the private member data _elems when GenericTaskQueue is instantiated with a non-volatile oop. Line #723 in pop_local(). This was a result of #1b, Solaris' lack of allowing for all flavors of volatile/non-volatile member wise assignment operators. Per Liden of the GC group did pre-review this specific change with regards to performance implications and was not concerned. 6. In utilities/hashtable.cpp, required
RE: S RFR JDK-8024050: Incorrect optimization level and comment specified for unsafe.cpp
Lois I am unofficial reviewer, but the change looks good. Ron -Original Message- From: Lois Foltan Sent: Friday, August 30, 2013 11:18 AM To: hotspot-runtime-...@openjdk.java.net; build-dev@openjdk.java.net Subject: S RFR JDK-8024050: Incorrect optimization level and comment specified for unsafe.cpp Please review the following fix: open webrev at http://cr.openjdk.java.net/~hseigel/bug_jdk8024050/ Bug: bug link at https://bugs.openjdk.java.net/browse/JDK-8024050 Summary of fix: The original sources used for the JDK-8022407 webrev sponsorship contained an incorrect optimization level specification for unsafe.cpp that was fixed on the MacOS machine prior to testing. Unfortunately, this incorrect specification of -01 instead of -O1 was committed. In addition, corrected the comment for the Clang optimization level skew issue between PCH Files and files of different optimization levels. Tests: MacOS: built fastdebug product images using clang++ and llvm-g++ Ran original JDK- 8022407 test case. JTREG testing in progress. Thank you, Lois
RE: code review (round 0) request for VS2010 IDE fix (8016601)
Your code looks good and it resolves the reported defect.
-Original Message-
From: Daniel D. Daugherty
Sent: Friday, August 02, 2013 4:36 PM
To: hotspot-runtime-...@openjdk.java.net;
serviceability-...@openjdk.java.net; build-dev
Subject: code review (round 0) request for VS2010 IDE fix (8016601)
Greetings,
I have have a proposed fix for the following bug:
8016601 Unable to build hsx24 on Windows using project creator and
Visual Studio
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016601
https://jbs.oracle.com/bugs/browse/JDK-8016601
Here are the HSX-25 webrev URLs:
OpenJDK: http://cr.openjdk.java.net/~dcubed/8016601-webrev/0-hsx25/
Internal: http://javaweb.us.oracle.com/~ddaugher/8016601-webrev/0-hsx25/
Testing:
- JPRT windows_i586 and windows_x64 build and test
- local windows_i586 cmd line builds for:
{OpenJDK, Oracle} x {Client, Server} VM x {product, debug, fastdebug}
- local windows_i586 VS2010 builds for
{OpenJDK, Oracle} x {Client, Server, Tiered} VM x {product, debug,
fastdebug}
- local windows_x64 VS2010 builds for
{OpenJDK, Oracle} x {Client, Server, Tiered} VM x {product, debug,
fastdebug}
Thanks to Ron for doing the windows_x64 testing
Gory details are below. As always, comments, questions and suggestions are
welome.
Dan
Gory Details:
Build fixes:
- VS2010 IDE builds are working again; fixes this failure mode:
1D:\hotspot_src\hsx\24\hotspot_14_jun\build\vs-i486\compiler2\debug\obj
1ectCountEventSender.obj
: warning LNK4042: object specified more than once; extras ignored
1D:\hotspot_src\hsx\24\hotspot_14_jun\build\vs-i486\compiler2\debug\err
1orReporter.obj
: warning LNK4042: object specified more than once; extras ignored
1 Creating library
D:\hotspot_src\hsx\24\hotspot_14_jun\build\vs-i486\compiler2\debug\jvm.lib
and object
D:\hotspot_src\hsx\24\hotspot_14_jun\build\vs-i486\compiler2\debug\jvm.exp
1jfrRequestables.obj : error LNK2019: unresolved external symbol
public: static void __cdecl
ObjectCountEventSender::disable_requestable_event(void)
(?disable_requestable_event@ObjectCountEventSender@@SAXXZ) referenced in
function public:
virtual void __thiscall VM_GC_SendObjectCountEvent::doit(void)
(?doit@VM_GC_SendObjectCountEvent@@UAEXXZ)
1jfrRequestables.obj : error LNK2019: unresolved external symbol
public: static void __cdecl
ObjectCountEventSender::enable_requestable_event(void)
(?enable_requestable_event@ObjectCountEventSender@@SAXXZ) referenced in
function public:
virtual void __thiscall VM_GC_SendObjectCountEvent::doit(void)
(?doit@VM_GC_SendObjectCountEvent@@UAEXXZ)
1D:\hotspot_src\hsx\24\hotspot_14_jun\build\vs-i486\compiler2\debug\jvm
1.dll
: fatal error LNK1120: 2 unresolved externals
- The ProjectCreator tool is modified to support two new options:
'-relativeAltSrcInclude' and '-altRelativeInclude'. Here's an
example use of the new options:
-relativeAltSrcInclude src\closed
-altRelativeInclude share\vm
which means config the project with some alternate source files in
src\closed\share\vm that will replace the corresponding files in
src\share\vm.
For example, src\closed\share\vm\utilities\errorReporter.cpp replaces
src\share\vm\utilities\errorReporter.cpp. In the VS2010 IDE, you'll
still be able to see src\share\vm\utilities\errorReporter.cpp in the
project source browser, but the icon will indicate that the file is
excluded from the project.
The ProjectCreator tool's file tree walking logic is modified to keep
track of each alternate source file that is found. If a corresponding
regular source file is found, then the regular source file is
excluded from the project in favor of the alternate source version.
- VS2010 cmd line build no longer issue the following linker warnings:
link.exe @C:\Users\lfoltan\AppData\Local\Temp\nm9B65.tmp
errorReporter.obj : warning LNK4042: object specified more than once;
extras ignored
objectCountEventSender.obj : warning LNK4042: object specified more than
once; extras
ignored
Misc cleanups:
- removed more core config support from various makefiles and scripts;
the core config is vestigal and was mostly removed years ago; the
core config is not the same as the minimalvm config.
- removed extra references to ${ALTSRC}/share/vm/jfr objects
- added some AltSrc versus OpenJDK identification to messages where
files are auto-generated
- added some missing copyright headers
RE: HSX-25: Code Review (round 0) request for MacOS X exported symbols fix (8014326)
Dan
These changes look good.
Ron
-Original Message-
From: Daniel D. Daugherty
Sent: Tuesday, June 18, 2013 2:30 PM
To: hotspot-runtime-...@openjdk.java.net; build-dev
Subject: HSX-25: Code Review (round 0) request for MacOS X exported symbols
fix (8014326)
Greetings,
I have picked up David Holmes' work on the following bug:
8014326 [OSX] All libjvm symbols are exported
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014326
https://jbs.oracle.com/bugs/browse/JDK-8014326
Here are the HSX-25 webrev URLs:
OpenJDK: http://cr.openjdk.java.net/~dcubed/8014326-webrev/0-hsx25/
Internal: http://javaweb.us.oracle.com/~ddaugher/8014326-webrev/0-hsx25/
Testing:
- JPRT test job on MacOS X
- (in process) Aurora Adhoc vm.quick batch for MacOS X in the following
configs: {Server VM} x {fastdebug} x {-Xmixed}
Gory details are below. As always, comments, questions and suggestions are
welome.
Dan
Gory Details:
The script and Makefile changes are easy to review via the webrev.
However, every function name in the MacOS X mapfiles had to be modified to
match the MacOS X symbol
export syntax. I created normalized
copies of the mapfiles in order to compare the semantic changes (and ignore
the syntactic changes).
These diffs are added to bug report, but JBS is not yet accessible outside
Oracle and bugs.sun.com can
be slow to update so I've included the diffs here.
mapfile-vers-debug: current BSD versus updated BSD
$ diff bsd-funcs-debug{.orig,}
195a196
JVM_SetNativeThreadName
219,236d219
# Old reflection routines
# These do not need to be present in the product build in JDK 1.4 # but
their code has not been
removed yet because there will not # be a substantial code savings until
JVM_InvokeMethod and #
JVM_NewInstanceFromConstructor can also be removed; see #
reflectionCompat.hpp.
JVM_GetClassConstructor
JVM_GetClassConstructors
JVM_GetClassField
JVM_GetClassFields
JVM_GetClassMethod
JVM_GetClassMethods
JVM_GetField
JVM_GetPrimitiveField
JVM_NewInstance
JVM_SetField
JVM_SetPrimitiveField
248,250d230
fork1
numa_warn
numa_error
252,254d231
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 196: add missing JVM_SetNativeThreadName entry Delete the old reflection
routines!
Delete other functions not compiled into BSD/MacOS X.
mapfile-vers-product: current BSD versus updated BSD
$ diff bsd-funcs-product{.orig,}
195a196
JVM_SetNativeThreadName
219,236d219
# Old reflection routines
# These do not need to be present in the product build in JDK 1.4 # but
their code has not been
removed yet because there will not # be a substantial code savings until
JVM_InvokeMethod and #
JVM_NewInstanceFromConstructor can also be removed; see #
reflectionCompat.hpp.
JVM_GetClassConstructor
JVM_GetClassConstructors
JVM_GetClassField
JVM_GetClassFields
JVM_GetClassMethod
JVM_GetClassMethods
JVM_GetField
JVM_GetPrimitiveField
JVM_NewInstance
JVM_SetField
JVM_SetPrimitiveField
243,245d225
fork1
numa_warn
numa_error
247,249d226
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 196: add missing JVM_SetNativeThreadName entry Delete the old reflection
routines!
Delete other functions not compiled into BSD/MacOS X.
Since the MacOS X port was originally derived from the Linux port, it also
makes sense to compare the
updated BSD files versus the current Linux files. This is a useful sanity
check.
mapfile-vers-debug: updated BSD versus current Linux
$ diff {bsd,linux}-funcs-debug
218c218
JVM_handle_bsd_signal
---
JVM_handle_linux_signal
230a231,233
fork1
numa_warn
numa_error
231a235,237
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 218 is an obvious rename.
Lines 231-233 are functions not compiled into BSD/MacOS X.
Line 236 is an error on Linux; sysThreadAvailableStackWithSlack is only on
Solaris, but we won't fix
that with this bug since we want the MacOS X fix in HSX24 and in HSX25.
mapfile-vers-product: updated BSD versus current Linux
$ diff {bsd,linux}-funcs-product
218c218
JVM_handle_bsd_signal
---
JVM_handle_linux_signal
225a226,228
fork1
numa_warn
numa_error
226a230,232
# Needed because there is no JVM interface for this.
RE: HSX-24: Code Review (round 0) request for MacOS X exported symbols fix (8014326)
Dan
These changes look good.
Ron
-Original Message-
From: Daniel D. Daugherty
Sent: Tuesday, June 18, 2013 2:50 PM
To: hotspot-runtime-...@openjdk.java.net; build-dev
Subject: HSX-24: Code Review (round 0) request for MacOS X exported symbols
fix (8014326)
Greetings,
I have picked up David Holmes' work on the following bug:
8014326 [OSX] All libjvm symbols are exported
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014326
https://jbs.oracle.com/bugs/browse/JDK-8014326
Here are the HSX-24 backport webrev URLs:
OpenJDK: http://cr.openjdk.java.net/~dcubed/8014326-webrev/0-hsx24/
Internal: http://javaweb.us.oracle.com/~ddaugher/8014326-webrev/0-hsx24/
Testing:
- JPRT test job on MacOS X
- (in process) Aurora Adhoc vm.quick batch for MacOS X in the following
configs: {Server VM} x {fastdebug} x {-Xmixed}
Gory details are below. As always, comments, questions and suggestions are
welome.
Dan
Gory Details:
The script and Makefile changes are easy to review via the webrev.
However, every function name in the MacOS X mapfiles had to be modified to
match the MacOS X symbol
export syntax. I created normalized
copies of the mapfiles in order to compare the semantic changes (and ignore
the syntactic changes).
These diffs are added to bug report, but JBS is not yet accessible outside
Oracle and bugs.sun.com can
be slow to update so I've included the diffs here.
mapfile-vers-debug: current BSD versus updated BSD
$ diff bsd-funcs-debug{.orig,}
191a192
JVM_SetNativeThreadName
215,232d215
# Old reflection routines
# These do not need to be present in the product build in JDK 1.4 # but
their code has not been
removed yet because there will not # be a substantial code savings until
JVM_InvokeMethod and #
JVM_NewInstanceFromConstructor can also be removed; see #
reflectionCompat.hpp.
JVM_GetClassConstructor
JVM_GetClassConstructors
JVM_GetClassField
JVM_GetClassFields
JVM_GetClassMethod
JVM_GetClassMethods
JVM_GetField
JVM_GetPrimitiveField
JVM_NewInstance
JVM_SetField
JVM_SetPrimitiveField
239,241d221
fork1
numa_warn
numa_error
243,245d222
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 192: add missing JVM_SetNativeThreadName entry Delete the old reflection
routines!
Delete other functions not compiled into BSD/MacOS X.
mapfile-vers-product: current BSD versus updated BSD
$ diff bsd-funcs-product.orig bsd-funcs-product
191a192
JVM_SetNativeThreadName
215,232d215
# Old reflection routines
# These do not need to be present in the product build in JDK 1.4 # but
their code has not been
removed yet because there will not # be a substantial code savings until
JVM_InvokeMethod and #
JVM_NewInstanceFromConstructor can also be removed; see #
reflectionCompat.hpp.
JVM_GetClassConstructor
JVM_GetClassConstructors
JVM_GetClassField
JVM_GetClassFields
JVM_GetClassMethod
JVM_GetClassMethods
JVM_GetField
JVM_GetPrimitiveField
JVM_NewInstance
JVM_SetField
JVM_SetPrimitiveField
239,241d221
fork1
numa_warn
numa_error
243,245d222
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 192: add missing JVM_SetNativeThreadName entry Delete the old reflection
routines!
Delete other functions not compiled into BSD/MacOS X.
Since the MacOS X port was originally derived from the Linux port, it also
makes sense to compare the
updated BSD files versus the current Linux files. This is a useful sanity
check.
mapfile-vers-debug: updated BSD versus current Linux
$ diff {bsd,linux}-funcs-debug
214c214
JVM_handle_bsd_signal
---
JVM_handle_linux_signal
226a227,229
fork1
numa_warn
numa_error
227a231,233
# Needed because there is no JVM interface for this.
sysThreadAvailableStackWithSlack
Line 214 is an obvious rename.
Lines 227-229 are functions not compiled into BSD/MacOS X.
Line 232 is an error on Linux; sysThreadAvailableStackWithSlack is only on
Solaris, but we won't fix
that with this bug since we want the MacOS X fix in HSX24 and in HSX25.
mapfile-vers-product: updated BSD versus current Linux
$ diff {bsd,linux}-funcs-product
214c214
JVM_handle_bsd_signal
---
JVM_handle_linux_signal
221a222,224
fork1
numa_warn
numa_error
222a226,228
# Needed because there is no JVM
RE: Code Review fix for 8005044 remove crufty '_g' support from HS runtime code
Harold,
Did you have any more points or counter points to make on the use of snprintf
and strncpy ?
Ron
From: Ron Durbin
Sent: Tuesday, December 18, 2012 4:50 PM
To: Daniel Daugherty; hotspot-runtime-...@openjdk.java.net; build-dev;
serviceability-...@openjdk.java.net
Subject: RE: Code Review fix for 8005044 remove crufty '_g' support from HS
runtime code
Thanks for the quick reviews by all.
I will agree with Dan on this one, sprint will stay.
Ron
From: Daniel D. Daugherty
Sent: Tuesday, December 18, 2012 4:03 PM
To: HYPERLINK
mailto:hotspot-runtime-...@openjdk.java.nethotspot-runtime-...@openjdk.java.net;
build-dev; HYPERLINK
mailto:serviceability-...@openjdk.java.netserviceability-...@openjdk.java.net
Subject: Re: Code Review fix for 8005044 remove crufty '_g' support from HS
runtime code
Sorry, I lost my manners somewhere... :-(
Thanks for the fast review!
Dan
On 12/18/12 3:57 PM, Daniel D. Daugherty wrote:
Adding back in the other aliases...
Harold,
The equivalent of:
snprintf(buf, n, str);
is:
strncpy(buf, str, n - 1);
buf[n - 1] = '\0';
because snprintf() will only write out (n - 1) bytes from
'str' to 'buf' and then write a NUL in the last slot.
strncpy() copies up to 'n' bytes from 'str' to 'buf'.
strncpy() will not copy past a NUL character in 'str', but
it will also not NUL terminate 'buf' if a NUL is not found
in 'str' before the limit 'n' is reached.
Dan
On 12/18/12 2:48 PM, harold seigel wrote:
Hi Ron,
I think these calls to snprintf() in os_solaris.cpp and the other os_cpp
files could be replaced with calls to strncpy(). It might be a little simpler.
Otherwise, it looks good.
Thanks, Harold
2539 if (0 == access(buf, F_OK)) {
2540 // Use current module name libjvm.so
2541 len = strlen(buf);
2542 snprintf(buf + len, buflen-len, /hotspot/libjvm.so);
2543 } else {
2544 // Go back to path of .so
2545 realpath((char *)dlinfo.dli_fname, buf); 2546 }
On 12/18/2012 3:46 PM, Daniel D. Daugherty wrote:
Greetings,
I'm sponsoring this code review request from Ron Durbin. This change
is targeted at JDK8/HSX-25 in the RT_Baseline repo.
Dan
Sending again with correct subject line, bug URLs and webrev URL.
Intro:
This set of changes removes the runtime support for generation of debug
versions that follow _g semantics.
Defect:
JDK-8005044 remove crufty '_g' support from HS runtime code
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005044
https://jbs.oracle.com/bugs/browse/JDK-8005044
Webrev
HYPERLINK
http://cr.openjdk.java.net/%7Edcubed/for_rdurbin/8005044-webrev/0http://cr.openjdk.java.net/~dcubed/for_rdurbin/8005044-webrev/0
Details:
Files have been modified to remove all reference and support for debug
versions that follow _g semantics.
Testing:
Passed JPRT last night:
Additional Testing In process: (suggested by Dan):
src/share/vm/runtime/arguments.cpp
- test with shared archive creation and use; see the e-mail
from Coleen
src/share/tools/ProjectCreator/ProjectCreator.java
- just a usage message; visual inspection of the code
src/os/windows/vm/os_windows.cpp
- comments only; no testing needed
src/os/{bsd,linux,solaris}/vm/os_{bsd,linux,solaris}.cpp
- the only code changes come into play when the gamma
launcher is used
- and when JAVA_HOME refers to a valid JDK, the function
fakes up a JVM path so that callers using the JVM path
to find other things in the JDK will work.
- I can't find any way that the actual JVM path value
that is returned is exposed
- I don't see a way to test this other than have a debug
printf() or manual code inspection.
RE: Code Review fix for 8005044 remove crufty '_g' support from HS runtime code
it looks like
1737 // libjvm.so is installed there (append a fake suffix
1738 // hotspot/libjvm.so).
1739 char* java_home_var = ::getenv(JAVA_HOME);
1740 if (java_home_var != NULL java_home_var[0] != 0) {
1741
1742 strncpy(buf, java_home_var, buflen);
1743
1744 // determine if this is a legacy image or modules image
1745 // modules image doesn't have jre subdirectory
1746 size_t len = strlen(buf);
1747 char* jrebin_p = buf + len;
1748 jio_snprintf(jrebin_p, buflen-len, HYPERLINK
file:///\\jre\bin\\\jre\\bin\\);
1749 if (0 != _access(buf, 0)) {
1750 jio_snprintf(jrebin_p, buflen-len, HYPERLINK
file:///\\bin\\\bin\\);
1751 }
1752 len = strlen(buf);
1753 jio_snprintf(buf + len, buflen-len, hotspot\\jvm.dll);
1754 }
1755 }
Linux, Solaris and Windows all recognize that if they
get to the point where they are appending a hotspot/...
path to the jvm_path value, then they are making a fake
path. Why are they making a fake path? Well, the gamma
launcher lets someone use a libjvm.so/jvm.dll that is
not located in the usual place in a JDK. And, you get
to use that libjvm.so/jvm.dll WITH THAT JDK. Sounds
pretty cool and it is pretty cool... until you realize
that there is code in the VM that tries to find other
libraries _relative_ to the libjvm.so/jvm.dll that is
running. Ouch! So enter the above code blocks that fake
a path to the libjvm.so/jvm.dll so that callers to
os::jvm_path() don't get confused.
To make matters more convoluted... yes, I know this is
convoluted enough... The os_bsd.cpp doesn't seem to
realize that it is creating a fake path here. At one
point, the os_bsd.cpp code was a copy of os_linux.cpp
code, but it was obviously tweaked. It will take more
investigation to figure out the right course of action
for os_bsd.cpp
Dan
On 12/19/12 7:51 AM, harold seigel wrote:
Hi Ron,
I'm ok with using snprinft().
Another nit: Is it worth checking that all of /hotspot/libjvm.so could get
copied to buf? (i.e.: that buflen-len strlen(/hotspot/libjvm.so)
Thanks, Harold
On 12/19/2012 9:17 AM, Ron Durbin wrote:
Harold,
Did you have any more points or counter points to make on the use of snprintf
and strncpy ?
Ron
From: Ron Durbin
Sent: Tuesday, December 18, 2012 4:50 PM
To: Daniel Daugherty; HYPERLINK
mailto:hotspot-runtime-...@openjdk.java.nethotspot-runtime-...@openjdk.java.net;
build-dev; HYPERLINK
mailto:serviceability-...@openjdk.java.netserviceability-...@openjdk.java.net
Subject: RE: Code Review fix for 8005044 remove crufty '_g' support from HS
runtime code
Thanks for the quick reviews by all.
I will agree with Dan on this one, sprint will stay.
Ron
From: Daniel D. Daugherty
Sent: Tuesday, December 18, 2012 4:03 PM
To: HYPERLINK
mailto:hotspot-runtime-...@openjdk.java.nethotspot-runtime-...@openjdk.java.net;
build-dev; HYPERLINK
mailto:serviceability-...@openjdk.java.netserviceability-...@openjdk.java.net
Subject: Re: Code Review fix for 8005044 remove crufty '_g' support from HS
runtime code
Sorry, I lost my manners somewhere... :-(
Thanks for the fast review!
Dan
On 12/18/12 3:57 PM, Daniel D. Daugherty wrote:
Adding back in the other aliases...
Harold,
The equivalent of:
snprintf(buf, n, str);
is:
strncpy(buf, str, n - 1);
buf[n - 1] = '\0';
because snprintf() will only write out (n - 1) bytes from
'str' to 'buf' and then write a NUL in the last slot.
strncpy() copies up to 'n' bytes from 'str' to 'buf'.
strncpy() will not copy past a NUL character in 'str', but
it will also not NUL terminate 'buf' if a NUL is not found
in 'str' before the limit 'n' is reached.
Dan
On 12/18/12 2:48 PM, harold seigel wrote:
Hi Ron,
I think these calls to snprintf() in os_solaris.cpp and the other os_cpp
files could be replaced with calls to strncpy(). It might be a little simpler.
Otherwise, it looks good.
Thanks, Harold
2539 if (0 == access(buf, F_OK)) {
2540 // Use current module name libjvm.so
2541 len = strlen(buf);
2542 snprintf(buf + len, buflen-len, /hotspot/libjvm.so);
2543 } else {
2544 // Go back to path of .so
2545 realpath((char *)dlinfo.dli_fname, buf); 2546 }
On 12/18/2012 3:46 PM, Daniel D. Daugherty wrote:
Greetings,
I'm sponsoring this code review request from Ron Durbin. This change
is targeted at JDK8/HSX-25 in the RT_Baseline repo.
Dan
Sending again with correct subject line, bug URLs and webrev URL.
Intro:
This set of changes removes the runtime support for generation of debug
versions that follow _g semantics.
Defect:
JDK-8005044 remove crufty '_g' support from HS runtime code
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005044
https://jbs.oracle.com/bugs/browse/JDK-8005044
Webrev
HYPERLINK
http://cr.openjdk.java.net/%7Edcubed/for_rdurbin/8005044-webrev/0http://cr.openjdk.java.net/~dcubed/for_rdurbin
FW: Code Review fix for 7153050 ?remove crufty '_g' support from HotSpot run time
Intro:
This set of changes removes the runtime support for generation of debug
versions that follow _g semantics.
Defect:
JDK-8005044 remove crufty '_g' support from HS runtime code
Webrev
/u/rdurbin/8005044_exp/webrev/index.html
Details:
Files have been modified to remove all reference and support for debug
versions that follow _g semantics.
Testing:
Passed JPRT last night:
Additional Testing In process: ( suggested by Dan):
src/share/vm/runtime/arguments.cpp
- test with shared archive creation and use; see the e-mail
from Coleen
src/share/tools/ProjectCreator/ProjectCreator.java
- just a usage message; visual inspection of the code
src/os/windows/vm/os_windows.cpp
- comments only; no testing needed
src/os/{bsd,linux,solaris}/vm/os_{bsd,linux,solaris}.cpp
- the only code changes come into play when the gamma
launcher is used
- and when JAVA_HOME refers to a valid JDK, the function
fakes up a JVM path so that callers using the JVM path
to find other things in the JDK will work.
- I can't find any way that the actual JVM path value
that is returned is exposed
- I don't see a way to test this other than have a debug
printf() or manual code inspection.
RE: Code Review fix for 8005044 remove crufty '_g' support from HS runtime code
Dan is correct.
-Original Message-
From: Daniel D. Daugherty
Sent: Tuesday, December 18, 2012 2:14 PM
To: Kelly O'Hair
Cc: serviceability-...@openjdk.java.net; build-dev;
hotspot-runtime-...@openjdk.java.net
Subject: Re: Code Review fix for 8005044 remove crufty '_g' support from HS
runtime code
Kelly,
The Makefile changes were reviewed under 7153050 last week and pushed to
RT_Baseline.
See the attached notification.
Dan
On 12/18/12 2:06 PM, Kelly O'Hair wrote:
I don't see any makefile changes.
-kto
On Dec 18, 2012, at 12:46 PM, Daniel D. Daugherty wrote:
Greetings,
I'm sponsoring this code review request from Ron Durbin. This change
is targeted at JDK8/HSX-25 in the RT_Baseline repo.
Dan
Sending again with correct subject line, bug URLs and webrev URL.
Intro:
This set of changes removes the runtimesupport for generation of debug
versions that follow _g semantics.
Defect:
JDK-8005044 remove crufty '_g' support from HS runtime code
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005044
https://jbs.oracle.com/bugs/browse/JDK-8005044
Webrev
http://cr.openjdk.java.net/~dcubed/for_rdurbin/8005044-webrev/0
Details:
Files have been modified to remove all reference and support for debug
versions that follow _g semantics.
Testing:
Passed JPRT last night:
Additional Testing In process: (suggested by Dan):
src/share/vm/runtime/arguments.cpp
- test with shared archive creation and use; see the e-mail
from Coleen
src/share/tools/ProjectCreator/ProjectCreator.java
- just a usage message; visual inspection of the code
src/os/windows/vm/os_windows.cpp
- comments only; no testing needed
src/os/{bsd,linux,solaris}/vm/os_{bsd,linux,solaris}.cpp
- the only code changes come into play when the gamma
launcher is used
- and when JAVA_HOME refers to a valid JDK, the function
fakes up a JVM path so that callers using the JVM path
to find other things in the JDK will work.
- I can't find any way that the actual JVM path value
that is returned is exposed
- I don't see a way to test this other than have a debug
printf() or manual code inspection.
RE: Code Review fix for 8005044 remove crufty '_g' support from HS runtime code
Thanks for the quick reviews by all.
I will agree with Dan on this one, sprint will stay.
Ron
From: Daniel D. Daugherty
Sent: Tuesday, December 18, 2012 4:03 PM
To: hotspot-runtime-...@openjdk.java.net; build-dev;
serviceability-...@openjdk.java.net
Subject: Re: Code Review fix for 8005044 remove crufty '_g' support from HS
runtime code
Sorry, I lost my manners somewhere... :-(
Thanks for the fast review!
Dan
On 12/18/12 3:57 PM, Daniel D. Daugherty wrote:
Adding back in the other aliases...
Harold,
The equivalent of:
snprintf(buf, n, str);
is:
strncpy(buf, str, n - 1);
buf[n - 1] = '\0';
because snprintf() will only write out (n - 1) bytes from
'str' to 'buf' and then write a NUL in the last slot.
strncpy() copies up to 'n' bytes from 'str' to 'buf'.
strncpy() will not copy past a NUL character in 'str', but
it will also not NUL terminate 'buf' if a NUL is not found
in 'str' before the limit 'n' is reached.
Dan
On 12/18/12 2:48 PM, harold seigel wrote:
Hi Ron,
I think these calls to snprintf() in os_solaris.cpp and the other os_cpp
files could be replaced with calls to strncpy(). It might be a little simpler.
Otherwise, it looks good.
Thanks, Harold
2539 if (0 == access(buf, F_OK)) {
2540 // Use current module name libjvm.so
2541 len = strlen(buf);
2542 snprintf(buf + len, buflen-len, /hotspot/libjvm.so);
2543 } else {
2544 // Go back to path of .so
2545 realpath((char *)dlinfo.dli_fname, buf); 2546 }
On 12/18/2012 3:46 PM, Daniel D. Daugherty wrote:
Greetings,
I'm sponsoring this code review request from Ron Durbin. This change
is targeted at JDK8/HSX-25 in the RT_Baseline repo.
Dan
Sending again with correct subject line, bug URLs and webrev URL.
Intro:
This set of changes removes the runtime support for generation of debug
versions that follow _g semantics.
Defect:
JDK-8005044 remove crufty '_g' support from HS runtime code
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005044
https://jbs.oracle.com/bugs/browse/JDK-8005044
Webrev
HYPERLINK
http://cr.openjdk.java.net/%7Edcubed/for_rdurbin/8005044-webrev/0http://cr.openjdk.java.net/~dcubed/for_rdurbin/8005044-webrev/0
Details:
Files have been modified to remove all reference and support for debug
versions that follow _g semantics.
Testing:
Passed JPRT last night:
Additional Testing In process: (suggested by Dan):
src/share/vm/runtime/arguments.cpp
- test with shared archive creation and use; see the e-mail
from Coleen
src/share/tools/ProjectCreator/ProjectCreator.java
- just a usage message; visual inspection of the code
src/os/windows/vm/os_windows.cpp
- comments only; no testing needed
src/os/{bsd,linux,solaris}/vm/os_{bsd,linux,solaris}.cpp
- the only code changes come into play when the gamma
launcher is used
- and when JAVA_HOME refers to a valid JDK, the function
fakes up a JVM path so that callers using the JVM path
to find other things in the JDK will work.
- I can't find any way that the actual JVM path value
that is returned is exposed
- I don't see a way to test this other than have a debug
printf() or manual code inspection.
RE: code review request for 7153050 remove crufty '_g' support
Thx again -Original Message- From: Daniel D. Daugherty Sent: Thursday, December 13, 2012 1:51 PM To: build-dev; serviceability-...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net; Ron Durbin Subject: Re: code review request for 7153050 remove crufty '_g' support Serguei, Thanks for the fast review! I could swear that Ron had removed lines 41 and 45 in an earlier version. Sigh... Will be fixed. Dan On 12/13/12 1:22 PM, serguei.spit...@oracle.com wrote: Dan, It is nice fix and simplified many places. Just one minor comment: make/bsd/makefiles/dtrace.make 41 #LIBJVM_DB = libjvm_db.dylib 42 LIBJVM_DB = libjvm_db.dylib 45 #LIBJVM_DTRACE = libjvm_dtrace.dylib 46 LIBJVM_DTRACE = libjvm_dtrace.dylib The lines #41 and #45 can be removed. Thanks, Serguei On 12/13/12 10:53 AM, Daniel D. Daugherty wrote: Greetings, I'm sponsoring this code review request from Ron Durbin. This change is targeted at JDK8/HSX-25 in the RT_Baseline repo. Please make sure you include Ron on any e-mail replies since he is not yet on the OpenJDK aliases. Dan Intro: This set of changes removes the makefile support for generation of debug versions that follow _g semantics. Defect: 7153050 remove crufty '_g' support from HotSpot repo http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153050 https://jbs.oracle.com/bugs/browse/JDK-7153050 Webrev http://cr.openjdk.java.net/~dcubed/for_rdurbin/7153050-webrev/0 Details: Many makefiles have been modified to remove all reference and support for debug versions that follow _g semantics.
RE: code review request for 7153050 remove crufty '_g' support
I am back and Dan has It correct (JDK-8005044) -Original Message- From: Daniel D. Daugherty Sent: Thursday, December 13, 2012 4:32 PM To: hotspot-runtime-...@openjdk.java.net; build-dev; serviceability-...@openjdk.java.net Subject: Re: code review request for 7153050 remove crufty '_g' support Adding back in the other OpenJDK aliases... Ron's e-mail has gone off-line (no e-mail for him since 1440 MT)... Coleen, Thanks for the review! This bug (7153050) is just for the Makefile changes so that the reviews are more focused. Ron has separate bugs for the other parts of the hotspot repo that need to be fixed. I believe he is going to attack the src/... paths in his second fix. Dan On 12/13/12 4:13 PM, Coleen Phillimore wrote: Ron, I'm so happy this is going away.The code changed looks good. There are some comments in some files that refer to the old jvm_g.dll and libjvm_g.so. Can you remove these too? carrs% ygrep -l libjvm_g ./os/bsd/vm/os_bsd.cpp ./os/linux/vm/os_linux.cpp ./os/solaris/vm/os_solaris.cpp carrs% ygre jvm_g.dll ygre: Command not found. carrs% ygrep jvm_g.dll ./os/windows/vm/os_windows.cpp:// Find the full path to the current module, jvm.dll or jvm_g.dll ./share/tools/ProjectCreator/ProjectCreator.java:+ jvm.dll and jvm_g.dll; no trailing slash); Thanks, Coleen On 12/13/2012 03:53 PM, Ron Durbin wrote: Thx again -Original Message- From: Daniel D. Daugherty Sent: Thursday, December 13, 2012 1:51 PM To: build-dev; serviceability-...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net; Ron Durbin Subject: Re: code review request for 7153050 remove crufty '_g' support Serguei, Thanks for the fast review! I could swear that Ron had removed lines 41 and 45 in an earlier version. Sigh... Will be fixed. Dan On 12/13/12 1:22 PM, serguei.spit...@oracle.com wrote: Dan, It is nice fix and simplified many places. Just one minor comment: make/bsd/makefiles/dtrace.make 41 #LIBJVM_DB = libjvm_db.dylib 42 LIBJVM_DB = libjvm_db.dylib 45 #LIBJVM_DTRACE = libjvm_dtrace.dylib 46 LIBJVM_DTRACE = libjvm_dtrace.dylib The lines #41 and #45 can be removed. Thanks, Serguei On 12/13/12 10:53 AM, Daniel D. Daugherty wrote: Greetings, I'm sponsoring this code review request from Ron Durbin. This change is targeted at JDK8/HSX-25 in the RT_Baseline repo. Please make sure you include Ron on any e-mail replies since he is not yet on the OpenJDK aliases. Dan Intro: This set of changes removes the makefile support for generation of debug versions that follow _g semantics. Defect: 7153050 remove crufty '_g' support from HotSpot repo http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153050 https://jbs.oracle.com/bugs/browse/JDK-7153050 Webrev http://cr.openjdk.java.net/~dcubed/for_rdurbin/7153050-webrev/0 Details: Many makefiles have been modified to remove all reference and support for debug versions that follow _g semantics.
