8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work #10894

vnkozlov · 2022-10-27T23:14:11Z

EA does not adjust NSR (not_scalar_replaceable) state for referenced allocations.
In the test case object A is NSR because it merges with NULL object. But this state is not propagated to allocations it references. As result other allocations are marked scalar replaceable and related Load node is moved above guarding condition (where A object is checked for NULL).
EA should propagate NSR state.

Thanks to @rwestrel who provided reproducer test case.

Testing tier1-4, xcomp, stress.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work

Reviewers

Roland Westrelin (@rwestrel - Reviewer)
Vladimir Ivanov (@iwanowww - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10894/head:pull/10894
$ git checkout pull/10894

Update a local copy of the PR:
$ git checkout pull/10894
$ git pull https://git.openjdk.org/jdk pull/10894/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10894

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10894.diff

bridgekeeper · 2022-10-27T23:15:06Z

👋 Welcome back kvn! 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 · 2022-10-27T23:17:24Z

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

hotspot-compiler

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 · 2022-10-27T23:20:33Z

Webrevs

00: Full (48f01281)

rwestrel

Looks reasonable to me. Have you verified that the failure with the replay file and jars from the initial crash doesn't reproduce either?

openjdk · 2022-10-28T07:25:38Z

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

8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work

Reviewed-by: roland, vlivanov

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

a44ebd5: 8295849: Consolidate Threads::owning_thread*
1c86cf5: 8294672: Typo in description of JDWP VirtualMachine/AllThreads command
823fd4a: 8293785: Add a jtreg test for TraceOptoParse
754bd53: 8296030: compiler/c2/irTests/TestVectorizeTypeConversion.java fails with release VMs after JDK-8291781
1fdbb1b: 8295926: RISC-V: C1: Fix LIRGenerator::do_LibmIntrinsic
cf5546b: 8291336: Add ideal rule to convert floating point multiply by 2 into addition
4b89fce: 8291781: assert(!is_visited) failed: visit only once with -XX:+SuperWordRTDepCheck
d5d3424: 8295405: Add cause in a couple of IllegalArgumentException and InvalidParameterException shown by sun/security/pkcs11 tests
fd668dc: 8295537: Enhance TRACE_METHOD_LINKAGE to show the target MethodHandle
182c215: 8295994: Remove left over InetAddressContainer class
... and 9 more: https://git.openjdk.org/jdk/compare/c2d7a35a4b3ec7d9c567cdd98c2db958c4b2e9d2...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.

uschindler · 2022-10-28T07:34:53Z

Looks reasonable to me. Have you verified that the failure with the replay file and jars from the initial crash doesn't reproduce either?

I had the same question. I commented on the issue. It would be good to understand how the test code relates to the code that fails in Lucene and possibly Ben Manes' Caffeine. Maybe a short explanation what is A, B, C in Lucene's code and which loop is affected.

rwestrel · 2022-10-28T08:30:56Z

Looks reasonable to me. Have you verified that the failure with the replay file and jars from the initial crash doesn't reproduce either?

I tried the replay from the bug and the crash is gone with this fix.

uschindler · 2022-10-28T08:55:09Z

Looks reasonable to me. Have you verified that the failure with the replay file and jars from the initial crash doesn't reproduce either?

I tried the replay from the bug and the crash is gone with this fix.

Thanks for confirmation. See also my comments here: https://bugs.openjdk.org/browse/JDK-8285835?focusedCommentId=14533010&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14533010

The test code in this PR looks like our Lucene code (A wraps/refers B wraps/refers C; all some nested Lucene DocValues wrappers).

vnkozlov · 2022-10-28T17:47:48Z

I ran replay and jar files from Tobias's example and looked on EA outputs. I found Allocation which was not marked NSR:

3310  Allocate  === 4539 2045 2046 386 1 (...) [[ ... ]]  rawptr:NotNull ( int:>=0, java/lang/Object:NotNull *, bool, top, bool ) Lucene90DocValuesProducer::getNumericValues @ bci:141 (line 647) Lucene90DocValuesProducer::getSortedNumeric @ bci:53 (line 1300) Lucene90DocValuesProducer::getSortedNumeric @ bci:19 (line 1287) AssertingDocValuesFormat$AssertingDocValuesProducer::getSortedNumeric @ bci:45 (line 270) PerFieldDocValuesFormat$FieldsReader::getSortedNumeric @ bci:27 (line 346) CodecReader::getSortedNumericDocValues @ bci:24 (line 176) DocValues::getSortedNumeric @ bci:2 (line 295) RangeFacetCounts::count @ bci:52 (line 122) !jvms: DirectMonotonicReader::getInstance @ bci:4 (line 101) Lucene90DocValuesProducer::getSortedNumeric @ bci:47 (line 1298) Lucene90DocValuesProducer::getSortedNumeric @ bci:19 (line 1287) AssertingDocValuesFormat$AssertingDocValuesProducer::getSortedNumeric @ bci:45 (line 270) PerFieldDocValuesFormat$FieldsReader::getSortedNumeric @ bci:27 (line 346) CodecReader::getSortedNumericDocValues @ bci:24 (line 176) DocValues::getSortedNumeric @ bci:2 (line 295) RangeFacetCounts::count @ bci:52 (line 122)

LocalVar(491) [ 3310P [ 2056 ]]   3313  Proj  === 3310  [[ 4542 2056 ]] #5 !jvms: DirectMonotonicReader::getInstance @ bci:14 (line 102) Lucene90DocValuesProducer::getSortedNumeric @ bci:47 (line 1298) Lucene90DocValuesProducer::getSortedNumeric @ bci:19 (line 1287) AssertingDocValuesFormat$AssertingDocValuesProducer::getSortedNumeric @ bci:45 (line 270) PerFieldDocValuesFormat$FieldsReader::getSortedNumeric @ bci:27 (line 346) CodecReader::getSortedNumericDocValues @ bci:24 (line 176) DocValues::getSortedNumeric @ bci:2 (line 295) RangeFacetCounts::count @ bci:52 (line 122)

2056  CheckCastPP  === 3312 3313  [[ ... ]]  #org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer$15:NotNull:exact *  Oop:org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer$15:NotNull:exact * !jvms: String::compareTo @ bci:5 (line 141) TreeMap::getEntry @ bci:37 (line 350) TreeMap::get @ bci:2 (line 279) PerFieldDocValuesFormat$FieldsReader::getSortedNumeric @ bci:8 (line 345) CodecReader::getSortedNumericDocValues @ bci:24 (line 176) DocValues::getSortedNumeric @ bci:2 (line 295) RangeFacetCounts::count @ bci:52 (line 122)

This allocation is also listed in bad dominators output.

After fix the the replay passed and the allocation is marked as NSR:

JavaObject(59) NoEscape(NoEscape) is NSR. is stored into field with NSR base

JavaObject(59) NoEscape(NoEscape) NSR [ 6054F 6052F 6055F 6556F 6537F 2723F 5743F 2807F 5645F [ 3313 2056 6291 6290 5740 5642 4937 4776 3732 3549 2542 2336 2547 2341 ]]   3310  Allocate  === 4539 2045 2046 386 1 (2717 4540 761 1 1 772 773 758 774 775 776 777 1 1 1 1 1 1 1 1 1 1 1 778 773 1 1 1 1 1 1 1 1 779 1 1 1 1 1 780 781 1 833 1 1 780 1 1 2047 3308 1 1 1 ) [[ 5280 4367 3075 6056 2054 3313 ]]  rawptr:NotNull ( int:>=0, java/lang/Object:NotNull *, bool, top, bool ) Lucene90DocValuesProducer::getNumericValues @ bci:141 (line 647) Lucene90DocValuesProducer::getSortedNumeric @ bci:53 (line 1300) Lucene90DocValuesProducer::getSortedNumeric @ bci:19 (line 1287) AssertingDocValuesFormat$AssertingDocValuesProducer::getSortedNumeric @ bci:45 (line 270) PerFieldDocValuesFormat$FieldsReader::getSortedNumeric @ bci:27 (line 346) CodecReader::getSortedNumericDocValues @ bci:24 (line 176) DocValues::getSortedNumeric @ bci:2 (line 295) RangeFacetCounts::count @ bci:52 (line 122) !jvms: DirectMonotonicReader::getInstance @ bci:4 (line 101) Lucene90DocValuesProducer::getSortedNumeric @ bci:47 (line 1298) Lucene90DocValuesProducer::getSortedNumeric @ bci:19 (line 1287) AssertingDocValuesFormat$AssertingDocValuesProducer::getSortedNumeric @ bci:45 (line 270) PerFieldDocValuesFormat$FieldsReader::getSortedNumeric @ bci:27 (line 346) CodecReader::getSortedNumericDocValues @ bci:24 (line 176) DocValues::getSortedNumeric @ bci:2 (line 295) RangeFacetCounts::count @ bci:52 (line 122)

vnkozlov · 2022-10-28T22:49:23Z

Thank you, @rwestrel, for review and additional testing.

iwanowww

Looks good.

vnkozlov · 2022-10-29T12:31:30Z

/integrate

vnkozlov · 2022-10-29T12:31:51Z

Thank you, Vladimir.

openjdk · 2022-10-29T12:32:50Z

Going to push as commit 8aa1526.
Since your change was applied there have been 19 commits pushed to the master branch:

a44ebd5: 8295849: Consolidate Threads::owning_thread*
1c86cf5: 8294672: Typo in description of JDWP VirtualMachine/AllThreads command
823fd4a: 8293785: Add a jtreg test for TraceOptoParse
754bd53: 8296030: compiler/c2/irTests/TestVectorizeTypeConversion.java fails with release VMs after JDK-8291781
1fdbb1b: 8295926: RISC-V: C1: Fix LIRGenerator::do_LibmIntrinsic
cf5546b: 8291336: Add ideal rule to convert floating point multiply by 2 into addition
4b89fce: 8291781: assert(!is_visited) failed: visit only once with -XX:+SuperWordRTDepCheck
d5d3424: 8295405: Add cause in a couple of IllegalArgumentException and InvalidParameterException shown by sun/security/pkcs11 tests
fd668dc: 8295537: Enhance TRACE_METHOD_LINKAGE to show the target MethodHandle
182c215: 8295994: Remove left over InetAddressContainer class
... and 9 more: https://git.openjdk.org/jdk/compare/c2d7a35a4b3ec7d9c567cdd98c2db958c4b2e9d2...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-10-29T12:33:06Z

@vnkozlov Pushed as commit 8aa1526.

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

8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work

48f0128

openjdk bot added the rfr Pull request is ready for review label Oct 27, 2022

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Oct 27, 2022

rwestrel approved these changes Oct 28, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 28, 2022

This was referenced Oct 28, 2022

SIGSEGV in PhaseIdealLoop::build_loop_late_post_work adoptium/adoptium-support#530

Open

Solr crashes on JDK17+ during JIT compile of BoundedLocalCache::put ben-manes/caffeine#797

Closed

iwanowww approved these changes Oct 29, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Oct 29, 2022

openjdk bot closed this Oct 29, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 29, 2022

vnkozlov deleted the JDK-8285835 branch October 29, 2022 12:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work #10894

8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work #10894

vnkozlov commented Oct 27, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 27, 2022

openjdk bot commented Oct 27, 2022

mlbridge bot commented Oct 27, 2022

rwestrel left a comment

openjdk bot commented Oct 28, 2022 •

edited

Loading

uschindler commented Oct 28, 2022

rwestrel commented Oct 28, 2022

uschindler commented Oct 28, 2022

vnkozlov commented Oct 28, 2022

vnkozlov commented Oct 28, 2022

iwanowww left a comment

vnkozlov commented Oct 29, 2022

vnkozlov commented Oct 29, 2022

openjdk bot commented Oct 29, 2022

openjdk bot commented Oct 29, 2022

8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work #10894

8285835: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work #10894

Conversation

vnkozlov commented Oct 27, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Oct 27, 2022

openjdk bot commented Oct 27, 2022

mlbridge bot commented Oct 27, 2022

Webrevs

rwestrel left a comment

Choose a reason for hiding this comment

openjdk bot commented Oct 28, 2022 • edited Loading

uschindler commented Oct 28, 2022

rwestrel commented Oct 28, 2022

uschindler commented Oct 28, 2022

vnkozlov commented Oct 28, 2022

vnkozlov commented Oct 28, 2022

iwanowww left a comment

Choose a reason for hiding this comment

vnkozlov commented Oct 29, 2022

vnkozlov commented Oct 29, 2022

openjdk bot commented Oct 29, 2022

openjdk bot commented Oct 29, 2022

vnkozlov commented Oct 27, 2022 •

edited by openjdk bot

Loading

openjdk bot commented Oct 28, 2022 •

edited

Loading