code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Oct 15 07:31:25 PDT 2013
On 10/15/13 12:05 AM, David Holmes wrote:
> On 15/10/2013 1:52 AM, Daniel D. Daugherty wrote:
>> On 10/14/13 9:18 AM, Daniel D. Daugherty wrote:
>>> On 10/13/13 7:57 PM, David Holmes wrote:
>>>
>>>> Hmm second comment - I don't see a .m4 file change that corresponds
>>>> to the DSYMUTIL configure change ??
>>>
>>> Yikes! I'll check into that shortly. Not sure what happened here.
>>
>> I somehow missed/lost two 1-liner changes:
>>
>> hg diff common/autoconf/basics.m4 common/autoconf/spec.gmk.in
>> diff -r 4faa09c7fe55 common/autoconf/basics.m4
>> --- a/common/autoconf/basics.m4 Wed Oct 02 15:08:03 2013 +0200
>> +++ b/common/autoconf/basics.m4 Mon Oct 14 08:49:37 2013 -0700
>> @@ -644,6 +644,7 @@ fi
>> fi
>>
>> if test "x$OPENJDK_TARGET_OS" = "xmacosx"; then
>> + BASIC_REQUIRE_PROG(DSYMUTIL, dsymutil)
>> BASIC_REQUIRE_PROG(XATTR, xattr)
>> AC_PATH_PROG(CODESIGN, codesign)
>> if test "x$CODESIGN" != "x"; then
>> diff -r 4faa09c7fe55 common/autoconf/spec.gmk.in
>> --- a/common/autoconf/spec.gmk.in Wed Oct 02 15:08:03 2013 +0200
>> +++ b/common/autoconf/spec.gmk.in Mon Oct 14 08:49:37 2013 -0700
>> @@ -484,6 +484,7 @@ DATE:=@DATE@
>> DATE:=@DATE@
>> DIFF:=@DIFF@
>> DIRNAME:=@DIRNAME@
>> +DSYMUTIL:=@DSYMUTIL@
>> FIND:=@FIND@
>> FIND_DELETE:=@FIND_DELETE@
>> ECHO:=@ECHO@
>>
>>
>> I don't think that these two 1-liners are worth re-rolling the webrev.
>> Please let me know if you don't agree.
>
> Look fine to me.
Thanks for the review and for catching the miss/loss on my part.
Dan
>
> David
>
>> Dan
>>
>>
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 12/10/2013 6:27 AM, Daniel D. Daugherty wrote:
>>>>> Greetings,
>>>>>
>>>>> I'm ready for code review round 1 of the FDS on MacOS X hotspot
>>>>> changes.
>>>>> Below is the original code review round 0 invite (slightly edited for
>>>>> clarity). Working on FDS is like pulling a thread on a sweater... so
>>>>> there are four additional changed files along with more changes to
>>>>> many of the files from round 0.
>>>>>
>>>>> Here is the code review round 1 JDK8/HSX-25 webrev URL:
>>>>>
>>>>> OpenJDK:
>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
>>>>> Internal:
>>>>> http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/
>>>>>
>>>>>
>>>>>
>>>>> Changes from code review round 0 to round 1:
>>>>>
>>>>> root repo:
>>>>> - add configure support for DSYMUTIL macro
>>>>>
>>>>> hotspot repo:
>>>>> - drop Minimal1 config support for FDS on MacOS X
>>>>> - use DSYMUTIL macro instead of literal 'dsymutil'
>>>>> - fix FASTDEBUG_CFLAGS typo and add USE_CLANG support
>>>>> - STRIP is not used on MacOS X
>>>>> - fix $(UNIVERSAL_COPY_LIST) rule
>>>>> - refine install-dir function
>>>>>
>>>>> jdk repo:
>>>>> - add prep-target-dir, install-import-dir, and
>>>>> install-import-debuginfo
>>>>> functions
>>>>> - support importing .dSYM directories from hotspot
>>>>> - create libjsig.dSYM symlink from VM specific directory to
>>>>> the real location
>>>>>
>>>>> Files changed from code review round 0 to round 1:
>>>>>
>>>>> common/autoconf/generated-configure.sh
>>>>> hotspot/make/Makefile
>>>>> hotspot/make/bsd/makefiles/defs.make
>>>>> hotspot/make/bsd/makefiles/dtrace.make
>>>>> hotspot/make/bsd/makefiles/gcc.make
>>>>> hotspot/make/bsd/makefiles/jsig.make
>>>>> hotspot/make/bsd/makefiles/saproc.make
>>>>> hotspot/make/bsd/makefiles/universal.gmk
>>>>> hotspot/make/bsd/makefiles/vm.make
>>>>> hotspot/make/defs.make
>>>>>
>>>>> New files changed in code review round 1:
>>>>>
>>>>> hotspot/make/bsd/makefiles/product.make
>>>>> jdk/make/common/Defs.gmk
>>>>> jdk/make/java/redist/Makefile
>>>>> jdk/makefiles/Import.gmk
>>>>>
>>>>> Additional bugs filed based on code review comments, testing and
>>>>> just plain looking at the Makefiles:
>>>>>
>>>>> 8026280 implement Full Debug Symbols on MacOS X jdk
>>>>> 8026281 hotspot 'jdk6_or_earlier logic' for FDS needs to be
>>>>> removed
>>>>> 8026282 the '64' subdir logic in Makefiles might be removable
>>>>> 8026283 literal 'lipo' cmd uses in HotSpot makefiles
>>>>> 8026284 Minimal1 references/support should be removed from
>>>>> BSD/MacOS X
>>>>> 8026285 JPRT -buildenv ZIP_DEBUGINFO_FILES=0 no longer works
>>>>>
>>>>> As always, comments, questions and suggestions are welcome.
>>>>>
>>>>> Dan
>>>>>
>>>>> > 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
>>>>> >
>>>>> > Once these changes are approved, I'm planning to push them to
>>>>> > RT_Baseline. From there, they can follow the normal path to
>>>>> > Main_Baseline and eventually JDK8.
>>>>> >
>>>>> > This work enables FDS on MacOS X for the 'hotspot' repo; the
>>>>> changes in
>>>>> > the other repos are necessary to support importing the .diz files
>>>>> from
>>>>> > the MacOS X 'hotspot' build into the forest build. I also fixed a
>>>>> few
>>>>> > FDS related errors in the magic incantations for the new build.
>>>>> This is
>>>>> > mostly a port from Linux -> MacOS X/BSD with the dtrace changes
>>>>> ported
>>>>> > from Solaris. In other words, this is Frankenstein's monster...
>>>>> >
>>>>> > Thanks to Staffan Larsen for providing an initial set of changes
>>>>> > which I morphed into what you see here.
>>>>> >
>>>>> > Testing:
>>>>> > - JPRT HSX build and test on all platforms; verification of .diz
>>>>> > files in the MacOS X JPRT bundles
>>>>> > - JPRT JDK8 forest build and test on all platforms;
>>>>> verification of
>>>>> > .diz files in the MacOS X JPRT bundles
>>>>> > Note: In previous FDS changesets, I also did a standalone 'jdk'
>>>>> > repo build and test, but that no longer seems to work.
>>>>> >
>>>>> > As always, comments, questions and suggestions are welcome.
>>>>> >
>>>>> > Dan
>>>
>>>
>>
More information about the serviceability-dev
mailing list