JDK-8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently #2157

tstuefe · 2021-01-20T06:39:06Z

When function descriptors are fed to os::print_function_and_library_name() to get debug output, it would be very helpful to transparently handle the case where the address is not a code pointer but a function descriptor.

Of the official OpenJDK platforms this affects only ppc, but also includes at least ia64 for those who maintain ports to that platform.

Fixing this will also fix the display of signal handlers on linux ppc which still show up without function names after JDK-8258606.

Before:

695 Signal Handlers:
696    SIGSEGV: 0x00000fff8d0f4348 in libjvm.so+34882376, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
697     SIGBUS: 0x00000fff8d0f4348 in libjvm.so+34882376, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
698     SIGFPE: 0x00000fff8d0f4348 in libjvm.so+34882376, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
699    SIGPIPE: 0x00000fff8d0e0158 in libjvm.so+34799960, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
700    SIGXFSZ: 0x00000fff8d0e0158 in libjvm.so+34799960, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
701     SIGILL: 0x00000fff8d0f4348 in libjvm.so+34882376, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
702    SIGUSR2: 0x00000fff8d0e0098 in libjvm.so+34799768, sa_mask[0]=00000000000000000000000000000000, sa_flags=SA_RESTART|SA_SIGINFO
703     SIGHUP: 0x00000fff8d0e0080 in libjvm.so+34799744, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
704     SIGINT: 0x00000fff8d0e0080 in libjvm.so+34799744, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
705    SIGTERM: 0x00000fff8d0e0080 in libjvm.so+34799744, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
706    SIGQUIT: 0x00000fff8d0e0080 in libjvm.so+34799744, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
707    SIGTRAP: 0x00000fff8d0f4348 in libjvm.so+34882376, sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO

Now:

693 Signal Handlers:
694    SIGSEGV: crash_handler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
695     SIGBUS: crash_handler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
696     SIGFPE: crash_handler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
697    SIGPIPE: javaSignalHandler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
698    SIGXFSZ: javaSignalHandler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
699     SIGILL: crash_handler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
700    SIGUSR2: SR_handler in libjvm.so (FD), sa_mask[0]=00000000000000000000000000000000, sa_flags=SA_RESTART|SA_SIGINFO
701     SIGHUP: UserHandler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
702     SIGINT: UserHandler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
703    SIGTERM: UserHandler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
704    SIGQUIT: UserHandler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO
705    SIGTRAP: crash_handler in libjvm.so (FD), sa_mask[0]=11100100010111111101111111111110, sa_flags=SA_RESTART|SA_SIGINFO

The patch also adds a little trailing marker (FD) as an indication for the developer that this had been a function descriptor.

Progress

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

Issue

JDK-8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently

Reviewers

Martin Doerr (@TheRealMDoerr - Reviewer) ⚠️ Review applies to 9cedffac379c3d8a4124ad2ca89176fc38972263
Lutz Schmidt (@RealLucy - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2157/head:pull/2157
$ git checkout pull/2157

bridgekeeper · 2021-01-20T06:39:43Z

👋 Welcome back stuefe! 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 · 2021-01-20T06:40:51Z

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

hotspot-runtime

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.

mlbridge · 2021-01-20T12:29:54Z

Webrevs

03: Full - Incremental (e4f7cac)
02: Full - Incremental (9cedffac379c3d8a4124ad2ca89176fc38972263)
01: Full - Incremental (a0bf1a7)
00: Full (c0a07f2)

TheRealMDoerr · 2021-01-20T14:44:11Z

Hi Thomas,
thanks for fixing it.
Please check PPC64le which doesn't use FunctionDescriptors. They are only used on Big Endian (linux + AIX).
Little Endian implements ABI v2 (also see usage of ABI_ELFv2 in hotspot code).

tstuefe · 2021-01-21T09:37:51Z

Hi Thomas,
thanks for fixing it.
Please check PPC64le which doesn't use FunctionDescriptors. They are only used on Big Endian (linux + AIX).
Little Endian implements ABI v2 (also see usage of ABI_ELFv2 in hotspot code).

Ouch, you are right. I fixed it. Thanks!

TheRealMDoerr

Hi Thomas, looks correct. I have only one wish.

src/hotspot/os_cpu/linux_ppc/os_linux_ppc.hpp

openjdk · 2021-01-21T09:53:42Z

@tstuefe 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:

8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently

Reviewed-by: mdoerr, lucy

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 108 new commits pushed to the master branch:

fa40a96: 8253420: Refactor HeapRegionManager::find_highest_free
4d004c9: 8260449: Remove stale declaration of SATBMarkQueue::apply_closure_and_empty()
fd2641e: 8260236: better init AnnotationCollector _contended_group
1c77046: 8260404: jvm_io.h include missing in a number of files
bd2744d: 8260106: Shenandoah: refactor reference updating closures and related code
c836da3: 8252412: [macos11] system dynamic libraries removed from filesystem
e1411fd: 6606673: Path2D.Double, Path2D.Float and GeneralPath ctors throw exception when initialCapacity is negative
6f2be9c: 8246112: Remove build-time and run-time checks for clock_gettime and CLOCK_MONOTONIC
19b6f61: 8260334: Remove deprecated sv_for_node_id() from Compile
1bebd41: 8260421: Shenandoah: Fix conc_mark_roots timing name and indentations
... and 98 more: https://git.openjdk.java.net/jdk/compare/cf25383d19e5b42768cdd07385a89dcbafbe02bb...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this 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.

TheRealMDoerr

Thanks!

tstuefe · 2021-01-26T12:45:25Z

Gentle ping.

RealLucy

The changes look good to me.
Thanks for enhancing the output. Looks much nicer now!
I don't like that you had to include additional asm headers. But that's not your fault. As in other places, the include hierarchy is messed up.

tstuefe · 2021-01-27T10:42:23Z

The changes look good to me.
Thanks for enhancing the output. Looks much nicer now!
I don't like that you had to include additional asm headers. But that's not your fault. As in other places, the include hierarchy is messed up.

Thank you Lutz! About the header, I got a build error without that include.

tstuefe · 2021-01-27T10:42:29Z

/integrate

openjdk · 2021-01-27T10:43:25Z

@tstuefe Since your change was applied there have been 108 commits pushed to the master branch:

fa40a96: 8253420: Refactor HeapRegionManager::find_highest_free
4d004c9: 8260449: Remove stale declaration of SATBMarkQueue::apply_closure_and_empty()
fd2641e: 8260236: better init AnnotationCollector _contended_group
1c77046: 8260404: jvm_io.h include missing in a number of files
bd2744d: 8260106: Shenandoah: refactor reference updating closures and related code
c836da3: 8252412: [macos11] system dynamic libraries removed from filesystem
e1411fd: 6606673: Path2D.Double, Path2D.Float and GeneralPath ctors throw exception when initialCapacity is negative
6f2be9c: 8246112: Remove build-time and run-time checks for clock_gettime and CLOCK_MONOTONIC
19b6f61: 8260334: Remove deprecated sv_for_node_id() from Compile
1bebd41: 8260421: Shenandoah: Fix conc_mark_roots timing name and indentations
... and 98 more: https://git.openjdk.java.net/jdk/compare/cf25383d19e5b42768cdd07385a89dcbafbe02bb...master

Your commit was automatically rebased without conflicts.

Pushed as commit 3e4194c.

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

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Jan 20, 2021

Initial

c0a07f2

tstuefe force-pushed the JDK-8260022-print_function_and_library_name-resolve-fundis-on-ppc branch from 4ccff8b to c0a07f2 Compare January 20, 2021 12:21

openjdk bot added hotspot hotspot-dev@openjdk.org and removed hotspot-runtime hotspot-runtime-dev@openjdk.org labels Jan 20, 2021

tstuefe marked this pull request as ready for review January 20, 2021 12:25

openjdk bot added the rfr Pull request is ready for review label Jan 20, 2021

Fix build errors on ppcle and AIX

a0bf1a7

TheRealMDoerr approved these changes Jan 21, 2021

View reviewed changes

src/hotspot/os_cpu/linux_ppc/os_linux_ppc.hpp Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Jan 21, 2021

TheRealMDoerr approved these changes Jan 21, 2021

View reviewed changes

Use ABI_ELFv2 instead of VM_LITTLE_ENDIAN

e4f7cac

tstuefe force-pushed the JDK-8260022-print_function_and_library_name-resolve-fundis-on-ppc branch from 9cedffa to e4f7cac Compare January 21, 2021 11:59

RealLucy approved these changes Jan 27, 2021

View reviewed changes

openjdk bot closed this Jan 27, 2021

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 Jan 27, 2021

tstuefe deleted the JDK-8260022-print_function_and_library_name-resolve-fundis-on-ppc branch February 1, 2021 07:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently #2157

JDK-8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently #2157

tstuefe commented Jan 20, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jan 20, 2021

openjdk bot commented Jan 20, 2021

mlbridge bot commented Jan 20, 2021 •

edited

Loading

TheRealMDoerr commented Jan 20, 2021

tstuefe commented Jan 21, 2021

TheRealMDoerr left a comment

openjdk bot commented Jan 21, 2021 •

edited

Loading

TheRealMDoerr left a comment

tstuefe commented Jan 26, 2021

RealLucy left a comment

tstuefe commented Jan 27, 2021

tstuefe commented Jan 27, 2021

openjdk bot commented Jan 27, 2021

JDK-8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently #2157

JDK-8260022: [ppc] os::print_function_and_library_name shall resolve function descriptors transparently #2157

Conversation

tstuefe commented Jan 20, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Jan 20, 2021

openjdk bot commented Jan 20, 2021

mlbridge bot commented Jan 20, 2021 • edited Loading

Webrevs

TheRealMDoerr commented Jan 20, 2021

tstuefe commented Jan 21, 2021

TheRealMDoerr left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 21, 2021 • edited Loading

TheRealMDoerr left a comment

Choose a reason for hiding this comment

tstuefe commented Jan 26, 2021

RealLucy left a comment

Choose a reason for hiding this comment

tstuefe commented Jan 27, 2021

tstuefe commented Jan 27, 2021

openjdk bot commented Jan 27, 2021

tstuefe commented Jan 20, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 20, 2021 •

edited

Loading

openjdk bot commented Jan 21, 2021 •

edited

Loading