RFR: JDK-8295865: Several issues with os::realloc

[David Holmes]

On Wed, 2 Nov 2022 05:23:57 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/runtime/os.cpp line 732:
>> 
>>> 730:       // NMT header then, since we marked it dead already. Otherwise subsequent os::realloc()
>>> 731:       // or os::free() calls would trigger block integrity asserts.
>>> 732:       void* p = MemTracker::record_malloc(old_outer_ptr, old_size, memflags, stack);
>> 
>> I'm not familiar with NMT details - are `memflags` and `stack` guaranteed to be the same as the original allocation?
>
> Not guaranteed, but it can happen.
> 
> `memflags` can theoretically be different, the memory would then just be re-booked to the new category. In practice, nobody does this.
> 
> `stack` can differ. Memory is accounted per { stack, flags } tupel in a hash table. Only matters for nmt "details" level though.

Don't you need to restore things exactly the way they were?

>> test/hotspot/gtest/testutils.hpp line 57:
>> 
>>> 55: #define ASSERT_NULL(p)      ASSERT_EQ(p2i(p), 0)
>>> 56: #define EXPECT_NOT_NULL(p)  EXPECT_NE(p2i(p), 0)
>>> 57: #define EXPECT_NULL(p)      EXPECT_EQ(p2i(p), 0)
>> 
>> These aren't "convenience asserts" - should they be somewhere else? What does EXPECT_NULL actually do?
>
> They extend the existing `ASSERT_xxx` and `EXPECT_xxx` macros we get from gtest (in gtest.h). I cannot add them to gtest, of course, they have to live within our gtest wrapper code. This is as good a place as any IMO. Note we do similar things with other test helpers, e.g. threadHelper.inline.hpp.

I meant different place in the file, rather than under "convenience asserts" when they are not assertions. I remain ignorant as to what the behaviour of EXPECT_X(p) is actually meant to be ??

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

PR: https://git.openjdk.org/jdk/pull/10857