8255466: C2 crashes at ciObject::get_oop() const+0x0 #890

vnkozlov · 2020-10-28T02:19:42Z

Graal testing hit this issue with product VM. Tom R. suggested that it could be the case of reflective unsafe static field access that would eventually be optimized away because the Class is null:
if (staticFieldBase != null) { return Unsafe.getInt(staticFieldBase, Unsafe.staticFieldOffset(field)); }

I suggest to replace assert with runtime check. Note, o value is assigned to _const_oop so semantically new code is the same except additional runtime check.

I also noticed that const_oop is accessed without check for NULL in new Vector API code. I added check there too.

Passed tier1-3 testing.

Progress

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

Testing

	Linux x64	Linux x86	Windows x64	macOS x64
Build	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)

Issue

JDK-8255466: C2 crashes at ciObject::get_oop() const+0x0

Reviewers

Vladimir Ivanov (@iwanowww - Reviewer) ⚠️ Review applies to da8be52

Download

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

bridgekeeper · 2020-10-28T02:20:14Z

👋 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 · 2020-10-28T02:22:52Z

@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 · 2020-10-28T02:25:25Z

Webrevs

01: Full - Incremental (0de9389)
00: Full (da8be52)

iwanowww

It would be nice to have a regression test for it.

Otherwise, looks good.

iwanowww · 2020-10-28T08:40:31Z

src/hotspot/share/opto/type.cpp

@@ -3046,9 +3046,11 @@ TypeOopPtr::TypeOopPtr(TYPES t, PTR ptr, ciKlass* k, bool xk, ciObject* o, int o
        } else if (klass() == ciEnv::current()->Class_klass() &&
                   _offset >= InstanceMirrorKlass::offset_of_static_fields()) {


You could turn the assert into the check in the enclosing if. IMO it makes the code clearer.

In that case it would fall into Instance fields code below under } else { which I don't want.

openjdk · 2020-10-28T08:41:39Z

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

8255466: C2 crashes at ciObject::get_oop() const+0x0

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

caec8d2: 8233560: [TESTBUG] ToolTipManager/Test6256140.java is failing on macos
a5b42ec: 8233570: [TESTBUG] HTMLEditorKit test bug5043626.java is failing on macos
7e305ad: 8255405: sun/net/ftp/imp/FtpClient uses SimpleDateFormat in not thread-safe manner
d82a6dc: 8255438: [Vector API] More instructs in x86.ad should use legacy mode for code-gen
1a5e6c9: 8253101: Clean up CallStaticJavaNode EA flags
a7595b2: 8250669: Running JMH micros is broken after JDK-8248135
edd1988: 8255530: Additional cleanup after JDK-8235710 (elliptic curve removal)
790d6e2: 8255533: Incorrect javadoc in DateTimeFormatterBuilder.appendPattern() for 'uu'/'yy'
3f20612: 8255555: Bad copyright headers in SocketChannelCompare.java SocketChannelConnectionSetup.java UnixSocketChannelReadWrite.java
42fc158: 8253939: [TESTBUG] Increase coverage of the cgroups detection code
... and 20 more: https://git.openjdk.java.net/jdk/compare/acd0e2560c90c9369c2c39a70275e57488bfcea1...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.

vnkozlov · 2020-10-29T22:32:59Z

/integrate

openjdk · 2020-10-29T22:37:32Z

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

5782a2a: 8254975: lambda proxy fails to access a protected member inherited from a split package
d5138d1: 8255604: java/nio/channels/DatagramChannel/Connect.java fails with java.net.BindException: Cannot assign requested address: connect
2a2fa13: 8255449: Improve the exception message of MethodHandles::permuteArguments
2a50c3f: 8241495: Make more compiler related flags available on a per method level
5c520c3: 8255232: G1: Make G1BiasedMappedArray freeable
9e5bbff: 8255550: x86: Assembler::cmpq(Address dst, Register src) encoding is incorrect
5b18558: 8255243: Reinforce escape barrier interactions with ZGC conc stack processing
faf23de: 8255534: Shenandoah: Fix CmpP optimization wrt native-LRB
579e50b: 8255564: InterpreterMacroAssembler::remove_activation() needs to restore thread right after VM call on x86_32
4b20e46: 8255579: x86: Use cmpq(Register,Address) in safepoint_poll
... and 34 more: https://git.openjdk.java.net/jdk/compare/acd0e2560c90c9369c2c39a70275e57488bfcea1...master

Your commit was automatically rebased without conflicts.

Pushed as commit 56eb5f5.

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

8255466: C2 crashes at ciObject::get_oop() const+0x0

da8be52

openjdk bot added the rfr Pull request is ready for review label Oct 28, 2020

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Oct 28, 2020

iwanowww approved these changes Oct 28, 2020

View reviewed changes

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

Added regression test provided by Tom

0de9389

openjdk bot closed this Oct 29, 2020

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 Oct 29, 2020

vnkozlov deleted the JDK-8255466 branch October 29, 2020 22:36

8255466: C2 crashes at ciObject::get_oop() const+0x0 #890

8255466: C2 crashes at ciObject::get_oop() const+0x0 #890

vnkozlov commented Oct 28, 2020 •

edited by openjdk bot

bridgekeeper bot commented Oct 28, 2020

openjdk bot commented Oct 28, 2020

mlbridge bot commented Oct 28, 2020 •

edited

iwanowww left a comment

iwanowww Oct 28, 2020

vnkozlov Oct 28, 2020

openjdk bot commented Oct 28, 2020 •

edited

vnkozlov commented Oct 29, 2020

openjdk bot commented Oct 29, 2020

		@@ -3046,9 +3046,11 @@ TypeOopPtr::TypeOopPtr(TYPES t, PTR ptr, ciKlass* k, bool xk, ciObject* o, int o
		} else if (klass() == ciEnv::current()->Class_klass() &&
		_offset >= InstanceMirrorKlass::offset_of_static_fields()) {

8255466: C2 crashes at ciObject::get_oop() const+0x0 #890

8255466: C2 crashes at ciObject::get_oop() const+0x0 #890

Conversation

vnkozlov commented Oct 28, 2020 • edited by openjdk bot

Progress

Testing

Issue

Reviewers

Download

bridgekeeper bot commented Oct 28, 2020

openjdk bot commented Oct 28, 2020

mlbridge bot commented Oct 28, 2020 • edited

Webrevs

iwanowww left a comment

Choose a reason for hiding this comment

iwanowww Oct 28, 2020

Choose a reason for hiding this comment

vnkozlov Oct 28, 2020

Choose a reason for hiding this comment

openjdk bot commented Oct 28, 2020 • edited

vnkozlov commented Oct 29, 2020

openjdk bot commented Oct 29, 2020

vnkozlov commented Oct 28, 2020 •

edited by openjdk bot

mlbridge bot commented Oct 28, 2020 •

edited

openjdk bot commented Oct 28, 2020 •

edited