8260589: Crash in JfrTraceIdLoadBarrier::load(_jclass*)

hotspot/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeSet.cpp

@@ -39,6 +39,7 @@
 #include "jfr/utilities/jfrTypes.hpp"
 #include "memory/iterator.hpp"
 #include "memory/resourceArea.hpp"
+#include "memory/universe.hpp"
 #include "oops/instanceKlass.hpp"
 #include "oops/objArrayKlass.hpp"
 #include "oops/oop.inline.hpp"
@@ -47,6 +48,8 @@
 
 // incremented on each checkpoint
 static u8 checkpoint_id = 0;
+static JfrCheckpointWriter* _writer = NULL;
+static int primitives_count = 9;
 
 // creates a unique id by combining a checkpoint relative symbol id (2^24)
 // with the current checkpoint id (2^40)
@@ -60,6 +63,47 @@ typedef const Symbol* SymbolPtr;
 typedef const JfrSymbolId::SymbolEntry* SymbolEntryPtr;
 typedef const JfrSymbolId::CStringEntry* CStringEntryPtr;
 
+static traceid create_symbol_id(traceid artifact_id) {
+  return artifact_id != 0 ? CREATE_SYMBOL_ID(artifact_id) : 0;
+}
+
+static bool is_initial_typeset_for_chunk(bool class_unload) {
+  return !class_unload;
+}
+
+static traceid mark_symbol(Symbol* symbol, JfrArtifactSet* artifacts) {
+  return symbol != NULL ? create_symbol_id(artifacts->mark(symbol)) : 0;
+}
+
+static const char* primitive_name(KlassPtr type_array_klass) {
+  switch (type_array_klass->name()->base()[1]) {
+    case JVM_SIGNATURE_BOOLEAN: return "boolean";
+    case JVM_SIGNATURE_BYTE: return "byte";
+    case JVM_SIGNATURE_CHAR: return "char";
+    case JVM_SIGNATURE_SHORT: return "short";
+    case JVM_SIGNATURE_INT: return "int";
+    case JVM_SIGNATURE_LONG: return "long";
+    case JVM_SIGNATURE_FLOAT: return "float";
+    case JVM_SIGNATURE_DOUBLE: return "double";
+  }
+  assert(false, "invalid type array klass");
+  return NULL;
+}
+
+static Symbol* primitive_symbol(KlassPtr type_array_klass) {
+  if (type_array_klass == NULL) {
+    // void.class
+    static Symbol* const void_class_name = SymbolTable::probe("void", 4);
+    assert(void_class_name != NULL, "invariant");
+    return void_class_name;
+  }
+  const char* const primitive_type_str = primitive_name(type_array_klass);
+  assert(primitive_type_str != NULL, "invariant");
+  Symbol* const primitive_type_sym = SymbolTable::probe(primitive_type_str, (int)strlen(primitive_type_str));
+  assert(primitive_type_sym != NULL, "invariant");
+  return primitive_type_sym;
+}
+
 inline uintptr_t package_name_hash(const char *s) {
   uintptr_t val = 0;
   while (*s != 0) {
@@ -83,6 +127,10 @@ static traceid cld_id(CldPtr cld) {
   return cld->is_anonymous() ? 0 : TRACE_ID(cld);
 }
 
+static u4 get_primitive_flags() {
+  return JVM_ACC_ABSTRACT | JVM_ACC_FINAL | JVM_ACC_PUBLIC;
+}
+
 static void tag_leakp_klass_artifacts(KlassPtr k, bool class_unload) {
   assert(k != NULL, "invariant");
   CldPtr cld = k->class_loader_data();
@@ -465,16 +513,22 @@ void JfrTypeSet::write_klass_constants(JfrCheckpointWriter* writer, JfrCheckpoin
     KlassCallback callback(&kwr);
     _subsystem_callback = &callback;
     do_klasses();
-    return;
+  } else {
+    TagLeakpKlassArtifact tagging(_class_unload);
+    LeakKlassWriter lkw(leakp_writer, _artifacts, _class_unload);
+    LeakpKlassArtifactTagging lpkat(&tagging, &lkw);
+    CompositeKlassWriter ckw(&lpkat, &kw);
+    CompositeKlassWriterRegistration ckwr(&ckw, &reg);
+    CompositeKlassCallback callback(&ckwr);
+    _subsystem_callback = &callback;
+    do_klasses();
+  }
+
+  if (is_initial_typeset_for_chunk(_class_unload)) {
+    // Because the set of primitives is written outside the callback,
+    // their count is not automatically incremented.
+    kw.add(primitives_count);
   }
-  TagLeakpKlassArtifact tagging(_class_unload);
-  LeakKlassWriter lkw(leakp_writer, _artifacts, _class_unload);
-  LeakpKlassArtifactTagging lpkat(&tagging, &lkw);
-  CompositeKlassWriter ckw(&lpkat, &kw);
-  CompositeKlassWriterRegistration ckwr(&ckw, &reg);
-  CompositeKlassCallback callback(&ckwr);
-  _subsystem_callback = &callback;
-  do_klasses();
 }
 
 typedef JfrArtifactWriterImplHost<CStringEntryPtr, write__artifact__package> PackageEntryWriterImpl;
@@ -653,12 +707,52 @@ void JfrTypeSet::do_klass(Klass* klass) {
   }
 }
 
+static traceid primitive_id(KlassPtr array_klass) {
+  if (array_klass == NULL) {
+    // The first klass id is reserved for the void.class.
+    return MaxJfrEventId + 101;
+  }
+  // Derive the traceid for a primitive mirror from its associated array klass (+1).
+  return JfrTraceId::get(array_klass) + 1;
+}
+
+static void write_primitive(JfrCheckpointWriter* writer, Klass* type_array_klass, JfrArtifactSet* artifacts) {
+  assert(writer != NULL, "invariant");
+  assert(artifacts != NULL, "invariant");
+  writer->write(primitive_id(type_array_klass));
+  writer->write(cld_id(Universe::boolArrayKlassObj()->class_loader_data()));
+  writer->write(mark_symbol(primitive_symbol(type_array_klass), artifacts));
+  writer->write(package_id(Universe::boolArrayKlassObj(), artifacts));
+  writer->write(get_primitive_flags());
+}
+
+// A mirror representing a primitive class (e.g. int.class) has no reified Klass*,
+// instead it has an associated TypeArrayKlass* (e.g. int[].class).
+// We can use the TypeArrayKlass* as a proxy for deriving the id of the primitive class.
+// The exception is the void.class, which has neither a Klass* nor a TypeArrayKlass*.
+// It will use a reserved constant.
+static void do_primitives(JfrArtifactSet* artifacts, bool class_unload) {
+  // Only write the primitive classes once per chunk.
+  if (is_initial_typeset_for_chunk(class_unload)) {
+    write_primitive(_writer, Universe::boolArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::byteArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::charArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::shortArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::intArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::longArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::singleArrayKlassObj(), artifacts);
+    write_primitive(_writer, Universe::doubleArrayKlassObj(), artifacts);
+    write_primitive(_writer, NULL, artifacts); // void.class
+  }
+}
+
 void JfrTypeSet::do_klasses() {
   if (_class_unload) {
     ClassLoaderDataGraph::classes_unloading_do(&do_unloaded_klass);
     return;
   }
   ClassLoaderDataGraph::classes_do(&do_klass);
+  do_primitives(_artifacts, _class_unload);
 }
 
 void JfrTypeSet::do_unloaded_class_loader_data(ClassLoaderData* cld) {
@@ -720,6 +814,7 @@ void JfrTypeSet::serialize(JfrCheckpointWriter* writer, JfrCheckpointWriter* lea
   assert(writer != NULL, "invariant");
   ResourceMark rm;
   // initialization begin
+  _writer = writer;
   _class_unload = class_unload;
   ++checkpoint_id;
   if (_artifacts == NULL) {
@@ -745,3 +840,4 @@ void JfrTypeSet::serialize(JfrCheckpointWriter* writer, JfrCheckpointWriter* lea
     clear_artifacts(_artifacts, class_unload);
   }
 }
+

hotspot/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceId.cpp

@@ -45,7 +45,7 @@ static traceid atomic_inc(traceid volatile* const dest) {
 }
 
 static traceid next_class_id() {
-  static volatile traceid class_id_counter = MaxJfrEventId + 100;
+  static volatile traceid class_id_counter = MaxJfrEventId + 101; // + 101 is for the void.class primitive;
   return atomic_inc(&class_id_counter) << TRACE_ID_SHIFT;
 }
 
@@ -107,6 +107,10 @@ void JfrTraceId::assign(const ClassLoaderData* cld) {
   cld->set_trace_id(next_class_loader_data_id());
 }
 
+traceid JfrTraceId::assign_primitive_klass_id() {
+  return next_class_id();
+}
+
 traceid JfrTraceId::assign_thread_id() {
   return next_thread_id();
 }
@@ -130,6 +134,28 @@ void JfrTraceId::restore(const Klass* k) {
   const traceid event_flags = k->trace_id();
   // get a fresh traceid and restore the original event flags
   k->set_trace_id(next_class_id() | event_flags);
+  if (k->oop_is_typeArray()) {
+    // the next id is reserved for the corresponding primitive class
+    next_class_id();
+  }
+}
+
+// A mirror representing a primitive class (e.g. int.class) has no reified Klass*,
+// instead it has an associated TypeArrayKlass* (e.g. int[].class).
+// We can use the TypeArrayKlass* as a proxy for deriving the id of the primitive class.
+// The exception is the void.class, which has neither a Klass* nor a TypeArrayKlass*.
+// It will use a reserved constant.
+static traceid load_primitive(const oop mirror) {
+  assert(java_lang_Class::is_primitive(mirror), "invariant");
+  const Klass* const tak = java_lang_Class::array_klass(mirror);
+  traceid id;
+  if (tak == NULL) {
+    // The first klass id is reserved for the void.class
+    id = MaxJfrEventId + 101;
+  } else {
+    id = JfrTraceId::get(tak) + 1;
+  }
+  return id;
 }
 
 traceid JfrTraceId::get(jclass jc) {
@@ -145,7 +171,8 @@ traceid JfrTraceId::use(jclass jc, bool leakp /* false */) {
   assert(((JavaThread*)Thread::current())->thread_state() == _thread_in_vm, "invariant");
   const oop my_oop = JNIHandles::resolve(jc);
   assert(my_oop != NULL, "invariant");
-  return use(java_lang_Class::as_Klass(my_oop), leakp);
+  const Klass* const k = java_lang_Class::as_Klass(my_oop);
+  return k != NULL ? use(k, leakp) : load_primitive(my_oop);
 }
 
 bool JfrTraceId::in_visible_set(const jclass jc) {

hotspot/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceId.hpp

@@ -77,6 +77,7 @@ class JfrTraceId : public AllStatic {
  public:
   static void assign(const Klass* klass);
   static void assign(const ClassLoaderData* cld);
+  static traceid assign_primitive_klass_id();
   static traceid assign_thread_id();
 
   static traceid get(const Klass* klass);

hotspot/src/share/vm/jfr/support/jfrTraceIdExtension.hpp

@@ -38,6 +38,7 @@
   static size_t trace_id_size() { return sizeof(traceid); }
 
 #define INIT_ID(data) JfrTraceId::assign(data)
+#define ASSIGN_PRIMITIVE_CLASS_ID(data) JfrTraceId::assign_primitive_klass_id()
 #define REMOVE_ID(k) JfrTraceId::remove(k);
 #define RESTORE_ID(k) JfrTraceId::restore(k);
 

hotspot/src/share/vm/oops/typeArrayKlass.cpp

@@ -68,6 +68,7 @@ TypeArrayKlass* TypeArrayKlass::create_klass(BasicType type,
   // including classes in the bootstrap (NULL) class loader.
   // GC walks these as strong roots.
   null_loader_data->add_class(ak);
+  JFR_ONLY(ASSIGN_PRIMITIVE_CLASS_ID(ak);)
 
   // Call complete_create_array_klass after all instance variables have been initialized.
   complete_create_array_klass(ak, ak->super(), CHECK_NULL);

jdk/test/jdk/jfr/jvm/TestPrimitiveClasses.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2021, Alibaba Group Holding Limited. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.jfr.jvm;
+
+import java.util.List;
+
+import jdk.jfr.Event;
+import jdk.jfr.Recording;
+import jdk.jfr.consumer.RecordedClass;
+import jdk.jfr.consumer.RecordedEvent;
+import jdk.test.lib.Asserts;
+import jdk.test.lib.jfr.Events;
+
+/**
+ * @test TestPrimitiveClasses
+ * @key jfr
+ * @library /lib /
+ * @run main/othervm jdk.jfr.jvm.TestPrimitiveClasses
+ */
+public class TestPrimitiveClasses {
+
+    private static class MyEvent extends Event {
+        Class<?> booleanClass = boolean.class;
+        Class<?> charClass = char.class;
+        Class<?> floatClass = float.class;
+        Class<?> doubleClass = double.class;
+        Class<?> byteClass = byte.class;
+        Class<?> shortClass = short.class;
+        Class<?> intClass = int.class;
+        Class<?> longClass = long.class;
+        Class<?> voidClass = void.class;
+    }
+
+    public static void main(String[] args) throws Exception {
+        try (Recording r = new Recording()) {
+            r.enable(MyEvent.class);
+            r.start();
+            MyEvent myEvent = new MyEvent();
+            myEvent.commit();
+            r.stop();
+            List<RecordedEvent> events = Events.fromRecording(r);
+            Events.hasEvents(events);
+            RecordedEvent event = events.get(0);
+            System.out.println(event);
+            testField(event, "booleanClass", boolean.class);
+            testField(event, "charClass", char.class);
+            testField(event, "floatClass", float.class);
+            testField(event, "doubleClass", double.class);
+            testField(event, "byteClass", byte.class);
+            testField(event, "shortClass", short.class);
+            testField(event, "intClass", int.class);
+            testField(event, "longClass", long.class);
+            testField(event, "voidClass", void.class);
+        }
+    }
+
+    private static void testField(RecordedEvent event, String fieldName, Class<?> expected) {
+        Asserts.assertTrue(event.hasField(fieldName));
+        RecordedClass classField = event.getValue(fieldName);
+        Asserts.assertEquals(classField.getName(), expected.getName());
+        Asserts.assertEquals(classField.getClassLoader().getName(), "<bootloader>");
+        Asserts.assertEquals(classField.getModifiers(), expected.getModifiers());
+    }
+}