From 5331a3ef739166b2a2b0871fc9615f2c99effa89 Mon Sep 17 00:00:00 2001 From: Justin King Date: Sat, 21 Jan 2023 08:57:14 +0000 Subject: [PATCH] 8298908: Instrument Metaspace for ASan Reviewed-by: stuefe, ihse, iklam --- make/autoconf/jdk-options.m4 | 2 +- .../share/memory/metaspace/chunkManager.cpp | 45 +++++++++++--- .../share/memory/metaspace/chunkManager.hpp | 2 + .../memory/metaspace/virtualSpaceNode.cpp | 9 +++ src/hotspot/share/sanitizers/address.h | 58 +++++++++++++++++++ .../gtest/metaspace/test_virtualspacenode.cpp | 8 ++- 6 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 src/hotspot/share/sanitizers/address.h diff --git a/make/autoconf/jdk-options.m4 b/make/autoconf/jdk-options.m4 index aa0f7c12dfb..3c71da95ca1 100644 --- a/make/autoconf/jdk-options.m4 +++ b/make/autoconf/jdk-options.m4 @@ -426,7 +426,7 @@ AC_DEFUN_ONCE([JDKOPT_SETUP_ADDRESS_SANITIZER], # ASan is simply incompatible with gcc -Wstringop-truncation. See # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85650 # It's harmless to be suppressed in clang as well. - ASAN_CFLAGS="-fsanitize=address -Wno-stringop-truncation -fno-omit-frame-pointer -DADDRESS_SANITIZER" + ASAN_CFLAGS="-fsanitize=address -Wno-stringop-truncation -fno-omit-frame-pointer -fno-common -DADDRESS_SANITIZER" ASAN_LDFLAGS="-fsanitize=address" JVM_CFLAGS="$JVM_CFLAGS $ASAN_CFLAGS" JVM_LDFLAGS="$JVM_LDFLAGS $ASAN_LDFLAGS" diff --git a/src/hotspot/share/memory/metaspace/chunkManager.cpp b/src/hotspot/share/memory/metaspace/chunkManager.cpp index 45f40c2db03..b13e29ac9e3 100644 --- a/src/hotspot/share/memory/metaspace/chunkManager.cpp +++ b/src/hotspot/share/memory/metaspace/chunkManager.cpp @@ -37,6 +37,7 @@ #include "memory/metaspace/virtualSpaceList.hpp" #include "memory/metaspace/virtualSpaceNode.hpp" #include "runtime/mutexLocker.hpp" +#include "sanitizers/address.h" #include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" @@ -107,6 +108,23 @@ void ChunkManager::split_chunk_and_add_splinters(Metachunk* c, chunklevel_t targ InternalStats::inc_num_chunk_splits(); } +Metachunk* ChunkManager::get_chunk(chunklevel_t preferred_level, chunklevel_t max_level, size_t min_committed_words) { + assert(preferred_level <= max_level, "Sanity"); + assert(chunklevel::level_fitting_word_size(min_committed_words) >= max_level, "Sanity"); + + Metachunk* c; + { + MutexLocker fcl(Metaspace_lock, Mutex::_no_safepoint_check_flag); + c = get_chunk_locked(preferred_level, max_level, min_committed_words); + } + + if (c != nullptr) { + ASAN_UNPOISON_MEMORY_REGION(c->base(), c->word_size() * BytesPerWord); + } + + return c; +} + // On success, returns a chunk of level of , but at most . // The first first of the chunk are guaranteed to be committed. // On error, will return NULL. @@ -116,12 +134,8 @@ void ChunkManager::split_chunk_and_add_splinters(Metachunk* c, chunklevel_t targ // is non-expandable but needs expanding - aka out of compressed class space). // - Or, if the necessary space cannot be committed because we hit a commit limit. // This may be either the GC threshold or MaxMetaspaceSize. -Metachunk* ChunkManager::get_chunk(chunklevel_t preferred_level, chunklevel_t max_level, size_t min_committed_words) { - assert(preferred_level <= max_level, "Sanity"); - assert(chunklevel::level_fitting_word_size(min_committed_words) >= max_level, "Sanity"); - - MutexLocker fcl(Metaspace_lock, Mutex::_no_safepoint_check_flag); - +Metachunk* ChunkManager::get_chunk_locked(chunklevel_t preferred_level, chunklevel_t max_level, size_t min_committed_words) { + assert_lock_strong(Metaspace_lock); DEBUG_ONLY(verify_locked();) DEBUG_ONLY(chunklevel::check_valid_level(max_level);) DEBUG_ONLY(chunklevel::check_valid_level(preferred_level);) @@ -231,6 +245,9 @@ Metachunk* ChunkManager::get_chunk(chunklevel_t preferred_level, chunklevel_t ma // !! Note: this may invalidate the chunk. Do not access the chunk after // this function returns !! void ChunkManager::return_chunk(Metachunk* c) { + // It is valid to poison the chunk payload area at this point since its physically separated from + // the chunk meta info. + ASAN_POISON_MEMORY_REGION(c->base(), c->word_size() * BytesPerWord); MutexLocker fcl(Metaspace_lock, Mutex::_no_safepoint_check_flag); return_chunk_locked(c); } @@ -279,8 +296,20 @@ void ChunkManager::return_chunk_locked(Metachunk* c) { // // On success, true is returned, false otherwise. bool ChunkManager::attempt_enlarge_chunk(Metachunk* c) { - MutexLocker fcl(Metaspace_lock, Mutex::_no_safepoint_check_flag); - return c->vsnode()->attempt_enlarge_chunk(c, &_chunks); + bool enlarged; + size_t old_word_size; + + { + MutexLocker fcl(Metaspace_lock, Mutex::_no_safepoint_check_flag); + old_word_size = c->word_size(); + enlarged = c->vsnode()->attempt_enlarge_chunk(c, &_chunks); + } + + if (enlarged) { + ASAN_UNPOISON_MEMORY_REGION(c->base() + old_word_size, (c->word_size() - old_word_size) * BytesPerWord); + } + + return enlarged; } static void print_word_size_delta(outputStream* st, size_t word_size_1, size_t word_size_2) { diff --git a/src/hotspot/share/memory/metaspace/chunkManager.hpp b/src/hotspot/share/memory/metaspace/chunkManager.hpp index 979db86b206..4c357c5b21f 100644 --- a/src/hotspot/share/memory/metaspace/chunkManager.hpp +++ b/src/hotspot/share/memory/metaspace/chunkManager.hpp @@ -95,6 +95,8 @@ class ChunkManager : public CHeapObj { // chunks. void split_chunk_and_add_splinters(Metachunk* c, chunklevel_t target_level); + Metachunk* get_chunk_locked(chunklevel_t preferred_level, chunklevel_t max_level, size_t min_committed_words); + // Return a single chunk to the freelist without doing any merging, and adjust accounting. void return_chunk_simple_locked(Metachunk* c); diff --git a/src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp b/src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp index 87fc5acde25..db994003a17 100644 --- a/src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp +++ b/src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp @@ -42,6 +42,7 @@ #include "runtime/globals.hpp" #include "runtime/mutexLocker.hpp" #include "runtime/os.hpp" +#include "sanitizers/address.h" #include "services/memTracker.hpp" #include "utilities/align.hpp" #include "utilities/debug.hpp" @@ -234,6 +235,10 @@ VirtualSpaceNode::VirtualSpaceNode(ReservedSpace rs, bool owns_rs, CommitLimiter assert_is_aligned(_base, chunklevel::MAX_CHUNK_BYTE_SIZE); assert_is_aligned(_word_size, chunklevel::MAX_CHUNK_WORD_SIZE); + + // Poison the memory region. It will be unpoisoned later on a per-chunk base for chunks that are + // handed to arenas. + ASAN_POISON_MEMORY_REGION(rs.base(), rs.size()); } // Create a node of a given size (it will create its own space). @@ -265,6 +270,10 @@ VirtualSpaceNode* VirtualSpaceNode::create_node(ReservedSpace rs, CommitLimiter* VirtualSpaceNode::~VirtualSpaceNode() { DEBUG_ONLY(verify_locked();) + // Undo the poisoning before potentially unmapping memory. This ensures that future mappings at + // the same address do not unexpectedly fail with use-after-poison. + ASAN_UNPOISON_MEMORY_REGION(_rs.base(), _rs.size()); + UL(debug, ": dies."); if (_owns_rs) { _rs.release(); diff --git a/src/hotspot/share/sanitizers/address.h b/src/hotspot/share/sanitizers/address.h new file mode 100644 index 00000000000..8519de1a849 --- /dev/null +++ b/src/hotspot/share/sanitizers/address.h @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2023, Google and/or its affiliates. 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. + * + */ + +#ifndef SHARE_SANITIZERS_ADDRESS_HPP +#define SHARE_SANITIZERS_ADDRESS_HPP + +#ifdef ADDRESS_SANITIZER +#include +#endif + +// ASAN_POISON_MEMORY_REGION()/ASAN_UNPOISON_MEMORY_REGION() +// +// Poisons/unpoisons the specified memory region. When ASan is available this is the macro of the +// same name from . When ASan is not available this macro is a NOOP +// which preserves the arguments, ensuring they still compile, but ensures they are stripped due to +// being unreachable. This helps ensure developers do not accidently break ASan builds. +#ifdef ADDRESS_SANITIZER +// ASAN_POISON_MEMORY_REGION is defined in +// ASAN_UNPOISON_MEMORY_REGION is defined in +#else +#define ASAN_POISON_MEMORY_REGION(addr, size) \ + do { \ + if (false) { \ + ((void) (addr)); \ + ((void) (size)); \ + } \ + } while (false) +#define ASAN_UNPOISON_MEMORY_REGION(addr, size) \ + do { \ + if (false) { \ + ((void) (addr)); \ + ((void) (size)); \ + } \ + } while (false) +#endif + +#endif // SHARE_SANITIZERS_ADDRESS_HPP diff --git a/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp b/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp index 344028cb646..5166eede645 100644 --- a/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp +++ b/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp @@ -33,6 +33,7 @@ #include "memory/metaspace/metaspaceSettings.hpp" #include "memory/metaspace/virtualSpaceNode.hpp" #include "runtime/mutexLocker.hpp" +#include "sanitizers/address.h" #include "utilities/debug.hpp" //#define LOG_PLEASE #include "metaspaceGtestCommon.hpp" @@ -378,7 +379,9 @@ public: } // Test-zap + ASAN_UNPOISON_MEMORY_REGION(c->base() + r.start(), r.size() * BytesPerWord); zap_range(c->base() + r.start(), r.size()); + ASAN_POISON_MEMORY_REGION(c->base() + r.start(), r.size() * BytesPerWord); // We should never reach commit limit since it is as large as the whole area. ASSERT_TRUE(rc); @@ -510,7 +513,9 @@ TEST_VM(metaspace, virtual_space_node_test_basics) { ASSERT_EQ(node->committed_words(), word_size); ASSERT_EQ(node->committed_words(), scomm.get()); DEBUG_ONLY(node->verify_locked();) + ASAN_UNPOISON_MEMORY_REGION(node->base(), node->word_size() * BytesPerWord); zap_range(node->base(), node->word_size()); + ASAN_POISON_MEMORY_REGION(node->base(), node->word_size() * BytesPerWord); node->uncommit_range(node->base(), node->word_size()); ASSERT_EQ(node->committed_words(), (size_t)0); @@ -524,14 +529,15 @@ TEST_VM(metaspace, virtual_space_node_test_basics) { ASSERT_EQ(node->committed_words(), i * Settings::commit_granule_words()); ASSERT_EQ(node->committed_words(), scomm.get()); DEBUG_ONLY(node->verify_locked();) + ASAN_UNPOISON_MEMORY_REGION(node->base(), i * Settings::commit_granule_words() * BytesPerWord); zap_range(node->base(), i * Settings::commit_granule_words()); + ASAN_POISON_MEMORY_REGION(node->base(), i * Settings::commit_granule_words() * BytesPerWord); } node->uncommit_range(node->base(), node->word_size()); ASSERT_EQ(node->committed_words(), (size_t)0); ASSERT_EQ(node->committed_words(), scomm.get()); DEBUG_ONLY(node->verify_locked();) - } // Note: we unfortunately need TEST_VM even though the system tested