JDK-8214363 : HeapWord should not be a fake class
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-11-27
  • Updated: 2019-02-27
  • Resolved: 2019-02-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 13
13 b09Fixed
Related Reports
Relates :  
Description
Memset is performed over HeapWord* which is "class {private: char* i;}" in 

src/hotspot/share/gc/parallel/objectStartArray.cpp: In member function 'void ObjectStartArray::set_covered_region(MemRegion)':
src/hotspot/share/gc/parallel/objectStartArray.cpp:106:56: error: 'void* memset(void*, int, size_t)' writing to an object of type 'class HeapWord' with 'private' member 'HeapWord::i' [-Werror=class-memaccess]
     memset(_blocks_region.end(), clean_block, expand_by);
                                                        ^
In file included from /src/hotspot/share/utilities/align.hpp:28,
                 from src/hotspot/share/runtime/globals.hpp:29,
                 from src/hotspot/share/memory/allocation.hpp:28,
                 from src/hotspot/share/classfile/classLoaderData.hpp:28,
                 from src/hotspot/share/precompiled/precompiled.hpp:34:
src/hotspot/share/utilities/globalDefinitions.hpp:174:7: note: 'class HeapWord' declared here
 class HeapWord {
       ^~~~~~~~
   ... (rest of output omitted)

HeapWord is defined as a "fake" class (with a private char* member) that is never really instantiated. -Wclass-memaccess is being triggered because the use of memset is bypassing access control to the data member of HeapWord.

And clean_block is -1.

Possible fix is to add (char*) cast similar to src/hotspot/share/gc/parallel/mutableNUMASpace.cpp

Another approach is to add HeapWord constructor that sets i and use placement new.
Comments
Proposed fix will be to change the definition of HeapWord, as described in 2018-12-12 comment. Changed the bug summary and description accordingly.
14-02-2019

I think the 2018-12-13 comment about the size calculation is incorrect. The size being calculated is the size of the object start array, not the size of the region it covers. The object start array has 1 byte per block in the covered region. So the calculation looks okay, if perhaps somewhat confusingly written.
14-02-2019

Rather than a direct call to memset, this probably ought to be calling Copy::fill_to_words or _bytes. That might solve the warning problem, and if it doesn't then there are probably issues with the Copy class that need to be addressed.
16-12-2018

I noticed one extra confusing thing -- how size is calculated. I suspect that size_t bytes_to_reserve = reserved_region.word_size() / block_size_in_words; and size_t requested_blocks_size_in_bytes = mr.word_size() / block_size_in_words should be size_t bytes_to_reserve = reserved_region.byte_size(); and size_t requested_blocks_size_in_bytes = mr.byte_size(); Probably it is not noticeable because top expressions give a number of blocks (few) which is stretched to a page size.
13-12-2018

I minimally implemented constructor approach and it doesn't look like a clean solution. It it still necessary to have -1 magic value exposed, 'clean' semantics besides of 'init' and there is anyway a mess in offset_addr_for_block(). And again, (char*) cast will still be there in os::numa_make_local() etc. So simple (char*) cast here looks more reasonable to me.
13-12-2018

An entirely different approach is to change HeapWord to: class HeapWordImpl; // Opaque type, never defined. typedef HeapWordImpl* HeapWord; and deal with the small amount of fallout from that change.
13-12-2018