8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' #19677

MBaesken · 2024-06-12T13:47:25Z

When building with ubsan on linuxppc64le we run into the issue below; seems some initialization is missing.

/jdk/src/hotspot/share/opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool'
#0 0x7fff8f52b71c in PhaseOutput::Process_OopMap_Node(MachNode*, int) (/jdk/lib/server/libjvm.so+0x74cb71c)
#1 0x7fff8f530ab4 in PhaseOutput::fill_buffer(C2_MacroAssembler*, unsigned int*) (/jdk/lib/server/libjvm.so+0x74d0ab4)
#2 0x7fff8f53c740 in PhaseOutput::Output() (/jdk/lib/server/libjvm.so+0x74dc740)
#3 0x7fff8d5b6350 in Compile::Code_Gen() (/jdk/lib/server/libjvm.so+0x5556350)
#4 0x7fff8d5be730 in Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*) (/jdk/lib/server/libjvm.so+0x555e730)
#5 0x7fff8cfe7434 in C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*) (/jdk/lib/server/libjvm.so+0x4f87434)
#6 0x7fff8d5e3bf4 in CompileBroker::invoke_compiler_on_method(CompileTask*) (/jdk/lib/server/libjvm.so+0x5583bf4)
#7 0x7fff8d5e5a74 in CompileBroker::compiler_thread_loop() (/jdk/lib/server/libjvm.so+0x5585a74)
#8 0x7fff8d6403c0 in CompilerThread::thread_entry(JavaThread*, JavaThread*) (/jdk/lib/server/libjvm.so+0x55e03c0)
#9 0x7fff8e428810 in JavaThread::thread_main_inner() (/jdk/lib/server/libjvm.so+0x63c8810)
#10 0x7fff900992e8 in Thread::call_run() (/jdk/lib/server/libjvm.so+0x80392e8)
#11 0x7fff8f4a6a08 in thread_native_entry(Thread*) (/jdk/lib/server/libjvm.so+0x7446a08)
#12 0x7fff95139714 in start_thread (/lib64/libpthread.so.0+0x9714)
#13 0x7fff944eb774 in __GI___clone (/lib64/libc.so.6+0x13b774)
... (rest of output omitted)

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-8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' (Bug - P4)

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer)
Martin Doerr (@TheRealMDoerr - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19677

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-06-12T13:48:05Z

👋 Welcome back mbaesken! 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 · 2024-06-12T13:49:22Z

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

8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool'

Reviewed-by: kvn, mdoerr

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

d751441: 8330586: GHA: Drop additional gcc/glibc packages installation for x86_32
5e09397: 8334222: exclude containers/cgroup/PlainRead.java
7b38bfe: 8333729: C2 SuperWord: remove some @requires usages in test/hotspot/jtreg/compiler/loopopts/superword
29b6392: 8334228: C2 SuperWord: fix JDK-24 regression in VPointer::cmp_for_sort after JDK-8325155
31e8deb: 8324781: runtime/Thread/TestAlwaysPreTouchStacks.java failed with Expected a higher ratio between stack committed and reserved
8464ce6: 8332113: Update nsk.share.Log to be always verbose
548e95a: 8330702: Update failure handler to don't generate Error message if cores actions are empty
dae0bda: 8334252: Verifier error for lambda declared in early construction context
b5212d7: 8328107: Shenandoah/C2: TestVerifyLoopOptimizations test failure
efab48c: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher
... and 26 more: https://git.openjdk.org/jdk/compare/ba67ad63ae7d7d399e41ab258576123fb6d9502c...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.

openjdk · 2024-06-12T13:50:15Z

@MBaesken 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 · 2024-06-12T13:53:20Z

Webrevs

01: Full - Incremental (6bfe0103)
00: Full (7e8902b5)

dean-long · 2024-06-12T20:04:20Z

I think this deserves a little more investigation. It looks like this value should have been set from the CallJavaNode in Matcher::match_sfpt. If it isn't being set, then we will always get the default value from the constructor and not the value copied from the CallJavaNode.

dean-long · 2024-06-13T00:25:47Z

src/hotspot/share/opto/machnode.hpp

@@ -937,7 +937,7 @@ class MachCallJavaNode : public MachCallNode {
  bool      _optimized_virtual;      // Tells if node is a static call or an optimized virtual
  bool      _method_handle_invoke;   // Tells if the call has to preserve SP
  bool      _arg_escape;             // ArgEscape in parameter list
-  MachCallJavaNode() : MachCallNode(), _override_symbolic_info(false) {
+  MachCallJavaNode() : MachCallNode(), _override_symbolic_info(false), _arg_escape(false) {


If we need to initialize one bool, then we probably need to initialize them all. However, I think something else is going wrong to make this appear only on one platform.

Agree, I wondered about the others too; probably we should initialize them as well.

reinrich · 2024-06-14T09:06:19Z

The field _arg_escape is not initialized in ppc.ad.

MBaesken · 2024-06-14T09:40:52Z

The field _arg_escape is not initialized in ppc.ad.

Thanks for looking at this; this explains why the issue was observed only on ppc machines and so far not on x86_64 / aarch64.
Should I add something like
call->_arg_escape = _arg_escape;

to the ad file ? Are there more additional fields of MachCallJavaNode that need initialization in ppc.ad ?

reinrich · 2024-06-14T10:28:28Z

The field _arg_escape is not initialized in ppc.ad.

Thanks for looking at this; this explains why the issue was observed only on ppc machines and so far not on x86_64 / aarch64. Should I add something like call->_arg_escape = _arg_escape;
to the ad file ?

Yes that should help.

Are there more additional fields of MachCallJavaNode that need initialization in ppc.ad ?

I'd look at Matcher::match_sfpt to see what needs to be done in ppc.ad.

MBaesken · 2024-06-14T11:53:07Z

I added handling/initialization of _arg_escape to ppc.ad; confirmed that the ubsan issue is gone with the ppc.ad change.
(the issue was already seen when configuring with the added flag --enable-ubsan and then starting a build on Linux ppc64le).

reinrich · 2024-06-14T12:33:46Z

It might be better to add a method that does the necessary initialization. That method can then be called from ppc.ad and from Matcher::match_sfpt.

MBaesken · 2024-06-14T13:01:45Z

It might be better to add a method that does the necessary initialization. That method can then be called from ppc.ad and from Matcher::match_sfpt.

Might be a good cleanup, but I prefer to do this in a separate issue (and fix here only the ppc issue with a small fix).

vnkozlov · 2024-06-14T15:44:44Z

It might be better to add a method that does the necessary initialization. That method can then be called from ppc.ad and from Matcher::match_sfpt.

Might be a good cleanup, but I prefer to do this in a separate issue (and fix here only the ppc issue with a small fix).

I created JDK-8334303 for that.

vnkozlov

Good.

TheRealMDoerr

LGTM. Thanks for fixing it! We should also backport this fix.

MBaesken · 2024-06-17T08:56:59Z

/integrate

openjdk · 2024-06-17T08:57:59Z

Going to push as commit 113a2c0.
Since your change was applied there have been 36 commits pushed to the master branch:

d751441: 8330586: GHA: Drop additional gcc/glibc packages installation for x86_32
5e09397: 8334222: exclude containers/cgroup/PlainRead.java
7b38bfe: 8333729: C2 SuperWord: remove some @requires usages in test/hotspot/jtreg/compiler/loopopts/superword
29b6392: 8334228: C2 SuperWord: fix JDK-24 regression in VPointer::cmp_for_sort after JDK-8325155
31e8deb: 8324781: runtime/Thread/TestAlwaysPreTouchStacks.java failed with Expected a higher ratio between stack committed and reserved
8464ce6: 8332113: Update nsk.share.Log to be always verbose
548e95a: 8330702: Update failure handler to don't generate Error message if cores actions are empty
dae0bda: 8334252: Verifier error for lambda declared in early construction context
b5212d7: 8328107: Shenandoah/C2: TestVerifyLoopOptimizations test failure
efab48c: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher
... and 26 more: https://git.openjdk.org/jdk/compare/ba67ad63ae7d7d399e41ab258576123fb6d9502c...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-06-17T08:58:04Z

@MBaesken Pushed as commit 113a2c0.

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

JDK-8332903

7e8902b

openjdk bot changed the title ~~JDK-8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool'~~ 8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' Jun 12, 2024

openjdk bot added the rfr Pull request is ready for review label Jun 12, 2024

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Jun 12, 2024

dean-long reviewed Jun 13, 2024

View reviewed changes

initialize _arg_escape in ppc.ad

6bfe010

vnkozlov approved these changes Jun 14, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 14, 2024

TheRealMDoerr approved these changes Jun 17, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jun 17, 2024

openjdk bot closed this Jun 17, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' #19677

8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' #19677

MBaesken commented Jun 12, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jun 12, 2024

openjdk bot commented Jun 12, 2024 •

edited

Loading

openjdk bot commented Jun 12, 2024

mlbridge bot commented Jun 12, 2024 •

edited

Loading

dean-long commented Jun 12, 2024

dean-long Jun 13, 2024

MBaesken Jun 13, 2024

reinrich commented Jun 14, 2024

MBaesken commented Jun 14, 2024

reinrich commented Jun 14, 2024

MBaesken commented Jun 14, 2024

reinrich commented Jun 14, 2024

MBaesken commented Jun 14, 2024

vnkozlov commented Jun 14, 2024

vnkozlov left a comment

TheRealMDoerr left a comment

MBaesken commented Jun 17, 2024

openjdk bot commented Jun 17, 2024

openjdk bot commented Jun 17, 2024

8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' #19677

8332903: ubsan: opto/output.cpp:1002:18: runtime error: load of value 171, which is not a valid value for type 'bool' #19677

Conversation

MBaesken commented Jun 12, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jun 12, 2024

openjdk bot commented Jun 12, 2024 • edited Loading

openjdk bot commented Jun 12, 2024

mlbridge bot commented Jun 12, 2024 • edited Loading

Webrevs

dean-long commented Jun 12, 2024

dean-long Jun 13, 2024

Choose a reason for hiding this comment

MBaesken Jun 13, 2024

Choose a reason for hiding this comment

reinrich commented Jun 14, 2024

MBaesken commented Jun 14, 2024

reinrich commented Jun 14, 2024

MBaesken commented Jun 14, 2024

reinrich commented Jun 14, 2024

MBaesken commented Jun 14, 2024

vnkozlov commented Jun 14, 2024

vnkozlov left a comment

Choose a reason for hiding this comment

TheRealMDoerr left a comment

Choose a reason for hiding this comment

MBaesken commented Jun 17, 2024

openjdk bot commented Jun 17, 2024

openjdk bot commented Jun 17, 2024

MBaesken commented Jun 12, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Jun 12, 2024 •

edited

Loading

mlbridge bot commented Jun 12, 2024 •

edited

Loading