RFR: 8225779: Remove unused CollectedHeap::cell_header_size()

Erik Österlund erik.osterlund at oracle.com
Fri Jun 14 09:26:08 UTC 2019


Hi Per,

I don't think it is a good idea to completely revert the changes made to 
MacroAssembler::needs_explicit_null_check(). This was not just done for 
interfacing. The code we had before was actually wrong.

We called that function with both addresses (from signal handlers) and 
offsets (from the compiler), and tried to look at the passed in value 
whether it quacks like an offset or an address based on its value. If it 
looked like an address it would first be normalized to look like an 
offset. But it was possible that a much-larger-than-page offset is 
passed in that actually looks like an address according to the address 
quacking code, which turns the supposedly address looking value into an 
offset normalized form that looks like it wouldn't need explicit null 
checks because in that form the offset is within one page, while the 
original offset passed in totally would need an explicit null check 
really, because it totally wasn't within that one page offset we allowed.

To make this more sound, the function was split into two - one used for 
addresses passed in by signal handlers, and one used for offsets used by 
the compiler. This completely removed the value based check to determine 
if the argument looks like an address or offset. I would definitely like 
to keep that structure going forward, because otherwise that bug will be 
re-introduced.

Otherwise, I do encourage removing the now unnecessary cell header size 
abstraction.

/Erik

On 2019-06-14 08:29, Per Liden wrote:
> CollectedHeap::cell_header_size() is no longer used and can be removed. 
> This patch is a hg backout of [1] where it was introduced, plus the two 
> followup bug fixes [2] & [3].
> 
> This is a clean backout, with the exception of light editing needed in 
> two places, because:
> 
> 1) The function Universe::narrow_oop_base() has since been renamed to 
> CompressedOops::base().
> 2) The include guard CPU_ARM_VM_MACROASSEMBLER_ARM_HPP has since been 
> renamed to CPU_ARM_MACROASSEMBLER_ARM_HPP.
> 
> Testing: Tier1-3 on all Oracle platforms
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8225779
> Webrev: http://cr.openjdk.java.net/~pliden/8225779/webrev.0
> 
> /Per
> 
> [1] http://hg.openjdk.java.net/jdk/jdk/rev/4ad404da0088
> [2] http://hg.openjdk.java.net/jdk/jdk/rev/f642ede2eb07
> [3] http://hg.openjdk.java.net/jdk/jdk/rev/18bd95c0e463


More information about the hotspot-dev mailing list