diff --git a/src/hotspot/share/nmt/mallocHeader.hpp b/src/hotspot/share/nmt/mallocHeader.hpp index 8472b5f8ce888..bd97ecebda129 100644 --- a/src/hotspot/share/nmt/mallocHeader.hpp +++ b/src/hotspot/share/nmt/mallocHeader.hpp @@ -39,14 +39,11 @@ class outputStream; * If NMT is active (state >= summary), we need to track allocations. A simple and cheap way to * do this is by using malloc headers. * - * The user allocation is preceded by a header and is immediately followed by a (possibly unaligned) - * footer canary: - * - * +--------------+------------- .... ------------------+-----+ - * | header | user | can | - * | | allocation | ary | - * +--------------+------------- .... ------------------+-----+ - * 16 bytes user size 2 byte + * +--------------+------------- .... ------------------+ + * | header | user | + * | | allocation | + * +--------------+------------- .... ------------------+ + * 16 bytes user size * * Alignment: * @@ -63,28 +60,20 @@ class outputStream; * * 8 9 10 11 12 13 14 15 16 ++ * +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------ - * ... | malloc site table marker | flags | unused | canary | ... User payload .... + * ... | malloc site table marker | tags | unused | ... User payload .... * +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------ * * Layout on 32-bit: * * 0 1 2 3 4 5 6 7 * +--------+--------+--------+--------+--------+--------+--------+--------+ - * | alt. canary | 32-bit size | ... + * | 32-bit size | ... * +--------+--------+--------+--------+--------+--------+--------+--------+ * * 8 9 10 11 12 13 14 15 16 ++ * +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------ - * ... | malloc site table marker | flags | unused | canary | ... User payload .... + * ... | malloc site table marker | tags | unused | ... User payload .... * +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------ - * - * Notes: - * - We have a canary in the two bytes directly preceding the user payload. That allows us to - * catch negative buffer overflows. - * - On 32-bit, due to the smaller size_t, we have some bits to spare. So we also have a second - * canary at the very start of the malloc header (generously sized 32 bits). - * - The footer canary consists of two bytes. Since the footer location may be unaligned to 16 bits, - * the bytes are stored individually. */ class MallocHeader { @@ -93,27 +82,13 @@ class MallocHeader { const size_t _size; const uint32_t _mst_marker; const MemTag _mem_tag; - const uint8_t _unused; - uint16_t _canary; - - static const uint16_t _header_canary_live_mark = 0xE99E; - static const uint16_t _header_canary_dead_mark = 0xD99D; - static const uint16_t _footer_canary_live_mark = 0xE88E; - static const uint16_t _footer_canary_dead_mark = 0xD88D; - NOT_LP64(static const uint32_t _header_alt_canary_live_mark = 0xE99EE99E;) - NOT_LP64(static const uint32_t _header_alt_canary_dead_mark = 0xD88DD88D;) + const uint8_t _unused[3]; // We discount sizes larger than these static const size_t max_reasonable_malloc_size = LP64_ONLY(256 * G) NOT_LP64(3500 * M); void print_block_on_error(outputStream* st, address bad_address) const; - static uint16_t build_footer(uint8_t b1, uint8_t b2) { return (uint16_t)(((uint16_t)b1 << 8) | (uint16_t)b2); } - - uint8_t* footer_address() const { return ((address)this) + sizeof(MallocHeader) + _size; } - uint16_t get_footer() const { return build_footer(footer_address()[0], footer_address()[1]); } - void set_footer(uint16_t v) { footer_address()[0] = (uint8_t)(v >> 8); footer_address()[1] = (uint8_t)v; } - template inline static OutTypeParam resolve_checked_impl(InTypeParam memblock); @@ -127,7 +102,7 @@ class MallocHeader { inline MallocHeader(size_t size, MemTag mem_tag, uint32_t mst_marker); - inline static size_t malloc_overhead() { return sizeof(MallocHeader) + sizeof(uint16_t); } + inline static size_t malloc_overhead() { return sizeof(MallocHeader); } inline size_t size() const { return _size; } inline MemTag mem_tag() const { return _mem_tag; } inline uint32_t mst_marker() const { return _mst_marker; } @@ -136,15 +111,6 @@ class MallocHeader { FreeInfo free_info() { return FreeInfo{this->size(), this->mem_tag(), this->mst_marker()}; } - inline void mark_block_as_dead(); - inline void revive(); - - - bool is_dead() const { return _canary == _header_canary_dead_mark; } - bool is_live() const { return _canary == _header_canary_live_mark; } - - // Used for debugging purposes only. Check header if it could constitute a valid (live or dead) header. - inline bool looks_valid() const; // If block is broken, fill in a short descriptive text in out, // an option pointer to the corruption in p_corruption, and return false. diff --git a/src/hotspot/share/nmt/mallocHeader.inline.hpp b/src/hotspot/share/nmt/mallocHeader.inline.hpp index 34ec891d33f78..654ec3f6c698e 100644 --- a/src/hotspot/share/nmt/mallocHeader.inline.hpp +++ b/src/hotspot/share/nmt/mallocHeader.inline.hpp @@ -36,30 +36,11 @@ inline MallocHeader::MallocHeader(size_t size, MemTag mem_tag, uint32_t mst_marker) : _size(size), _mst_marker(mst_marker), _mem_tag(mem_tag), - _unused(0), _canary(_header_canary_live_mark) + _unused() { assert(size < max_reasonable_malloc_size, "Too large allocation size?"); - // On 32-bit we have some bits more, use them for a second canary - // guarding the start of the header. - NOT_LP64(_alt_canary = _header_alt_canary_live_mark;) - set_footer(_footer_canary_live_mark); // set after initializing _size } -inline void MallocHeader::revive() { - assert(_canary == _header_canary_dead_mark, "must be dead"); - assert(get_footer() == _footer_canary_dead_mark, "must be dead"); - NOT_LP64(assert(_alt_canary == _header_alt_canary_dead_mark, "must be dead")); - _canary = _header_canary_live_mark; - NOT_LP64(_alt_canary = _header_alt_canary_live_mark); - set_footer(_footer_canary_live_mark); -} - -// The effects of this method must be reversible with MallocHeader::revive() -inline void MallocHeader::mark_block_as_dead() { - _canary = _header_canary_dead_mark; - NOT_LP64(_alt_canary = _header_alt_canary_dead_mark); - set_footer(_footer_canary_dead_mark); -} inline bool MallocHeader::is_valid_malloced_pointer(const void* payload, char* msg, size_t msglen) { // Handle the pointer as an integral type @@ -116,50 +97,16 @@ inline const MallocHeader* MallocHeader::resolve_checked(const void* memblock) { return MallocHeader::resolve_checked_impl(memblock); } - -// Used for debugging purposes only. Check header if it could constitute a valid (live or dead) header. -inline bool MallocHeader::looks_valid() const { - // Note: we define these restrictions loose enough to also catch moderately corrupted blocks. - // E.g. we don't check footer canary. - return ( (_canary == _header_canary_live_mark NOT_LP64(&& _alt_canary == _header_alt_canary_live_mark)) || - (_canary == _header_canary_dead_mark NOT_LP64(&& _alt_canary == _header_alt_canary_dead_mark)) ) && - _size > 0 && _size < max_reasonable_malloc_size; -} - inline bool MallocHeader::check_block_integrity(char* msg, size_t msglen, address* p_corruption) const { // Note: if you modify the error messages here, make sure you // adapt the associated gtests too. - // Check header canary - if (_canary != _header_canary_live_mark) { - *p_corruption = (address)this; - jio_snprintf(msg, msglen, "header canary broken"); - return false; - } - -#ifndef _LP64 - // On 32-bit we have a second canary, check that one too. - if (_alt_canary != _header_alt_canary_live_mark) { - *p_corruption = (address)this; - jio_snprintf(msg, msglen, "header canary broken"); - return false; - } -#endif - // Does block size seems reasonable? if (_size >= max_reasonable_malloc_size) { *p_corruption = (address)this; jio_snprintf(msg, msglen, "header looks invalid (weirdly large block size)"); return false; } - - // Check footer canary - if (get_footer() != _footer_canary_live_mark) { - *p_corruption = footer_address(); - jio_snprintf(msg, msglen, "footer canary broken at " PTR_FORMAT " (buffer overflow?)", - p2i(footer_address())); - return false; - } return true; } diff --git a/src/hotspot/share/nmt/mallocTracker.cpp b/src/hotspot/share/nmt/mallocTracker.cpp index 3c3236d2cbcce..12bf4b7831bcb 100644 --- a/src/hotspot/share/nmt/mallocTracker.cpp +++ b/src/hotspot/share/nmt/mallocTracker.cpp @@ -206,8 +206,6 @@ void* MallocTracker::record_free_block(void* memblock) { deaccount(header->free_info()); - header->mark_block_as_dead(); - return (void*)header; } @@ -217,99 +215,3 @@ void MallocTracker::deaccount(MallocHeader::FreeInfo free_info) { MallocSiteTable::deallocation_at(free_info.size, free_info.mst_marker); } } - -// Given a pointer, look for the containing malloc block. -// Print the block. Note that since there is very low risk of memory looking -// accidentally like a valid malloc block header (canaries and all) so this is not -// totally failproof and may give a wrong answer. It is safe in that it will never -// crash, even when encountering unmapped memory. -bool MallocTracker::print_pointer_information(const void* p, outputStream* st) { - assert(MemTracker::enabled(), "NMT not enabled"); - -#if !INCLUDE_ASAN - - address addr = (address)p; - - if (p2u(addr) < MAX2(os::vm_min_address(), (size_t)16 * M)) { - return false; // bail out - } - - // Carefully feel your way upwards and try to find a malloc header. Then check if - // we are within the block. - // We give preference to found live blocks; but if no live block had been found, - // but the pointer points into remnants of a dead block, print that instead. - const MallocHeader* likely_dead_block = nullptr; - const MallocHeader* likely_live_block = nullptr; - { - const size_t smallest_possible_alignment = sizeof(void*); - uintptr_t here = (uintptr_t)align_down(addr, smallest_possible_alignment); - uintptr_t end = MAX2(smallest_possible_alignment, here - (0x1000 + sizeof(MallocHeader))); // stop searching after 4k - for (; here >= end; here -= smallest_possible_alignment) { - // JDK-8306561: cast to a MallocHeader needs to guarantee it can reside in readable memory - if (!os::is_readable_range((void*)here, (void*)(here + sizeof(MallocHeader)))) { - break; // Probably OOB, give up - } - const MallocHeader* const candidate = (const MallocHeader*)here; - if (!candidate->looks_valid()) { - // This is definitely not a header, go on to the next candidate. - continue; - } - - // fudge factor: - // We don't report blocks for which p is clearly outside of. That would cause us to return true and possibly prevent - // subsequent tests of p, see os::print_location(). But if p is just outside of the found block, this may be a - // narrow oob error and we'd like to know that. - const int fudge = 8; - const address start_block = (address)candidate; - const address start_payload = (address)(candidate + 1); - const address end_payload = start_payload + candidate->size(); - const address end_payload_plus_fudge = end_payload + fudge; - if (addr >= start_block && addr < end_payload_plus_fudge) { - // We found a block the pointer is pointing into, or almost into. - // If its a live block, we have our info. If its a dead block, we still - // may be within the borders of a larger live block we have not found yet - - // continue search. - if (candidate->is_live()) { - likely_live_block = candidate; - break; - } else { - likely_dead_block = candidate; - continue; - } - } - } - } - - // If we've found a reasonable candidate. Print the info. - const MallocHeader* block = likely_live_block != nullptr ? likely_live_block : likely_dead_block; - if (block != nullptr) { - const char* where = nullptr; - const address start_block = (address)block; - const address start_payload = (address)(block + 1); - const address end_payload = start_payload + block->size(); - if (addr < start_payload) { - where = "into header of"; - } else if (addr < end_payload) { - where = "into"; - } else { - where = "just outside of"; - } - st->print_cr(PTR_FORMAT " %s %s malloced block starting at " PTR_FORMAT ", size %zu, tag %s", - p2i(p), where, - (block->is_dead() ? "dead" : "live"), - p2i(block + 1), // lets print the payload start, not the header - block->size(), NMTUtil::tag_to_enum_name(block->mem_tag())); - if (MemTracker::tracking_level() == NMT_detail) { - NativeCallStack ncs; - if (MallocSiteTable::access_stack(ncs, *block)) { - ncs.print_on(st); - st->cr(); - } - } - return true; - } - -#endif // !INCLUDE_ASAN - - return false; -} diff --git a/src/hotspot/share/nmt/memTracker.cpp b/src/hotspot/share/nmt/memTracker.cpp index 8f84948fa5ce4..0e50837bd6d77 100644 --- a/src/hotspot/share/nmt/memTracker.cpp +++ b/src/hotspot/share/nmt/memTracker.cpp @@ -122,9 +122,7 @@ void MemTracker::final_report(outputStream* output) { // Given an unknown pointer, check if it points into a known region; print region if found // and return true; false if not found. bool MemTracker::print_containing_region(const void* p, outputStream* out) { - return enabled() && - (MallocTracker::print_pointer_information(p, out) || - VirtualMemoryTracker::print_containing_region(p, out)); + return enabled() && VirtualMemoryTracker::print_containing_region(p, out); } void MemTracker::report(bool summary_only, outputStream* output, size_t scale) { diff --git a/src/hotspot/share/runtime/os.cpp b/src/hotspot/share/runtime/os.cpp index d266b632ed1c3..0216f8aefe462 100644 --- a/src/hotspot/share/runtime/os.cpp +++ b/src/hotspot/share/runtime/os.cpp @@ -718,15 +718,11 @@ void* os::realloc(void *memblock, size_t size, MemTag mem_tag, const NativeCallS NMTUtil::tag_to_name(mem_tag), NMTUtil::tag_to_name(header->mem_tag())); const MallocHeader::FreeInfo free_info = header->free_info(); - header->mark_block_as_dead(); - // the real realloc ALLOW_C_FUNCTION(::realloc, void* const new_outer_ptr = ::realloc(header, new_outer_size);) if (new_outer_ptr == nullptr) { // realloc(3) failed and the block still exists. - // We have however marked it as dead, revert this change. - header->revive(); return nullptr; } // realloc(3) succeeded, variable header now points to invalid memory and we need to deaccount the old block. diff --git a/test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp b/test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp index f987d83755c9d..86afa472667e1 100644 --- a/test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp +++ b/test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp @@ -49,104 +49,20 @@ } \ } -/////// - -static void test_overwrite_front() { - address p = (address) os::malloc(1, mtTest); - *(p - 1) = 'a'; - os::free(p); -} - -DEFINE_TEST(test_overwrite_front, "header canary broken") - -/////// - -static void test_overwrite_back() { - address p = (address) os::malloc(1, mtTest); - *(p + 1) = 'a'; - os::free(p); -} - -DEFINE_TEST(test_overwrite_back, "footer canary broken") - -/////// - -// A overwrite farther away from the NMT header; the report should show the hex dump split up -// in two parts, containing both header and corruption site. -static void test_overwrite_back_long(size_t distance) { - address p = (address) os::malloc(distance, mtTest); - *(p + distance) = 'a'; - os::free(p); -} -static void test_overwrite_back_long_aligned_distance() { test_overwrite_back_long(0x2000); } -DEFINE_TEST(test_overwrite_back_long_aligned_distance, "footer canary broken") -static void test_overwrite_back_long_unaligned_distance() { test_overwrite_back_long(0x2001); } -DEFINE_TEST(test_overwrite_back_long_unaligned_distance, "footer canary broken") - -/////// - -static void test_double_free() { - address p = (address) os::malloc(1, mtTest); - os::free(p); - // Now a double free. Note that this is susceptible to concurrency issues should - // a concurrent thread have done a malloc and gotten the same address after the - // first free. To decrease chance of this happening, we repeat the double free - // several times. - for (int i = 0; i < 100; i ++) { - os::free(p); - } -} - -// What assertion message we will see depends on whether the VM wipes the memory-to-be-freed -// on the first free(), and whether the libc uses the freed memory to store bookkeeping information. -// If the death marker in the header is still intact after the first free, we will recognize this as -// double free; if it got wiped, we should at least see a broken header canary. -// The message would be either -// - "header canary broken" or -// - "header canary dead (double free?)". -// However, since gtest regex expressions do not support unions (a|b), I search for a reasonable -// subset here. -DEFINE_TEST(test_double_free, "header canary") - -/////// - static void test_invalid_block_address() { // very low, like the result of an overflow or of accessing a null this pointer os::free((void*)0x100); } DEFINE_TEST(test_invalid_block_address, "invalid block address") -/////// - static void test_unaliged_block_address() { address p = (address) os::malloc(1, mtTest); os::free(p + 6); } DEFINE_TEST(test_unaliged_block_address, "block address is unaligned"); -/////// - -// Test that we notice block corruption on realloc too -static void test_corruption_on_realloc(size_t s1, size_t s2) { - address p1 = (address) os::malloc(s1, mtTest); - *(p1 + s1) = 'a'; - address p2 = (address) os::realloc(p1, s2, mtTest); - - // Still here? - tty->print_cr("NMT did not detect corruption on os::realloc?"); - // Note: don't use ASSERT here, that does not work as expected in death tests. Just - // let the test run its course, it should notice something is amiss. -} -static void test_corruption_on_realloc_growing() { test_corruption_on_realloc(0x10, 0x11); } -DEFINE_TEST(test_corruption_on_realloc_growing, COMMON_NMT_HEAP_CORRUPTION_MESSAGE_PREFIX); -static void test_corruption_on_realloc_shrinking() { test_corruption_on_realloc(0x11, 0x10); } -DEFINE_TEST(test_corruption_on_realloc_shrinking, COMMON_NMT_HEAP_CORRUPTION_MESSAGE_PREFIX); - -/////// - // realloc is the trickiest of the bunch. Test that realloc works and correctly takes over -// NMT header and footer to the resized block. We just test that nothing crashes - if the -// header/footer get corrupted, NMT heap corruption checker will trigger alert on os::free()). +// NMT header and footer to the resized block. We just test that nothing crashes. TEST_VM(NMT, test_realloc) { // We test both directions (growing and shrinking) and a small range for each to cover all // size alignment variants. Should not matter, but this should be cheap. diff --git a/test/hotspot/gtest/nmt/test_nmt_cornercases.cpp b/test/hotspot/gtest/nmt/test_nmt_cornercases.cpp index d1da65cb39656..0e0f4cd5ff79c 100644 --- a/test/hotspot/gtest/nmt/test_nmt_cornercases.cpp +++ b/test/hotspot/gtest/nmt/test_nmt_cornercases.cpp @@ -146,16 +146,3 @@ TEST_VM(NMT, random_reallocs) { os::free(p); } - -TEST_VM(NMT, HeaderKeepsIntegrityAfterRevival) { - if (!MemTracker::enabled()) { - return; - } - size_t some_size = 16; - void* p = os::malloc(some_size, mtTest); - ASSERT_NOT_NULL(p) << "Failed to malloc()"; - MallocHeader* hdr = MallocTracker::malloc_header(p); - hdr->mark_block_as_dead(); - hdr->revive(); - check_expected_malloc_header(p, mtTest, some_size); -} diff --git a/test/hotspot/gtest/nmt/test_nmt_locationprinting.cpp b/test/hotspot/gtest/nmt/test_nmt_locationprinting.cpp index e0e3c28910225..f46440e8500c6 100644 --- a/test/hotspot/gtest/nmt/test_nmt_locationprinting.cpp +++ b/test/hotspot/gtest/nmt/test_nmt_locationprinting.cpp @@ -47,71 +47,6 @@ static void test_pointer(const void* p, bool expected_return_code, const char* e } } -static void test_for_live_c_heap_block(size_t sz, ssize_t offset) { - char* c = NEW_C_HEAP_ARRAY(char, sz, mtTest); - LOG_HERE("C-block starts " PTR_FORMAT ", size %zu.", p2i(c), sz); - memset(c, 0, sz); - if (MemTracker::enabled()) { - const char* expected_string = "into live malloced block"; - if (offset < 0) { - expected_string = "into header of live malloced block"; - } else if ((size_t)offset >= sz) { - expected_string = "just outside of live malloced block"; - } - test_pointer(c + offset, true, expected_string); - } else { - // NMT disabled: we should see nothing. - test_pointer(c + offset, false, ""); - } - FREE_C_HEAP_ARRAY(char, c); -} - -#ifdef LINUX -static void test_for_dead_c_heap_block(size_t sz, ssize_t offset) { - if (!MemTracker::enabled()) { - return; - } - char* c = NEW_C_HEAP_ARRAY(char, sz, mtTest); - LOG_HERE("C-block starts " PTR_FORMAT ", size %zu.", p2i(c), sz); - memset(c, 0, sz); - // We cannot just free the allocation to try dead block printing, since the memory - // may be immediately reused by concurrent code. Instead, we mark the block as dead - // manually, and revert that before freeing it. - MallocHeader* const hdr = MallocHeader::resolve_checked(c); - hdr->mark_block_as_dead(); - - const char* expected_string = "into dead malloced block"; - if (offset < 0) { - expected_string = "into header of dead malloced block"; - } else if ((size_t)offset >= sz) { - expected_string = "just outside of dead malloced block"; - } - - test_pointer(c + offset, true, expected_string); - - hdr->revive(); - FREE_C_HEAP_ARRAY(char, c); -} -#endif - -TEST_VM(NMT, location_printing_cheap_live_1) { test_for_live_c_heap_block(2 * K, 0); } // start of payload -TEST_VM(NMT, location_printing_cheap_live_2) { test_for_live_c_heap_block(2 * K, -7); } // into header -TEST_VM(NMT, location_printing_cheap_live_3) { test_for_live_c_heap_block(2 * K, K + 1); } // into payload -TEST_VM(NMT, location_printing_cheap_live_4) { test_for_live_c_heap_block(2 * K, K + 2); } // into payload (check for even/odd errors) -TEST_VM(NMT, location_printing_cheap_live_5) { test_for_live_c_heap_block(2 * K + 1, 2 * K + 2); } // just outside payload -TEST_VM(NMT, location_printing_cheap_live_6) { test_for_live_c_heap_block(4, 0); } // into a very small block -TEST_VM(NMT, location_printing_cheap_live_7) { test_for_live_c_heap_block(4, 4); } // just outside a very small block - -#ifdef LINUX -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_1) { test_for_dead_c_heap_block(2 * K, 0); } // start of payload -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_2) { test_for_dead_c_heap_block(2 * K, -7); } // into header -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_3) { test_for_dead_c_heap_block(2 * K, K + 1); } // into payload -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_4) { test_for_dead_c_heap_block(2 * K, K + 2); } // into payload (check for even/odd errors) -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_5) { test_for_dead_c_heap_block(2 * K + 1, 2 * K + 2); } // just outside payload -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_6) { test_for_dead_c_heap_block(4, 0); } // into a very small block -TEST_VM(NMT, DISABLED_location_printing_cheap_dead_7) { test_for_dead_c_heap_block(4, 4); } // just outside a very small block -#endif - static void test_for_mmap(size_t sz, ssize_t offset) { char* addr = os::reserve_memory(sz, false, mtTest); if (MemTracker::enabled()) {