Skip to content

Commit

Permalink
8160404: RelocationHolder constructors have bugs
Browse files Browse the repository at this point in the history
Reviewed-by: kvn, jrose, jvernee
  • Loading branch information
Kim Barrett committed Dec 16, 2022
1 parent bf9a8ce commit bfa921a
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 109 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/cpu/x86/assembler_x86.cpp
Expand Up @@ -2495,7 +2495,7 @@ void Assembler::jmp_literal(address dest, RelocationHolder const& rspec) {
assert(dest != NULL, "must have a target");
intptr_t disp = dest - (pc() + sizeof(int32_t));
assert(is_simm32(disp), "must be 32bit offset (jmp)");
emit_data(disp, rspec.reloc(), call32_operand);
emit_data(disp, rspec, call32_operand);
}

void Assembler::jmpb_0(Label& L, const char* file, int line) {
Expand Down
41 changes: 39 additions & 2 deletions src/hotspot/share/code/relocInfo.cpp
Expand Up @@ -36,6 +36,9 @@
#include "utilities/align.hpp"
#include "utilities/copy.hpp"

#include <new>
#include <type_traits>

const RelocationHolder RelocationHolder::none; // its type is relocInfo::none


Expand Down Expand Up @@ -235,12 +238,44 @@ Relocation* RelocIterator::reloc() {
APPLY_TO_RELOCATIONS(EACH_TYPE);
#undef EACH_TYPE
assert(t == relocInfo::none, "must be padding");
return new(_rh) Relocation(t);
_rh = RelocationHolder::none;
return _rh.reloc();
}

// Verify all the destructors are trivial, so we don't need to worry about
// destroying old contents of a RelocationHolder being assigned or destroyed.
#define VERIFY_TRIVIALLY_DESTRUCTIBLE_AUX(Reloc) \
static_assert(std::is_trivially_destructible<Reloc>::value, "must be");

//////// Methods for flyweight Relocation types
#define VERIFY_TRIVIALLY_DESTRUCTIBLE(name) \
VERIFY_TRIVIALLY_DESTRUCTIBLE_AUX(PASTE_TOKENS(name, _Relocation));

APPLY_TO_RELOCATIONS(VERIFY_TRIVIALLY_DESTRUCTIBLE)
VERIFY_TRIVIALLY_DESTRUCTIBLE_AUX(Relocation)

#undef VERIFY_TRIVIALLY_DESTRUCTIBLE_AUX
#undef VERIFY_TRIVIALLY_DESTRUCTIBLE

// Define all the copy_into functions. These rely on all Relocation types
// being trivially destructible (verified above). So it doesn't matter
// whether the target holder has been previously initialized or not. There's
// no need to consider that distinction and destruct the relocation in an
// already initialized holder.
#define DEFINE_COPY_INTO_AUX(Reloc) \
void Reloc::copy_into(RelocationHolder& holder) const { \
copy_into_helper(*this, holder); \
}

#define DEFINE_COPY_INTO(name) \
DEFINE_COPY_INTO_AUX(PASTE_TOKENS(name, _Relocation))

APPLY_TO_RELOCATIONS(DEFINE_COPY_INTO)
DEFINE_COPY_INTO_AUX(Relocation)

#undef DEFINE_COPY_INTO_AUX
#undef DEFINE_COPY_INTO

//////// Methods for RelocationHolder

RelocationHolder RelocationHolder::plus(int offset) const {
if (offset != 0) {
Expand All @@ -264,6 +299,8 @@ RelocationHolder RelocationHolder::plus(int offset) const {
return (*this);
}

//////// Methods for flyweight Relocation types

// some relocations can compute their own values
address Relocation::value() {
ShouldNotReachHere();
Expand Down

1 comment on commit bfa921a

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.