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.

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

8266967: debug.cpp utility find() should print Java Object fields. #4011

Closed
wants to merge 3 commits into from

Conversation

kevinjwalls
Copy link
Contributor

@kevinjwalls kevinjwalls commented May 13, 2021

This change enables debug.cpp's find() utility to print Java Objects with their fields.

find() calls os::print_location, and Java heap objects are printed with instanceKlass oop_print_on.
Removing the ifdef for defining oop_print_on for instanceKlass, and also on methods in FieldPrinter and FieldDescriptor, make this work.

Checking other uses of os::print_location this might affect:

macroAssembler_x86.cpp has MacroAssembler::print_state32 and MacroAssembler::print_state64
which use os::print_location to print register contents and print words at top of stack.
These will be more verbose, as it already is in non-PRODUCT builds.

vmError uses os::print_location when showing the stack, i.e. this output:

Stack slot to memory mapping:
stack at sp + 0 slots: 0x0000000000000002 is an unknown value
..etc...

...will be more verbose when Java object references are found (for the 8 stack slots it tries to show).

Shenandoah uses os::print_location once, but for non-Java heap objects so nothing changes.

Manual testing on Linux-x64 and Windows: old behaviour shows these two lines only:

"Executing find"
0x00000000ff0a03e0 is an oop: jdk.internal.loader.ClassLoaders$AppClassLoader
{0x00000000ff0a03e0} - klass: 'jdk/internal/loader/ClassLoaders$AppClassLoader'

...then with the change the full info:

"Executing find"
0x00000000ff0a03e0 is an oop: jdk.internal.loader.ClassLoaders$AppClassLoader
{0x00000000ff0a03e0} - klass: 'jdk/internal/loader/ClassLoaders$AppClassLoader'

  • ---- fields (total size 13 words):
  • private 'defaultAssertionStatus' 'Z' @12 false
  • private final 'parent' 'Ljava/lang/ClassLoader;' @24 a 'jdk/internal/loader/ClassLoaders$PlatformClassLoader'{0x00000000ff0a0a
    40} (ff0a0a40)
  • private final 'name' 'Ljava/lang/String;' @28 "app"{0x00000000ff0d0060} (ff0d0060)
  • private final 'unnamedModule' 'Ljava/lang/Module;' @32 a 'java/lang/Module'{0x00000000ff0a0448} (ff0a044)
    ...etc...

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8266967: debug.cpp utility find() should print Java Object fields.

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4011/head:pull/4011
$ git checkout pull/4011

Update a local copy of the PR:
$ git checkout pull/4011
$ git pull https://git.openjdk.java.net/jdk pull/4011/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4011

View PR using the GUI difftool:
$ git pr show -t 4011

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4011.diff

@bridgekeeper
Copy link

bridgekeeper bot commented May 13, 2021

👋 Welcome back kevinw! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label May 13, 2021
@openjdk
Copy link

openjdk bot commented May 13, 2021

@kevinjwalls The following label will be automatically applied to this pull request:

  • hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the hotspot hotspot-dev@openjdk.org label May 13, 2021
@mlbridge
Copy link

mlbridge bot commented May 13, 2021

Webrevs

@kevinjwalls
Copy link
Contributor Author

Oops the #ifndef PRODUCT move in instanceKlass.hpp made more than just the oop printers visible, moving it to line 1270, above print_dependent_nmethods.

@kevinjwalls
Copy link
Contributor Author

/label serviceability

@openjdk openjdk bot added the serviceability serviceability-dev@openjdk.org label May 19, 2021
@openjdk
Copy link

openjdk bot commented May 19, 2021

@kevinjwalls
The serviceability label was successfully added.

Copy link
Contributor

@sspitsyn sspitsyn left a comment

Choose a reason for hiding this comment

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

Hi Kevin,
The fix looks okay to me.
Thanks,
Serguei

@openjdk
Copy link

openjdk bot commented May 20, 2021

@kevinjwalls This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8266967: debug.cpp utility find() should print Java Object fields.

Reviewed-by: sspitsyn, coleenp

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label May 20, 2021
Copy link
Contributor

@coleenp coleenp left a comment

Choose a reason for hiding this comment

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

This looks good!

@kevinjwalls
Copy link
Contributor Author

Thanks Serguei and Coleen!

@kevinjwalls
Copy link
Contributor Author

/integrate

@openjdk openjdk bot closed this Jun 7, 2021
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 7, 2021
@openjdk
Copy link

openjdk bot commented Jun 7, 2021

@kevinjwalls Since your change was applied there has been 1 commit pushed to the master branch:

  • c7c77fd: 8255557: Decouple GCM from CipherCore

Your commit was automatically rebased without conflicts.

Pushed as commit 5e557d8.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotspot hotspot-dev@openjdk.org integrated Pull request has been integrated serviceability serviceability-dev@openjdk.org
Development

Successfully merging this pull request may close these issues.

3 participants