RFR: 8283469: Don't use memset to initialize members in FileMapInfo and fix memory leak

Kim Barrett kbarrett at openjdk.java.net
Sun Mar 27 20:57:46 UTC 2022


On Tue, 22 Mar 2022 00:05:32 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

> `FileMapInfo` uses `memset` in its constructor to initialize data member, it is generally a bad idea. It can break things badly if adds a new member with a non-trivial constructor and `memset` is not suitable.
> 
> Besides, it leaks malloc'd `_header` member.
> 
> Test:
> 
> - [x]  hotspot_cds
> - [x] tier1
> - [x] tier2
> - [x] runtime/cds/DeterministicDump.java

Looks good.

memxxx of an object is only correct under specific circumstances.  A
case where it's not is when the object has virtual functions, which
FileMapInfo does in debug builds, because CHeapObj derives from
AllocatedObj in debug builds and that class has virtual functions.
Clearing the entire object is zeroing out vtable info, and trying to
call those functions will almost certainly crash.  So the current code
is already invalid; there's no need to wait for future members with
non-trivial constructors.

It looks like FileMapInfo might really a union of two cases, the
static and !static case, with some members associated with only one or
the other case.  In the future it might be worth considering whether
that could be made more formal.

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

Marked as reviewed by kbarrett (Reviewer).

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


More information about the hotspot-runtime-dev mailing list