8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' #19885

MBaesken · 2024-06-25T15:00:03Z

With ubsan enabled binaries we run on Linux aarch64 and Linux x86_64 into this issue :

runtime/CommandLine/PrintClasses_id0.jtr

src/hotspot/share/oops/instanceKlass.cpp:3603:84: runtime error: member call on null pointer of type 'struct AnnotationArray'
#0 0xfffface09b40 in InstanceKlass::print_on(outputStream*) const src/hotspot/share/oops/instanceKlass.cpp:3603
#1 0xffffacdcd088 in PrintClassClosure::do_klass(Klass*) src/hotspot/share/oops/instanceKlass.cpp:2228
#2 0xffffac464200 in ClassLoaderData::classes_do(KlassClosure*) src/hotspot/share/classfile/classLoaderData.cpp:387
#3 0xffffac475c4c in ClassLoaderDataGraph::classes_do(KlassClosure*) src/hotspot/share/classfile/classLoaderDataGraph.cpp:303
#4 0xffffac7bc4f4 in VM_PrintClasses::doit() src/hotspot/share/services/diagnosticCommand.cpp:989
#5 0xffffae599c88 in VM_Operation::evaluate() src/hotspot/share/runtime/vmOperations.cpp:75
#6 0xffffae5a5a14 in VMThread::evaluate_operation(VM_Operation*) src/hotspot/share/runtime/vmThread.cpp:283
#7 0xffffae5a779c in VMThread::inner_execute(VM_Operation*) src/hotspot/share/runtime/vmThread.cpp:427
#8 0xffffae5a7fd8 in VMThread::loop() src/hotspot/share/runtime/vmThread.cpp:493
#9 0xffffae5a80bc in VMThread::run() src/hotspot/share/runtime/vmThread.cpp:177
#10 0xffffae396958 in Thread::call_run() src/hotspot/share/runtime/thread.cpp:225
#11 0xffffadba1b0c in thread_native_entry src/hotspot/os/linux/os_linux.cpp:846
#12 0xffffb1a9d5c4 (/lib/aarch64-linux-gnu/libc.so.6+0x7d5c4)
#13 0xffffb1b05ed8 (/lib/aarch64-linux-gnu/libc.so.6+0xe5ed8)

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-8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' (Bug - P4)

Reviewers

Coleen Phillimore (@coleenp - Reviewer)
Stefan Karlsson (@stefank - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19885

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-06-25T15:01:06Z

👋 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-25T15:01:23Z

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

8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray'

Reviewed-by: coleenp, stefank

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

f23295e: 8334600: TEST java/net/MulticastSocket/IPMulticastIF.java fails on linux-aarch64
741a0f3: 8334241: Adjust API docs side bar dimensions
4ce8822: 8334037: Local class creation in lambda in pre-construction context crashes javac
7f6804c: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
e139005: 8333994: NMT: call stacks should show source information
b88af94: 8269870: PS: Membar in PSPromotionManager::copy_unmarked_to_survivor_space could be relaxed
a5f401f: 8334650: Add debug information about whether an Assertion Predicate is for the init or last value
25c3845: 8333133: Simplify QuickSort::sort
c66f785: 8334505: RISC-V: Several tests fail when MaxVectorSize does not match VM_Version::_initial_vector_length
f101e15: 8333583: Crypto-XDH.generateSecret regression after JDK-8329538
... and 10 more: https://git.openjdk.org/jdk/compare/75a2afacc8f5fdec53350b1cb66076cdfeae12f0...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-25T15:01:57Z

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

mlbridge · 2024-06-25T15:04:33Z

Webrevs

MBaesken · 2024-06-25T15:04:47Z

The other returns pointers (e.g. class_type_annotations()) can be nullptr too, so we need the same checking there as well.

coleenp

Can you make this a bigger change to handle all the potentially null pointers the same way?

coleenp · 2024-06-25T15:30:02Z

src/hotspot/share/oops/instanceKlass.cpp

-  st->print(BULLET"class type annotations:  "); class_type_annotations()->print_value_on(st); st->cr();
-  st->print(BULLET"field annotations:       "); fields_annotations()->print_value_on(st); st->cr();
-  st->print(BULLET"field type annotations:  "); fields_type_annotations()->print_value_on(st); st->cr();
+  if (class_annotations() != nullptr) {


I hate to say it but this whole function looks like it should be rewritten. There are other places that could be null, like local_interfaces, and transitive_interfaces. I wonder if you should have a macro above with a string BULLET string, and do them all like this.

it might also be that all the metadata print_on functions should use the same thing. There's

static void print_value_on_maybe_null(outputStream* st, const Metadata* m) { if (nullptr == m) st->print("null"); else m->print_value_on(st); }

maybe that should take the string with BULLET and print in the whole thing in the else statement.

Or add a similar one that prints the address.

MBaesken · 2024-06-25T15:59:42Z

There are other places that could be null, like local_interfaces, and transitive_interfaces.

Hi Coleen, in this change I only adjusted the ones that were really reported when running HS :tier1 with ubsan enabled binaries.
So yes, maybe some others could be null too, but for those I really saw it then running the tests.

coleenp · 2024-06-25T17:53:55Z

Why not add this to metadata.hpp:

+  template <typename M>
+  static void print_on_maybe_null(outputStream* st, const char* str, const M* m) {
+    if (nullptr != m) {
+      st->print_raw(str);
+      m->print_value_on(st);
+      st->cr();
+    }
+  }

and use it for the things that ubsan complains about now. Then you could use it for the next set of ubsan complaints.

MBaesken · 2024-06-26T11:34:19Z

Hi Coleen, that print_on_maybe_null template is a great idea ! Added that and used it at the places where we check for nullptr.

src/hotspot/share/oops/instanceKlass.cpp

coleenp

This looks good. I think print_on_maybe_null will come in handy. Thank you for doing this ubsan work.

MBaesken · 2024-06-27T06:52:08Z

Hi Coleen and Stefan, thanks for the reviews !

/integrate

openjdk · 2024-06-27T06:53:04Z

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

0fc5b27: 8332014: since-checker - Fix @ since tags in jdk.jshell
9d20b58: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal
9bb675f: 8334719: (se) Deferred close of SelectableChannel may result in a Selector doing the final close before concurrent I/O on channel has completed
6682305: 8334779: Test compiler/c1/CanonicalizeArrayLength.java is timing out
3796fdf: 8328536: javac - crash on unknown type referenced in yield statement
07bc523: 8334670: SSLSocketOutputRecord buffer miscalculation
4ebb771: 8334769: Shenandoah: Move CodeCache_lock close to its use in ShenandoahConcurrentNMethodIterator
817edcb: 8331411: Shenandoah: Reconsider spinning duration in ShenandoahLock
bffc848: 8333755: NumberFormat integer only parsing breaks when format has suffix
b5d5896: 8335108: Build error after JDK-8333658 due to class templates
... and 26 more: https://git.openjdk.org/jdk/compare/75a2afacc8f5fdec53350b1cb66076cdfeae12f0...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-06-27T06:53:10Z

@MBaesken Pushed as commit 46b817b.

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

JDK-8333363

530375f

openjdk bot changed the title ~~JDK-8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray'~~ 8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' Jun 25, 2024

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

openjdk bot added the hotspot [email protected] label Jun 25, 2024

coleenp suggested changes Jun 25, 2024

View reviewed changes

introduce print_on_maybe_null

fb63c5a

stefank reviewed Jun 26, 2024

View reviewed changes

src/hotspot/share/oops/instanceKlass.cpp Outdated Show resolved Hide resolved

add some blanks removed before

fd99642

coleenp approved these changes Jun 26, 2024

View reviewed changes

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

stefank approved these changes Jun 26, 2024

View reviewed changes

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

openjdk bot closed this Jun 27, 2024

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' #19885

8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' #19885

MBaesken commented Jun 25, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jun 25, 2024

openjdk bot commented Jun 25, 2024 •

edited

Loading

openjdk bot commented Jun 25, 2024

mlbridge bot commented Jun 25, 2024 •

edited

Loading

MBaesken commented Jun 25, 2024

coleenp left a comment

coleenp Jun 25, 2024

coleenp Jun 25, 2024 •

edited

Loading

MBaesken commented Jun 25, 2024

coleenp commented Jun 25, 2024

MBaesken commented Jun 26, 2024

coleenp left a comment

MBaesken commented Jun 27, 2024

openjdk bot commented Jun 27, 2024

openjdk bot commented Jun 27, 2024

8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' #19885

8333363: ubsan: instanceKlass.cpp: runtime error: member call on null pointer of type 'struct AnnotationArray' #19885

Conversation

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

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jun 25, 2024

openjdk bot commented Jun 25, 2024 • edited Loading

openjdk bot commented Jun 25, 2024

mlbridge bot commented Jun 25, 2024 • edited Loading

Webrevs

MBaesken commented Jun 25, 2024

coleenp left a comment

Choose a reason for hiding this comment

coleenp Jun 25, 2024

Choose a reason for hiding this comment

coleenp Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

MBaesken commented Jun 25, 2024

coleenp commented Jun 25, 2024

MBaesken commented Jun 26, 2024

coleenp left a comment

Choose a reason for hiding this comment

MBaesken commented Jun 27, 2024

openjdk bot commented Jun 27, 2024

openjdk bot commented Jun 27, 2024

MBaesken commented Jun 25, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Jun 25, 2024 •

edited

Loading

mlbridge bot commented Jun 25, 2024 •

edited

Loading

coleenp Jun 25, 2024 •

edited

Loading