8269125: Klass enqueue element size calculation wrong when traceid value cross compress limit

Markus Grönlund · Markus Grönlund · commit 1b2147a23f1b · 2021-06-23T11:49:25.000Z
Reviewed-by: jbachorik, egahlin
diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdKlassQueue.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdKlassQueue.cpp
@@ -67,7 +67,7 @@ static const size_t ELEMENT_SIZE = sizeof(JfrEpochQueueKlassElement);
 static const size_t NARROW_ELEMENT_SIZE = sizeof(JfrEpochQueueNarrowKlassElement);
 static const size_t THRESHOLD_SHIFT = 30;
 
-// If the upshifted traceid value is less than this threshold (1 073 741 824),
+// If the traceid value is less than this threshold (1 073 741 824),
 // compress the element for more effective queue storage.
 static const traceid uncompressed_threshold = ((traceid)1) << THRESHOLD_SHIFT;
 
@@ -121,30 +121,36 @@ static traceid read_element(const u1* pos, const Klass** klass, bool compressed)
   return compressed ? read_compressed_element(pos, klass) : read_uncompressed_element(pos, klass);
 }
 
+template <typename T>
+static inline void store_traceid(T* element, traceid id, bool uncompressed) {
+#ifdef VM_LITTLE_ENDIAN
+  id <<= METADATA_SHIFT;
+#endif
+  element->id = uncompressed ? id | UNCOMPRESSED : id;
+}
+
 static void store_compressed_element(traceid id, const Klass* klass, u1* pos) {
+  assert(can_compress_element(id), "invariant");
   JfrEpochQueueNarrowKlassElement* const element = new (pos) JfrEpochQueueNarrowKlassElement();
-  element->id = id;
+  store_traceid(element, id, false);
   element->compressed_klass = encode(klass);
 }
 
 static void store_uncompressed_element(traceid id, const Klass* klass, u1* pos) {
   JfrEpochQueueKlassElement* const element = new (pos) JfrEpochQueueKlassElement();
-  element->id = id | UNCOMPRESSED;
+  store_traceid(element, id, true);
   element->klass = klass;
 }
 
 static void store_element(const Klass* klass, u1* pos) {
   assert(pos != NULL, "invariant");
   assert(klass != NULL, "invariant");
-  traceid id = JfrTraceId::load_raw(klass);
-#ifdef VM_LITTLE_ENDIAN
-  id <<= METADATA_SHIFT;
-#endif
+  const traceid id = JfrTraceId::load_raw(klass);
   if (can_compress_element(id)) {
     store_compressed_element(id, klass, pos);
-  } else {
-    store_uncompressed_element(id, klass, pos);
+    return;
   }
+  store_uncompressed_element(id, klass, pos);
 }
 
 static void set_unloaded(const u1* pos) {
diff --git a/src/hotspot/share/jfr/utilities/jfrEpochQueue.inline.hpp b/src/hotspot/share/jfr/utilities/jfrEpochQueue.inline.hpp
@@ -68,7 +68,7 @@ JfrEpochQueue<ElementPolicy>::storage_for_element(JfrEpochQueue<ElementPolicy>::
 template <template <typename> class ElementPolicy>
 void JfrEpochQueue<ElementPolicy>::enqueue(JfrEpochQueue<ElementPolicy>::TypePtr t) {
   assert(t != NULL, "invariant");
-  static size_t element_size = _policy.element_size(t);
+  size_t element_size = _policy.element_size(t);
   BufferPtr buffer = storage_for_element(t, element_size);
   assert(buffer != NULL, "invariant");
   _policy.store_element(t, buffer);
