Skip to content

Commit

Permalink
8327971: Multiple ASAN errors reported for metaspace
Browse files Browse the repository at this point in the history
8327988: When running ASAN, disable dangerous NMT test

Reviewed-by: stuefe, shade
Backport-of: 9e566d76d1d8acca27d8f69fffcbeb0b49b060ba
  • Loading branch information
Sonia Zaldana Calles authored and shipilev committed Apr 23, 2024
1 parent ac402fb commit 324cef1
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/hotspot/share/memory/metaspace/metachunk.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2021 SAP SE. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
Expand Down Expand Up @@ -34,6 +34,7 @@
#include "utilities/align.hpp"
#include "utilities/copy.hpp"
#include "utilities/debug.hpp"
#include "utilities/macros.hpp"
#include "utilities/ostream.hpp"

namespace metaspace {
Expand Down Expand Up @@ -285,7 +286,9 @@ void Metachunk::verify() const {
const size_t required_alignment = word_size() * sizeof(MetaWord);
assert_is_aligned(base(), required_alignment);

// Test accessing the committed area.
// Test accessing the committed area. But not for ASAN. We don't know which portions
// of the chunk are still poisoned.
#if !INCLUDE_ASAN
SOMETIMES(
if (_committed_words > 0) {
for (const MetaWord* p = _base; p < _base + _committed_words; p += os::vm_page_size()) {
Expand All @@ -294,6 +297,7 @@ void Metachunk::verify() const {
dummy = *(_base + _committed_words - 1);
}
)
#endif // !INCLUDE_ASAN
}
#endif // ASSERT

Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "utilities/align.hpp"
#include "utilities/debug.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/macros.hpp"
#include "utilities/ostream.hpp"

namespace metaspace {
Expand Down Expand Up @@ -433,13 +434,17 @@ void VirtualSpaceNode::verify_locked() const {
_commit_mask.verify();

// Verify memory against commit mask.
// Down here, from ASAN's view, this memory may be poisoned, since we only unpoison
// way up at the ChunkManager level.
#if !INCLUDE_ASAN
SOMETIMES(
for (MetaWord* p = base(); p < base() + used_words(); p += os::vm_page_size()) {
if (_commit_mask.is_committed_address(p)) {
test_access += *(uint*)p;
}
}
)
#endif // !INCLUDE_ASAN

assert(committed_words() <= word_size(), "Sanity");
assert_is_aligned(committed_words(), Settings::commit_granule_words());
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/runtime/os.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#include "utilities/count_trailing_zeros.hpp"
#include "utilities/defaultStream.hpp"
#include "utilities/events.hpp"
#include "utilities/macros.hpp"
#include "utilities/powerOfTwo.hpp"

#ifndef _WINDOWS
Expand Down Expand Up @@ -1144,6 +1145,8 @@ void os::print_location(outputStream* st, intptr_t x, bool verbose) {
return;
}

#if !INCLUDE_ASAN

bool accessible = is_readable_pointer(addr);

// Check if addr is a JNI handle.
Expand Down Expand Up @@ -1230,7 +1233,10 @@ void os::print_location(outputStream* st, intptr_t x, bool verbose) {
return;
}

#endif // !INCLUDE_ASAN

st->print_cr(INTPTR_FORMAT " is an unknown value", p2i(addr));

}

bool is_pointer_bad(intptr_t* ptr) {
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/services/mallocTracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "services/mallocTracker.hpp"
#include "services/memTracker.hpp"
#include "utilities/debug.hpp"
#include "utilities/macros.hpp"
#include "utilities/ostream.hpp"
#include "utilities/vmError.hpp"

Expand Down Expand Up @@ -198,6 +199,8 @@ void MallocTracker::deaccount(MallocHeader::FreeInfo free_info) {
bool MallocTracker::print_pointer_information(const void* p, outputStream* st) {
assert(MemTracker::enabled(), "NMT not enabled");

#if !INCLUDE_ASAN

address addr = (address)p;

// Carefully feel your way upwards and try to find a malloc header. Then check if
Expand Down Expand Up @@ -275,5 +278,8 @@ bool MallocTracker::print_pointer_information(const void* p, outputStream* st) {
}
return true;
}

#endif // !INCLUDE_ASAN

return false;
}
6 changes: 6 additions & 0 deletions src/hotspot/share/utilities/macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -635,4 +635,10 @@
#define NOT_CDS_JAVA_HEAP_RETURN_(code) { return code; }
#endif

#ifdef ADDRESS_SANITIZER
#define INCLUDE_ASAN 1
#else
#define INCLUDE_ASAN 0
#endif

#endif // SHARE_UTILITIES_MACROS_HPP
6 changes: 6 additions & 0 deletions test/hotspot/gtest/metaspace/test_virtualspacenode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "memory/metaspace/virtualSpaceNode.hpp"
#include "runtime/mutexLocker.hpp"
#include "sanitizers/address.hpp"
#include "utilities/macros.hpp"
#include "utilities/debug.hpp"
//#define LOG_PLEASE
#include "metaspaceGtestCommon.hpp"
Expand Down Expand Up @@ -156,6 +157,11 @@ class VirtualSpaceNodeTest {
// The chunk should be as far committed as was requested
EXPECT_GE(c->committed_words(), request_commit_words);

// At the VirtualSpaceNode level, all memory is still poisoned.
// Since we bypass the normal way of allocating chunks (ChunkManager::get_chunk), we
// need to unpoison this chunk.
ASAN_UNPOISON_MEMORY_REGION(c->base(), c->committed_words() * BytesPerWord);

// Zap committed portion.
DEBUG_ONLY(zap_range(c->base(), c->committed_words());)

Expand Down
5 changes: 5 additions & 0 deletions test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@
#include "memory/allocation.hpp"
#include "runtime/os.hpp"
#include "services/memTracker.hpp"
#include "sanitizers/address.hpp"
#include "utilities/debug.hpp"
#include "utilities/ostream.hpp"
#include "unittest.hpp"
#include "testutils.hpp"

#if !INCLUDE_ASAN

// This prefix shows up on any c heap corruption NMT detects. If unsure which assert will
// come, just use this one.
#define COMMON_NMT_HEAP_CORRUPTION_MESSAGE_PREFIX "NMT corruption"
Expand Down Expand Up @@ -161,3 +164,5 @@ TEST_VM(NMT, test_realloc) {
}
}
}

#endif // !INCLUDE_ASAN
5 changes: 5 additions & 0 deletions test/hotspot/gtest/nmt/test_nmt_cornercases.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "precompiled.hpp"
#include "memory/allocation.hpp"
#include "runtime/os.hpp"
#include "sanitizers/address.hpp"
#include "services/mallocHeader.inline.hpp"
#include "services/mallocTracker.hpp"
#include "services/memTracker.hpp"
Expand All @@ -38,6 +39,9 @@ static void check_expected_malloc_header(const void* payload, MEMFLAGS type, siz
EXPECT_EQ(hdr->flags(), type);
}

// ASAN complains about allocating very large sizes
#if !INCLUDE_ASAN

// Check that a malloc with an overflowing size is rejected.
TEST_VM(NMT, malloc_failure1) {
void* p = os::malloc(SIZE_MAX, mtTest);
Expand Down Expand Up @@ -85,6 +89,7 @@ TEST_VM(NMT, realloc_failure_overflowing_size) {
TEST_VM(NMT, realloc_failure_gigantic_size) {
check_failing_realloc(SIZE_MAX - M);
}
#endif // !INCLUDE_ASAN

static void* do_realloc(void* p, size_t old_size, size_t new_size, uint8_t old_content, bool check_nmt_header) {

Expand Down
5 changes: 5 additions & 0 deletions test/hotspot/gtest/nmt/test_nmt_locationprinting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "precompiled.hpp"
#include "memory/allocation.hpp"
#include "runtime/os.hpp"
#include "sanitizers/address.hpp"
#include "services/mallocHeader.inline.hpp"
#include "services/memTracker.hpp"
#include "unittest.hpp"
Expand All @@ -33,6 +34,8 @@
//#define LOG_PLEASE
#include "testutils.hpp"

#if !INCLUDE_ASAN

using ::testing::HasSubstr;

static void test_pointer(const void* p, bool expected_return_code, const char* expected_message) {
Expand Down Expand Up @@ -121,3 +124,5 @@ static void test_for_mmap(size_t sz, ssize_t offset) {

TEST_VM(NMT, location_printing_mmap_1) { test_for_mmap(os::vm_page_size(), 0); }
TEST_VM(NMT, location_printing_mmap_2) { test_for_mmap(os::vm_page_size(), os::vm_page_size() - 1); }

#endif // !INCLUDE_ASAN

1 comment on commit 324cef1

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.