From e2407fd47e21b46473cbc1822c1cb67a28742bbd Mon Sep 17 00:00:00 2001 From: MBaesken Date: Wed, 9 Jul 2025 12:32:20 +0200 Subject: [PATCH 1/9] JDK-8360941 --- src/hotspot/share/memory/memRegion.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/memory/memRegion.hpp b/src/hotspot/share/memory/memRegion.hpp index 920efe7528844..86e86a783fe2d 100644 --- a/src/hotspot/share/memory/memRegion.hpp +++ b/src/hotspot/share/memory/memRegion.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2025, Oracle 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 @@ -63,7 +63,8 @@ class MemRegion { MemRegion minus(const MemRegion mr2) const; HeapWord* start() const { return _start; } - HeapWord* end() const { return _start + _word_size; } + // in the gtests we call end() with a _start == nullptr so adjust the addition to avoid ub + HeapWord* end() const { return reinterpret_cast(reinterpret_cast(_start) + (_word_size * sizeof(HeapWord))); } HeapWord* last() const { return _start + _word_size - 1; } void set_start(HeapWord* start) { _start = start; } From d31a4f396d2539d2449ad0f69cd70eb55be4bc56 Mon Sep 17 00:00:00 2001 From: MBaesken Date: Wed, 16 Jul 2025 16:12:23 +0200 Subject: [PATCH 2/9] Change the test, avoid using address 0 --- src/hotspot/share/memory/memRegion.hpp | 5 ++--- test/hotspot/gtest/gc/g1/test_freeRegionList.cpp | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/memory/memRegion.hpp b/src/hotspot/share/memory/memRegion.hpp index 86e86a783fe2d..aadacc22f993e 100644 --- a/src/hotspot/share/memory/memRegion.hpp +++ b/src/hotspot/share/memory/memRegion.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2024, Oracle 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 @@ -63,8 +63,7 @@ class MemRegion { MemRegion minus(const MemRegion mr2) const; HeapWord* start() const { return _start; } - // in the gtests we call end() with a _start == nullptr so adjust the addition to avoid ub - HeapWord* end() const { return reinterpret_cast(reinterpret_cast(_start) + (_word_size * sizeof(HeapWord))); } + HeapWord* end() const { return _start + _word_size; } HeapWord* last() const { return _start + _word_size - 1; } void set_start(HeapWord* start) { _start = start; } diff --git a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp index ea8027df2a715..e768f695e0179 100644 --- a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp +++ b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp @@ -44,7 +44,9 @@ TEST_OTHER_VM(G1FreeRegionList, length) { // Create a fake heap. It does not need to be valid, as the G1HeapRegion constructor // does not access it. - MemRegion heap(nullptr, num_regions_in_test * G1HeapRegion::GrainWords); + int val = 1; + HeapWord* ptr = reinterpret_cast(&val); + MemRegion heap(ptr, num_regions_in_test * G1HeapRegion::GrainWords); // Allocate a fake BOT because the G1HeapRegion constructor initializes // the BOT. From 850e92d36d59fa7f1978b1c622d1150e75e988cf Mon Sep 17 00:00:00 2001 From: MBaesken Date: Wed, 16 Jul 2025 16:37:48 +0200 Subject: [PATCH 3/9] blank --- src/hotspot/share/memory/memRegion.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/memory/memRegion.hpp b/src/hotspot/share/memory/memRegion.hpp index aadacc22f993e..68d698c1e73e6 100644 --- a/src/hotspot/share/memory/memRegion.hpp +++ b/src/hotspot/share/memory/memRegion.hpp @@ -63,7 +63,7 @@ class MemRegion { MemRegion minus(const MemRegion mr2) const; HeapWord* start() const { return _start; } - HeapWord* end() const { return _start + _word_size; } + HeapWord* end() const { return _start + _word_size; } HeapWord* last() const { return _start + _word_size - 1; } void set_start(HeapWord* start) { _start = start; } From f325d8774d49a972fea562424b67f4d5efa4fb7d Mon Sep 17 00:00:00 2001 From: MBaesken Date: Wed, 16 Jul 2025 16:39:05 +0200 Subject: [PATCH 4/9] blank --- src/hotspot/share/memory/memRegion.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/memory/memRegion.hpp b/src/hotspot/share/memory/memRegion.hpp index 68d698c1e73e6..920efe7528844 100644 --- a/src/hotspot/share/memory/memRegion.hpp +++ b/src/hotspot/share/memory/memRegion.hpp @@ -63,7 +63,7 @@ class MemRegion { MemRegion minus(const MemRegion mr2) const; HeapWord* start() const { return _start; } - HeapWord* end() const { return _start + _word_size; } + HeapWord* end() const { return _start + _word_size; } HeapWord* last() const { return _start + _word_size - 1; } void set_start(HeapWord* start) { _start = start; } From 842077ad6eb24d452c1dca9d54acd3c1d5da678c Mon Sep 17 00:00:00 2001 From: MBaesken Date: Thu, 17 Jul 2025 17:38:17 +0200 Subject: [PATCH 5/9] Avoid assertions, align address in test --- test/hotspot/gtest/gc/g1/test_freeRegionList.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp index e768f695e0179..6e9215e459b18 100644 --- a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp +++ b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp @@ -46,6 +46,8 @@ TEST_OTHER_VM(G1FreeRegionList, length) { // does not access it. int val = 1; HeapWord* ptr = reinterpret_cast(&val); + ptr = align_up(ptr, os::vm_page_size()); + MemRegion heap(ptr, num_regions_in_test * G1HeapRegion::GrainWords); // Allocate a fake BOT because the G1HeapRegion constructor initializes From f0888ceab15a2f9c94efa689d07cc727006d0d19 Mon Sep 17 00:00:00 2001 From: MBaesken Date: Fri, 18 Jul 2025 10:17:47 +0200 Subject: [PATCH 6/9] Simplify test coding --- test/hotspot/gtest/gc/g1/test_freeRegionList.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp index 6e9215e459b18..1e780d3a2c786 100644 --- a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp +++ b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp @@ -44,9 +44,8 @@ TEST_OTHER_VM(G1FreeRegionList, length) { // Create a fake heap. It does not need to be valid, as the G1HeapRegion constructor // does not access it. - int val = 1; - HeapWord* ptr = reinterpret_cast(&val); - ptr = align_up(ptr, os::vm_page_size()); + HeapWord val{}; + HeapWord* ptr = align_up(&val, os::vm_page_size()); MemRegion heap(ptr, num_regions_in_test * G1HeapRegion::GrainWords); From 0a9adaab2a2b77c1f068fa1a29434f0d53b48a09 Mon Sep 17 00:00:00 2001 From: MBaesken Date: Mon, 21 Jul 2025 15:45:05 +0200 Subject: [PATCH 7/9] Suggestion by Thomas Stuefe --- test/hotspot/gtest/gc/g1/test_freeRegionList.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp index 1e780d3a2c786..9d44ab53531b4 100644 --- a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp +++ b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp @@ -44,10 +44,10 @@ TEST_OTHER_VM(G1FreeRegionList, length) { // Create a fake heap. It does not need to be valid, as the G1HeapRegion constructor // does not access it. - HeapWord val{}; - HeapWord* ptr = align_up(&val, os::vm_page_size()); - - MemRegion heap(ptr, num_regions_in_test * G1HeapRegion::GrainWords); + const size_t szw = num_regions_in_test * G1HeapRegion::GrainWords; + const size_t sz = szw * BytesPerWord; + char* addr = os::reserve_memory(sz, mtTest); + MemRegion heap((HeapWord*)addr, szw); // Allocate a fake BOT because the G1HeapRegion constructor initializes // the BOT. @@ -90,5 +90,6 @@ TEST_OTHER_VM(G1FreeRegionList, length) { bot_storage->uncommit_regions(0, num_regions_in_test); delete bot_storage; + os::release_memory(addr, szw); FREE_C_HEAP_ARRAY(HeapWord, bot_data); } From 53d20a2de54a73192a3ced0265ba4442013b13ee Mon Sep 17 00:00:00 2001 From: MBaesken Date: Tue, 22 Jul 2025 09:45:24 +0200 Subject: [PATCH 8/9] Alignment --- test/hotspot/gtest/gc/g1/test_freeRegionList.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp index 9d44ab53531b4..76cd30adea5b7 100644 --- a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp +++ b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp @@ -46,7 +46,8 @@ TEST_OTHER_VM(G1FreeRegionList, length) { // does not access it. const size_t szw = num_regions_in_test * G1HeapRegion::GrainWords; const size_t sz = szw * BytesPerWord; - char* addr = os::reserve_memory(sz, mtTest); + const size_t lpsz = os::large_page_size(); + char* addr = os::reserve_memory_aligned(sz, lpsz, mtTest); MemRegion heap((HeapWord*)addr, szw); // Allocate a fake BOT because the G1HeapRegion constructor initializes From bf783386d29c3d6e14277838c5f09439c5440f21 Mon Sep 17 00:00:00 2001 From: MBaesken Date: Tue, 22 Jul 2025 16:12:18 +0200 Subject: [PATCH 9/9] Adjust alignment and freeing --- test/hotspot/gtest/gc/g1/test_freeRegionList.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp index 76cd30adea5b7..e61ce43fdb618 100644 --- a/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp +++ b/test/hotspot/gtest/gc/g1/test_freeRegionList.cpp @@ -46,8 +46,7 @@ TEST_OTHER_VM(G1FreeRegionList, length) { // does not access it. const size_t szw = num_regions_in_test * G1HeapRegion::GrainWords; const size_t sz = szw * BytesPerWord; - const size_t lpsz = os::large_page_size(); - char* addr = os::reserve_memory_aligned(sz, lpsz, mtTest); + char* addr = os::reserve_memory_aligned(sz, G1HeapRegion::GrainBytes, mtTest); MemRegion heap((HeapWord*)addr, szw); // Allocate a fake BOT because the G1HeapRegion constructor initializes @@ -91,6 +90,6 @@ TEST_OTHER_VM(G1FreeRegionList, length) { bot_storage->uncommit_regions(0, num_regions_in_test); delete bot_storage; - os::release_memory(addr, szw); + os::release_memory(addr, sz); FREE_C_HEAP_ARRAY(HeapWord, bot_data); }