On Mon, 4 Nov 2024 09:24:13 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
>> If I recall correctly this was a bug where one of the stackChunk fields was
>> allocated in that gap, but since we didn't zeroed it out it would start with
>> some invalid value. I guess the reason why we are not hitting this today is
>> because one of the fields we do initialize (sp/bottom/size) is being
>> allocated there, but with the new fields I added to stackChunk that is not
>> the case anymore.
>
> This code in `StackChunkAllocator::initialize` mimics the clearing code in:
>
> void MemAllocator::mem_clear(HeapWord* mem) const {
> assert(mem != nullptr, "cannot initialize null object");
> const size_t hs = oopDesc::header_size();
> assert(_word_size >= hs, "unexpected object size");
> oopDesc::set_klass_gap(mem, 0);
> Copy::fill_to_aligned_words(mem + hs, _word_size - hs);
> }
>
>
> but with a limited amount of clearing at the end of the object, IIRC. So,
> this looks like a good fix. With JEP 450 we have added an assert to
> set_klass_gap and changed the code in `mem_clear` to be:
>
> if (oopDesc::has_klass_gap()) {
> oopDesc::set_klass_gap(mem, 0);
> }
>
>
> So, unchanged, this code will start to assert when the to projects merge.
> Maybe it would be nice to make a small/trivial upstream PR to add this code
> to both `MemAllocator::mem_clear` and `StackChunkAllocator::initialize`?
Thanks for confirming. I added the check here which I think should cover any
merge order.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1828614946
