Integrated: JDK-8263455: NMT: assert on registering a region which completely engulfs an existing region

[Thomas Stuefe]

On Thu, 11 Mar 2021 16:49:06 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> tl;dr:
> fixes an error in an NMT function which decides whether memory regions overlap. Shows up as an assert in gtests but the error is old and may cause wrong NMT results.
> 
> ---------------------
> 
> I am testing a prototype for JDK-8256844 which makes NMT late initializable. One of the many benefits that will bring is that we can now run gtests with NMT enabled.
> 
> Which exercises NMT in new ways: we promptly crash in the metaspace tests, which do a lot of arbitrary, random, but entirely valid range commits as part of the VirtualSpaceNode stress tests:
> 
> [ RUN ] metaspace.virtual_space_node_test_5_vm
> # To suppress the following error report, specify this argument
> # after -XX: or in .hotspotrc: SuppressErrorAt=/virtualMemoryTracker.hpp:243
> assert failed: assert(rgn.base() >= end()) failed: Sanity#
> # A fatal error has been detected by the Java Runtime Environment:
> #
> # Internal Error (/shared/projects/openjdk/jdk-jdk/source/src/hotspot/share/services/virtualMemoryTracker.hpp:243), pid=183572, tid=183572
> # assert(rgn.base() >= end()) failed: Sanity
> #
> Stack:
> (gdb) bt
> #0 0x00007ffff5b36cca in VirtualMemoryRegion::compare (this=0x555555b02278, rgn=...) at /shared/projects/openjdk/jdk-jdk/source/src/hotspot/share/services/virtualMemoryTracker.hpp:243
> #1 0x00007ffff6cf6823 in compare_committed_region (r1=..., r2=...) at /shared/projects/openjdk/jdk-...
> (gdb) p *this
> $1 = {_base_address = 0x7fffb24c0000 "", _size = 1179648}
> (gdb) p rgn
> $2 = (const VirtualMemoryRegion &) @0x555555b02318: {_base_address = 0x7fffb2460000 "", _size = 2424832}
> As we can see, the new committed to-be-registered region [0x7fffb2460000...7FFFB26B0000) completely engulfs an existing region [0x7fffb24c0000...0x7FFFB25E0000).
> 
> This triggers an assert in VirtualMemoryRegion:
> 
>   inline int compare(const VirtualMemoryRegion& rgn) const {
>     if (overlap_region(rgn.base(), rgn.size())) {
>       return 0;
>     } else if (base() >= rgn.end()) {
>       return 1;
>     } else {
>       assert(rgn.base() >= end(), "Sanity"); <<<
>       return -1;
>     }
>   }
> which calls
>   inline bool overlap_region(address addr, size_t sz) const {
>     assert(sz > 0, "Invalid size");
>     assert(size() > 0, "Invalid size");
>     return contain_address(addr) ||
>            contain_address(addr + sz - 1);
>   }
> but VirtualMemoryRegion::overlap_region does not handle the engulfing case correctly.
> 
> --------------
> 
> Fix: fixed the overlap function to handle the engulfing case.
> 
> Tests: GAs; manually executed runtime/NMT; manually tested (with my prototype VM) that the gtest works after applying this fix.

This pull request has now been integrated.

Changeset: 444a80b9
Author:    Thomas Stuefe <stuefe at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/444a80b9
Stats:     2 lines in 1 file changed: 0 ins; 1 del; 1 mod

8263455: NMT: assert on registering a region which completely engulfs an existing region

Reviewed-by: zgu, coleenp

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

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