Skip to content

8360941: [ubsan] MemRegion::end() shows runtime error: applying non-zero offset 8388608 to null pointer #26216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

8360941: [ubsan] MemRegion::end() shows runtime error: applying non-zero offset 8388608 to null pointer #26216

Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e2407fd
JDK-8360941
MBaesken Jul 9, 2025
d31a4f3
Change the test, avoid using address 0
MBaesken Jul 16, 2025
850e92d
blank
MBaesken Jul 16, 2025
f325d87
blank
MBaesken Jul 16, 2025
842077a
Avoid assertions, align address in test
MBaesken Jul 17, 2025
f0888ce
Simplify test coding
MBaesken Jul 18, 2025
0a9adaa
Suggestion by Thomas Stuefe
MBaesken Jul 21, 2025
53d20a2
Alignment
MBaesken Jul 22, 2025
bf78338
Adjust alignment and freeing
MBaesken Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion test/hotspot/gtest/gc/g1/test_freeRegionList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ 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);
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);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
char* addr = os::reserve_memory_aligned(sz, lpsz, mtTest);
char* addr = os::reserve_memory_aligned(sz, G1HeapRegion::GrainBytes, mtTest);

@MBaesken you want to align the start address at region size; not sure the G1 code we call in this test cares, but it might.

MemRegion heap((HeapWord*)addr, szw);
Copy link

Choose a reason for hiding this comment

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

So far as I can tell, there's no guarantee that os::reserve_memory will return an address with any
particular alignment. Since the earlier attempt with unaligned storage failed, it may only be by accident
that this isn't failing as well. We have os::reserve_memory_aligned, or could add an extra region to
the desired size and align up the result.

Copy link
Member

Choose a reason for hiding this comment

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

os::reserve_memory addresses are always regular-page-aligned. But os::reserve_memory_aligned may be better here since I guess the addresses would better have been region-size-aligned, so aligned to G1HeapRegion::GrainWords. That could be larger than system page size.

Copy link

Choose a reason for hiding this comment

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

Too bad the only way to find out about that alignment behavior is to dig into the sources for all ports
and read the documentation for the underlying OS function. :(


// Allocate a fake BOT because the G1HeapRegion constructor initializes
// the BOT.
Expand Down Expand Up @@ -87,5 +91,6 @@ TEST_OTHER_VM(G1FreeRegionList, length) {

bot_storage->uncommit_regions(0, num_regions_in_test);
delete bot_storage;
os::release_memory(addr, szw);
Copy link
Member

Choose a reason for hiding this comment

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

Wrong size; release_memory takes bytes, not words. Use sz.

This explains also the crash on Windows you told me about in your mail. Was this patch also in the testing queue?

FREE_C_HEAP_ARRAY(HeapWord, bot_data);
}