Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DDR: add support for OSX core files #5127

Merged
merged 2 commits into from
Mar 23, 2019
Merged

DDR: add support for OSX core files #5127

merged 2 commits into from
Mar 23, 2019

Conversation

mikezhang1234567890
Copy link
Contributor

@mikezhang1234567890 mikezhang1234567890 commented Mar 15, 2019 •
edited
Loading

Added a core reader for reading OSX Mach-O core files.
Also re-enables DDR tests for OSX.

Resolves #3444, resolves #3366, resolves #3483

Signed-off-by: Mike Zhang mike.h.zhang@ibm.com

@pshipton
Copy link
Member

pshipton commented Mar 15, 2019 •
edited
Loading

Please fix the copyright check problems
https://ci.eclipse.org/openj9/job/PullRequest-CopyrightCheck-OpenJ9/13521/ - passed

@pshipton
Copy link
Member

pshipton commented Mar 15, 2019 •
edited
Loading

jenkins test sanity osx jdk11
https://ci.eclipse.org/openj9/job/PullRequest-Sanity-JDK11-osx_x86-64_cmprssptrs-OpenJ9/167/ - this passed

@pshipton
Copy link
Member

pshipton commented Mar 16, 2019 •
edited
Loading

jenkins test extended osx jdk11 - this passed

keithc-ca
keithc-ca reviewed Mar 18, 2019
View reviewed changes
Copy link
Contributor

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix formatting inconsistencies:

  • all indents should be with tabs, not spaces
  • opening braces not on a separate line
  • no trailing whitespace

debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/BuildVersionCommand.java Outdated Show resolved Hide resolved
@pshipton
Copy link
Member

@mikezhang1234567890 please try signing the eca again, it has worked for others today.

@DanHeidinga
Copy link
Member

@keithc-ca Can you review this again? It looks like Mike has updated it

@keithc-ca
Copy link
Contributor

Looking at this now.

keithc-ca
keithc-ca requested changes Mar 20, 2019
View reviewed changes
Copy link
Contributor

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can wait for a future pull request, but I think the LoadCommand class hierarchy ought to be constructed from the input rather than via a call to readCommand(). That will allow many of the fields of those classes to be final.

debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/MachoDumpReader.java Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/MachoDumpReader.java Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/MachoDumpReader.java Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/MachoDumpReader.java Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/OSXProcessAddressSpace.java Outdated
getBytesAt(nameAddress, stringBuffer);

try {
return new String(stringBuffer, "ASCII");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use StandardCharsets.ASCII you wan't have to catch UnsupportedEncodingException.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also fix LoadCommand and SymtabCommand.

debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/SourceVersionCommand.java Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/ThreadCommand.java Outdated Show resolved Hide resolved
debugtools/DDR_VM/src/com/ibm/j9ddr/vm29/tools/ddrinteractive/VMRegMapHelper.java Outdated Show resolved Hide resolved
@mikezhang1234567890
DDR: add support for OSX core files
6ef4f50 
Added a core reader for reading OSX Mach-O core files.
Parts of the core reader remain unimplemented, however enough is done
so that DDR can be used on the OSX core files.

Signed-off-by: Mike Zhang <mike.h.zhang@ibm.com>
@mikezhang1234567890
Unexclude DDR tests from OSX
a5611a5 
Signed-off-by: Mike Zhang <mike.h.zhang@ibm.com>
keithc-ca
keithc-ca reviewed Mar 22, 2019
View reviewed changes
debugtools/DDR_VM/src/com/ibm/j9ddr/corereaders/macho/MachoDumpReader.java
}
for (LoadCommand lc : dumpFile.otherLoads) {
if (lc instanceof ThreadCommand) {
dumpFile.threads = (ThreadCommand) lc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open an issue to improve this to handle core files with more than one thread.

@keithc-ca
Copy link
Contributor

Jenkins test extended osx,zlinux jdk11

@pshipton
Copy link
Member

Jenkins test sanity osx,zlinux jdk11

@pshipton
Copy link
Member

pshipton commented Mar 23, 2019 •
edited
Loading

@keithc-ca approved and the testing has passed.

@pshipton pshipton merged commit da9f2f0 into eclipse-openj9:master Mar 23, 2019
@keithc-ca
Copy link
Contributor

@pshipton Thanks for merging.

@pshipton
Copy link
Member

pshipton commented Mar 23, 2019 •
edited
Loading

jdk8 DDR tests are failing
https://ci.eclipse.org/openj9/job/Test-extended.functional-JDK8-osx_x86-64_cmprssptrs/104

 [ERR] _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
 [ERR] 2019-03-23 02:03:35.362 jdmpview[37189:54586460] CFPasteboardRef CFPasteboardCreate(CFAllocatorRef, CFStringRef) : failed to create global data

@keithc-ca
Copy link
Contributor

keithc-ca commented Mar 23, 2019 •
edited
Loading

Maybe those tests need jdmpview needs -XstartOnFirstThread on OSX?

@keithc-ca keithc-ca mentioned this pull request Mar 23, 2019
Merged
@pshipton
Copy link
Member

ibmruntimes/openj9-openjdk-jdk8#278 seems to have fixed jdmpview, but the two testDDRExt_* tests must not be using jdmpview and will need to be fixed separately.

@llxia llxia mentioned this pull request Mar 25, 2019
Merged
@mikezhang1234567890 mikezhang1234567890 mentioned this pull request Mar 25, 2019
Closed
@mikezhang1234567890 mikezhang1234567890 deleted the osx-ddr branch March 26, 2019 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keithc-ca keithc-ca keithc-ca approved these changes

Assignees
No one assigned
Labels
comp:vm
Projects
None yet
Milestone
Release 0.14.0 (Java 8, 11, 12) April...
4 participants
@mikezhang1234567890 @pshipton @DanHeidinga @keithc-ca