[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Integrated: 8331953: ubsan: metaspaceShared.cpp:1305:57: runtime error: applying non-zero offset 128
From:       Thomas Stuefe <stuefe () openjdk ! org>
Date:       2024-05-16 10:58:46
Message-ID: lHFjL3e2fL1RsrPlqBrXgHuOBzP4oKlYjct-sqwA2Wo=.ba7a06ab-cc3b-45d2-9756-94647dc4b109 () github ! com
[Download RAW message or body]

On Thu, 9 May 2024 06:28:48 GMT, Thomas Stuefe <stuefe@openjdk.org> wrote:

> Reported by @MBaesken at SAP. ubsan complains about this line:
> 
> 
> const size_t ccs_begin_offset = align_up(base_address + archive_space_size,
> class_space_alignment) - base_address;
> 
> 
> `base_address` here is the wish address, with NULL being an explicitly allowed \
> value that indicates "no preference". The line calculates the offset of the class \
> space within the future combined CDS+class-space mapping. Ubsan complains about \
> `base_address` being possibly NULL. 
> Ubsan is missing the point somewhat. The addition is not a problem, on our \
> platforms at least. 
> However, it highlights a slight incorrectness (which it did not notice): \
> `base_address` is the wish for the future base of the Klass range. That wish is not \
> guaranteed to be fulfilled; the eventual start of the Klass range could be \
> somewhere else. Therefore, calculating the class space offset with an alignment \
> based on that wish address is wrong. It always worked in practice since \
> `base_address` was always aligned to class_space_alignment (16MB). 
> Hence, the fix is simple: We just make the alignment requirement for the base \
> address explicit. When running with class space, we now assert that `base_address` \
> is aligned to  class space alignment (as well as CDS core region alignment, but \
> that is much smaller). Since `base_address` is calculated either from a hard-wired \
> default or from the `SharedBaseAddress` user input, and both are ensured to be \
> properly aligned, that assert should never fire. 
> Then, the offending calculation can be simplified by removing the base address from \
> it.

This pull request has now been integrated.

Changeset: 910d77be
Author:    Thomas Stuefe <stuefe@openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/910d77becd15580296687b00fed085ab106cb2eb
                
Stats:     31 lines in 1 file changed: 14 ins; 7 del; 10 mod

8331953: ubsan: metaspaceShared.cpp:1305:57: runtime error: applying non-zero offset \
12849152 to null pointer

Reviewed-by: iklam, mbaesken

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

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


[prev in list] [next in list] [prev in thread] [next in thread]