Skip to content

Commit

Permalink
8333354: ubsan: frame.inline.hpp:91:25: and src/hotspot/share/runtime…
Browse files Browse the repository at this point in the history 
…/frame.inline.hpp:88:29: runtime error: member call on null pointer of type 'const struct SmallRegisterMap'

Co-authored-by: Kim Barrett <kbarrett@openjdk.org>
Reviewed-by: rrich, clanger
  • Loading branch information
@MBaesken
MBaesken and Kim Barrett committed Jul 30, 2024
1 parent 0325ab8 commit 8162832
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 81 deletions.
20 changes: 8 additions & 12 deletions src/hotspot/cpu/aarch64/smallRegisterMap_aarch64.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@

// Java frames don't have callee saved registers (except for rfp), so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}
private:
static void assert_is_rfp(VMReg r) NOT_DEBUG_RETURN
DEBUG_ONLY({ assert (r == rfp->as_VMReg() || r == rfp->as_VMReg()->next(), "Reg: %s", r->name()); })
Expand All @@ -48,17 +55,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
#ifdef ASSERT
for(int i = 0; i < RegisterMap::reg_count; i++) {
VMReg r = VMRegImpl::as_VMReg(i);
if (map->location(r, (intptr_t*)nullptr) != nullptr) assert_is_rfp(r);
}
#endif
}

inline address location(VMReg reg, intptr_t* sp) const {
assert_is_rfp(reg);
return (address)(sp - frame::sender_sp_offset);
Expand Down
15 changes: 8 additions & 7 deletions src/hotspot/cpu/arm/smallRegisterMap_arm.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@

// Java frames don't have callee saved registers (except for rfp), so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}
private:
static void assert_is_rfp(VMReg r) NOT_DEBUG_RETURN
DEBUG_ONLY({ Unimplemented(); })
Expand All @@ -46,12 +53,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
Unimplemented();
}

inline address location(VMReg reg, intptr_t* sp) const {
Unimplemented();
return nullptr;
Expand Down
24 changes: 9 additions & 15 deletions src/hotspot/cpu/ppc/smallRegisterMap_ppc.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,16 @@

// Java frames don't have callee saved registers, so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
public:
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}

// as_RegisterMap is used when we didn't want to templatize and abstract over RegisterMap type to support SmallRegisterMap
// Consider enhancing SmallRegisterMap to support those cases
const RegisterMap* as_RegisterMap() const { return nullptr; }
Expand All @@ -44,19 +51,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
#ifdef ASSERT
for(int i = 0; i < RegisterMap::reg_count; i++) {
VMReg r = VMRegImpl::as_VMReg(i);
if (map->location(r, (intptr_t*)nullptr) != nullptr) {
assert(false, "Reg: %s", r->name()); // Should not reach here
}
}
#endif
}

inline address location(VMReg reg, intptr_t* sp) const {
assert(false, "Reg: %s", reg->name());
return nullptr;
Expand Down
20 changes: 8 additions & 12 deletions src/hotspot/cpu/riscv/smallRegisterMap_riscv.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@

// Java frames don't have callee saved registers (except for fp), so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}
private:
static void assert_is_fp(VMReg r) NOT_DEBUG_RETURN
DEBUG_ONLY({ assert (r == fp->as_VMReg() || r == fp->as_VMReg()->next(), "Reg: %s", r->name()); })
Expand All @@ -48,17 +55,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
#ifdef ASSERT
for(int i = 0; i < RegisterMap::reg_count; i++) {
VMReg r = VMRegImpl::as_VMReg(i);
if (map->location(r, (intptr_t*)nullptr) != nullptr) assert_is_fp(r);
}
#endif
}

inline address location(VMReg reg, intptr_t* sp) const {
assert_is_fp(reg);
return (address)(sp - 2);
Expand Down
15 changes: 8 additions & 7 deletions src/hotspot/cpu/s390/smallRegisterMap_s390.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@

// Java frames don't have callee saved registers (except for rfp), so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}
private:
static void assert_is_rfp(VMReg r) NOT_DEBUG_RETURN
DEBUG_ONLY({ Unimplemented(); })
Expand All @@ -46,12 +53,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
Unimplemented();
}

inline address location(VMReg reg, intptr_t* sp) const {
Unimplemented();
return nullptr;
Expand Down
21 changes: 9 additions & 12 deletions src/hotspot/cpu/x86/smallRegisterMap_x86.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,16 @@

// Java frames don't have callee saved registers (except for rbp), so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}

private:
static void assert_is_rbp(VMReg r) NOT_DEBUG_RETURN
DEBUG_ONLY({ assert(r == rbp->as_VMReg() || r == rbp->as_VMReg()->next(), "Reg: %s", r->name()); })
Expand All @@ -48,17 +56,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
#ifdef ASSERT
for(int i = 0; i < RegisterMap::reg_count; i++) {
VMReg r = VMRegImpl::as_VMReg(i);
if (map->location(r, (intptr_t*)nullptr) != nullptr) assert_is_rbp(r);
}
#endif
}

inline address location(VMReg reg, intptr_t* sp) const {
assert_is_rbp(reg);
return (address)(sp - frame::sender_sp_offset);
Expand Down
15 changes: 8 additions & 7 deletions src/hotspot/cpu/zero/smallRegisterMap_zero.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@

// Java frames don't have callee saved registers (except for rfp), so we can use a smaller RegisterMap
class SmallRegisterMap {
constexpr SmallRegisterMap() = default;
~SmallRegisterMap() = default;
NONCOPYABLE(SmallRegisterMap);

public:
static constexpr SmallRegisterMap* instance = nullptr;
static const SmallRegisterMap* instance() {
static constexpr SmallRegisterMap the_instance{};
return &the_instance;
}
private:
static void assert_is_rfp(VMReg r) NOT_DEBUG_RETURN
DEBUG_ONLY({ Unimplemented(); })
Expand All @@ -46,12 +53,6 @@ class SmallRegisterMap {
return map;
}

SmallRegisterMap() {}

SmallRegisterMap(const RegisterMap* map) {
Unimplemented();
}

inline address location(VMReg reg, intptr_t* sp) const {
Unimplemented();
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/stackChunkOop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ static int num_java_frames(const StackChunkFrameStream<ChunkFrames::Mixed>& f) {
int stackChunkOopDesc::num_java_frames() const {
int n = 0;
for (StackChunkFrameStream<ChunkFrames::Mixed> f(const_cast<stackChunkOopDesc*>(this)); !f.is_done();
f.next(SmallRegisterMap::instance)) {
f.next(SmallRegisterMap::instance())) {
if (!f.is_stub()) {
n += ::num_java_frames(f);
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/stackChunkOop.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ inline void stackChunkOopDesc::iterate_stack(StackChunkFrameClosureType* closure

template <ChunkFrames frame_kind, class StackChunkFrameClosureType>
inline void stackChunkOopDesc::iterate_stack(StackChunkFrameClosureType* closure) {
const SmallRegisterMap* map = SmallRegisterMap::instance;
const SmallRegisterMap* map = SmallRegisterMap::instance();
assert(!map->in_cont(), "");

StackChunkFrameStream<frame_kind> f(this);
Expand Down
14 changes: 7 additions & 7 deletions src/hotspot/share/runtime/continuationFreezeThaw.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@ int ThawBase::remove_top_compiled_frame_from_chunk(stackChunkOop chunk, int &arg
const int frame_size = f.cb()->frame_size();
argsize = f.stack_argsize();

f.next(SmallRegisterMap::instance, true /* stop */);
f.next(SmallRegisterMap::instance(), true /* stop */);
empty = f.is_done();
assert(!empty || argsize == chunk->argsize(), "");

Expand Down Expand Up @@ -2075,7 +2075,7 @@ bool ThawBase::recurse_thaw_java_frame(frame& caller, int num_frames) {

int argsize = _stream.stack_argsize();

_stream.next(SmallRegisterMap::instance);
_stream.next(SmallRegisterMap::instance());
assert(_stream.to_frame().is_empty() == _stream.is_done(), "");

// we never leave a compiled caller of an interpreted frame as the top frame in the chunk
Expand Down Expand Up @@ -2183,7 +2183,7 @@ NOINLINE void ThawBase::recurse_thaw_interpreted_frame(const frame& hf, frame& c
assert(hf.is_interpreted_frame(), "");

if (UNLIKELY(seen_by_gc())) {
_cont.tail()->do_barriers<stackChunkOopDesc::BarrierType::Store>(_stream, SmallRegisterMap::instance);
_cont.tail()->do_barriers<stackChunkOopDesc::BarrierType::Store>(_stream, SmallRegisterMap::instance());
}

const bool is_bottom_frame = recurse_thaw_java_frame<ContinuationHelper::InterpretedFrame>(caller, num_frames);
Expand Down Expand Up @@ -2226,7 +2226,7 @@ NOINLINE void ThawBase::recurse_thaw_interpreted_frame(const frame& hf, frame& c

if (!is_bottom_frame) {
// can only fix caller once this frame is thawed (due to callee saved regs)
_cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance);
_cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance());
} else if (_cont.tail()->has_bitmap() && locals > 0) {
assert(hf.is_heap_frame(), "should be");
address start = (address)(heap_frame_bottom - locals);
Expand All @@ -2243,7 +2243,7 @@ void ThawBase::recurse_thaw_compiled_frame(const frame& hf, frame& caller, int n
assert(_cont.is_preempted() || !stub_caller, "stub caller not at preemption");

if (!stub_caller && UNLIKELY(seen_by_gc())) { // recurse_thaw_stub_frame already invoked our barriers with a full regmap
_cont.tail()->do_barriers<stackChunkOopDesc::BarrierType::Store>(_stream, SmallRegisterMap::instance);
_cont.tail()->do_barriers<stackChunkOopDesc::BarrierType::Store>(_stream, SmallRegisterMap::instance());
}

const bool is_bottom_frame = recurse_thaw_java_frame<ContinuationHelper::CompiledFrame>(caller, num_frames);
Expand Down Expand Up @@ -2302,7 +2302,7 @@ void ThawBase::recurse_thaw_compiled_frame(const frame& hf, frame& caller, int n

if (!is_bottom_frame) {
// can only fix caller once this frame is thawed (due to callee saved regs); this happens on the stack
_cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance);
_cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance());
} else if (_cont.tail()->has_bitmap() && added_argsize > 0) {
address start = (address)(heap_frame_top + ContinuationHelper::CompiledFrame::size(hf) + frame::metadata_words_at_top);
int stack_args_slots = f.cb()->as_nmethod()->num_stack_arg_slots(false /* rounded */);
Expand Down Expand Up @@ -2384,7 +2384,7 @@ void ThawBase::finish_thaw(frame& f) {
f.set_sp(align_down(f.sp(), frame::frame_alignment));
}
push_return_frame(f);
chunk->fix_thawed_frame(f, SmallRegisterMap::instance); // can only fix caller after push_return_frame (due to callee saved regs)
chunk->fix_thawed_frame(f, SmallRegisterMap::instance()); // can only fix caller after push_return_frame (due to callee saved regs)

assert(_cont.is_empty() == _cont.last_frame().is_empty(), "");

Expand Down

5 comments on commit 8162832

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@MBaesken
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk23u

@openjdk
Copy link

@openjdk openjdk bot commented on 8162832 Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBaesken the backport was successfully created on the branch backport-MBaesken-81628328-master in my personal fork of openjdk/jdk23u. To create a pull request with this backport targeting openjdk/jdk23u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 81628328 from the openjdk/jdk repository.

The commit being backported was authored by Matthias Baesken on 30 Jul 2024 and was reviewed by Richard Reingruber and Christoph Langer.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk23u:

$ git fetch https://github.com/openjdk-bots/jdk23u.git backport-MBaesken-81628328-master:backport-MBaesken-81628328-master
$ git checkout backport-MBaesken-81628328-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk23u.git backport-MBaesken-81628328-master

@MBaesken
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 8162832 Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBaesken the backport was successfully created on the branch backport-MBaesken-81628328-master in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 81628328 from the openjdk/jdk repository.

The commit being backported was authored by Matthias Baesken on 30 Jul 2024 and was reviewed by Richard Reingruber and Christoph Langer.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u-dev:

$ git fetch https://github.com/openjdk-bots/jdk21u-dev.git backport-MBaesken-81628328-master:backport-MBaesken-81628328-master
$ git checkout backport-MBaesken-81628328-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u-dev.git backport-MBaesken-81628328-master

Please sign in to comment.