start

src/hotspot/share/logging/logDecorations.cpp

@@ -57,10 +57,10 @@ void LogDecorations::create_decorations(const LogDecorators &decorators) {
   char* position = _decorations_buffer;
   #define DECORATOR(full_name, abbr) \
   if (decorators.is_decorator(LogDecorators::full_name##_decorator)) { \
-    _decoration_offset[LogDecorators::full_name##_decorator] = position; \
+    _decoration_offset[LogDecorators::full_name##_decorator] = (offset_t)(position - _decorations_buffer); \
     position = create_##full_name##_decoration(position) + 1; \
   } else { \
-    _decoration_offset[LogDecorators::full_name##_decorator] = NULL; \
+    _decoration_offset[LogDecorators::full_name##_decorator] = invalid_offset; \
   }
   DECORATOR_LIST
 #undef DECORATOR

src/hotspot/share/logging/logDecorations.hpp

@@ -32,8 +32,15 @@ class LogDecorations {
  public:
   static const int DecorationsBufferSize = 256;
  private:
+  // Buffer for resolved decorations
   char _decorations_buffer[DecorationsBufferSize];
-  char* _decoration_offset[LogDecorators::Count];
+  // Lookup table, contains offsets of the decoration string start addresses by logtag
+  // To keep its size small (which matters, see e.g. JDK-8229517) we use a byte index. That is
+  //  fine since the max. size of the decorations buffer is 256. Note: 255 is reserved
+  //  as "invalid offset" marker.
+  typedef uint8_t offset_t;
+  static const offset_t invalid_offset = DecorationsBufferSize - 1;
+  offset_t _decoration_offset[LogDecorators::Count];
   LogLevelType _level;
   const LogTagSet& _tagset;
   static const char* volatile _host_name;
@@ -56,7 +63,11 @@ class LogDecorations {
     if (decorator == LogDecorators::level_decorator) {
       return LogLevel::name(_level);
     }
-    return _decoration_offset[decorator];
+    const offset_t offset = _decoration_offset[decorator];
+    if (offset == invalid_offset) {
+      return NULL;
+    }
+    return _decorations_buffer + offset;
   }
 };
 

test/hotspot/gtest/logging/test_logDecorations.cpp

@@ -230,3 +230,40 @@ TEST(LogDecorations, identifiers) {
     EXPECT_EQ(ids[i].expected, strtol(reported, NULL, 10));
   }
 }
+
+// Test that the LogDecorations object can be safely copied.
+TEST_VM(LogDecorations, copy) {
+  // A literal with all decorations, comma-separated
+  const char* const all_decorators =
+#define DECORATOR(name, abbr) "," #name
+  DECORATOR_LIST
+#undef DECORATOR
+  ;
+  LogDecorators decorator_selection;
+  ASSERT_TRUE(decorator_selection.parse(all_decorators + 1 // skip leading comma
+              , tty));
+  LogDecorations decorations_orig(LogLevel::Info, tagset, decorator_selection);
+  LogDecorations decorations_copy(decorations_orig);
+
+  for (uint i = 0; i < LogDecorators::Count; i++) {
+    LogDecorators::Decorator decorator = static_cast<LogDecorators::Decorator>(i);
+    // Each decorator should be set here
+    EXPECT_TRUE(decorator_selection.is_decorator(decorator));
+
+    // We expect the resolved decorations to be not-null, to be semantically equal but
+    // to point to different addresses within each LogDecorations object
+    // (unless level string, which is a constant kept somewhere else)
+    const char* orig_deco = decorations_orig.decoration(decorator);
+    const char* copy_deco = decorations_copy.decoration(decorator);
+    EXPECT_NE(orig_deco, (const char*)NULL);
+    EXPECT_NE(copy_deco, (const char*)NULL);
+    EXPECT_EQ(::strcmp(orig_deco, copy_deco), 0);
+    if (decorator != LogDecorators::level_decorator) {
+      EXPECT_NE(orig_deco, copy_deco);
+      EXPECT_GE((address)orig_deco, (address)(&decorations_orig));
+      EXPECT_LT((address)orig_deco, (address)(&decorations_orig + 1));
+      EXPECT_GE((address)copy_deco, (address)(&decorations_copy));
+      EXPECT_LT((address)copy_deco, (address)(&decorations_copy + 1));
+    }
+  }
+}