diff --git a/src/hotspot/share/logging/logDecorations.cpp b/src/hotspot/share/logging/logDecorations.cpp index eb4ae0b10f6..a8f76db6548 100644 --- a/src/hotspot/share/logging/logDecorations.cpp +++ b/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 diff --git a/src/hotspot/share/logging/logDecorations.hpp b/src/hotspot/share/logging/logDecorations.hpp index 64f79c34b82..5fde6ef497b 100644 --- a/src/hotspot/share/logging/logDecorations.hpp +++ b/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; } }; diff --git a/test/hotspot/gtest/logging/test_logDecorations.cpp b/test/hotspot/gtest/logging/test_logDecorations.cpp index 27402bc7970..995e964da50 100644 --- a/test/hotspot/gtest/logging/test_logDecorations.cpp +++ b/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(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)); + } + } +}