RFR: 8266787: Potential overflow of pointer arithmetic in G1ArchiveAllocator

[Albert Mingkun Yang]

On Sun, 9 May 2021 19:11:19 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> Why is this line being moved?

Two reasons:

1. To make it more symmetric in updating `old_top`, `_bottom` and `_max`.


if (_max != _allocation_region->end()) {
  ... // updating the triplet in one way
} else {
  ... // updating them in another way
}


2. In general, the range of `[_bottom, _top]` marks the allocatable space, and `_top` fits in the middle. In the original code, when `alloc_new_region` fails, `_bottom == _max`, which leaves zero allocatable space. With this PR, `[_bottom, _top]` is still a valid allocatable space, but full.

> is that okay?

When `alloc_new_region` fails, `G1ArchiveAllocator::archive_mem_allocate` returns NULL. Callers seem to expect this method to *not* return NULL in the normal case; they either terminate the VM (`HeapShared::archive_heap_object`) or directly treat the return value as non-null (`HeapShared::copy_roots()` and `G1ArchiveAllocator::complete_archive`). Therefore, both the existing code and the PR are fine.

BTW, it seems that `_bottom` is only used for verification purpose; maybe remove it or make this more explicit.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3936