8160404: RelocationHolder constructors have bugs

Reviewed-by: kvn, jrose, jvernee
This commit is contained in:
Kim Barrett 2022-12-16 20:47:40 +00:00
parent bf9a8ce0bb
commit bfa921ae6c
3 changed files with 206 additions and 110 deletions

View File

@ -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) {

View File

@ -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
@ -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) {
@ -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();

View File

@ -28,8 +28,10 @@
#include "memory/allocation.hpp"
#include "oops/oopsHierarchy.hpp"
#include "runtime/osInfo.hpp"
#include "utilities/macros.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/macros.hpp"
#include <new>
class nmethod;
class CodeBlob;
@ -471,24 +473,70 @@ inline relocInfo prefix_relocInfo(int datalen = 0) {
// the holder is "one size fits all".
class RelocationHolder {
friend class Relocation;
friend class CodeSection;
private:
// this preallocated memory must accommodate all subclasses of Relocation
// (this number is assertion-checked in Relocation::operator new)
enum { _relocbuf_size = 5 };
void* _relocbuf[ _relocbuf_size ];
// A Relocation is "held" by placement constructing a Relocation into
// _relocbuf. Hence, _relocbuf must accomodate all subclasses of
// Relocation. We also need the Relocation base class to be at the same
// address as the start of the object, e.g. at the address of _relocbuf.
// Both of these requirements are checked (see emplace_relocation).
// The placement of the base class subobject isn't guaranteed by C++, since
// these aren't standard layout classes, but all supported implementations
// provide that behavior. If that changes, we can instead add a Relocation*
// _reloc member to capture the result of the placement new, and use that to
// access the base subobject.
static const size_t _relocbuf_size = 5 * sizeof(void*);
alignas(void*) char _relocbuf[_relocbuf_size];
template<typename Reloc, typename... Args>
void emplace_relocation(const Args&... args) {
static_assert(std::is_base_of<Relocation, Reloc>::value, "not Relocation");
static_assert(sizeof(Reloc) <= sizeof(_relocbuf), "_relocbuf too small");
Relocation* reloc = ::new (_relocbuf) Reloc(args...);
// Verify the base class subobject of the object constructed into
// _relocbuf is at the same address as the derived object.
assert(static_cast<const void*>(reloc) == _relocbuf, "invariant");
}
// Support for Relocation::copy_into.
// reloc should be a most derived object.
template<typename Reloc>
void copy_into_impl(const Reloc& reloc) {
emplace_relocation<Reloc>(reloc);
}
// Tag for selecting the constructor below and carrying the type of the
// relocation object the new holder will (initially) contain.
template<typename Reloc> struct Construct {};
// Constructor used by construct(). Constructs a new holder containing a
// relocation of type Reloc that is constructed using the provided args.
template<typename Reloc, typename... Args>
RelocationHolder(Construct<Reloc>, const Args&... args) {
emplace_relocation<Reloc>(args...);
}
public:
Relocation* reloc() const { return (Relocation*) &_relocbuf[0]; }
Relocation* reloc() const { return (Relocation*)_relocbuf; }
inline relocInfo::relocType type() const;
// Add a constant offset to a relocation. Helper for class Address.
RelocationHolder plus(int offset) const;
inline RelocationHolder(); // initializes type to none
// Return a holder containing a relocation of type Reloc, constructed using args.
template<typename Reloc, typename... Args>
static RelocationHolder construct(const Args&... args) {
return RelocationHolder(Construct<Reloc>(), args...);
}
inline RelocationHolder(Relocation* r); // make a copy
RelocationHolder(); // Initializes type to none.
// Depends on the destructor for all relocation types being trivial
// (verified in .cpp file).
~RelocationHolder() = default;
RelocationHolder(const RelocationHolder& from);
RelocationHolder& operator=(const RelocationHolder& from);
static const RelocationHolder none;
};
@ -636,7 +684,6 @@ class RelocIterator : public StackObj {
// So, the RelocIterator unpacks relocInfos into Relocations.
class Relocation {
friend class RelocationHolder;
friend class RelocIterator;
private:
@ -658,19 +705,18 @@ class Relocation {
assert(_binding != NULL, "must now be bound");
}
Relocation(relocInfo::relocType rtype) : _binding(NULL), _rtype(rtype) { }
explicit Relocation(relocInfo::relocType rtype) : _binding(NULL), _rtype(rtype) { }
static RelocationHolder newHolder() {
return RelocationHolder();
// Helper for copy_into functions for derived classes.
// Forwards operation to RelocationHolder::copy_into_impl so that
// RelocationHolder only needs to befriend this class, rather than all
// derived classes that implement copy_into.
template<typename Reloc>
static void copy_into_helper(const Reloc& reloc, RelocationHolder& holder) {
holder.copy_into_impl(reloc);
}
public:
void* operator new(size_t size, const RelocationHolder& holder) throw() {
assert(size <= sizeof(holder._relocbuf), "Make _relocbuf bigger!");
assert((void* const *)holder.reloc() == &holder._relocbuf[0], "ptrs must agree");
return holder.reloc();
}
// make a generic relocation for a given type (if possible)
static RelocationHolder spec_simple(relocInfo::relocType rtype);
@ -793,8 +839,20 @@ class Relocation {
int format() const { return binding()->format(); }
public:
// Make a filler relocation.
Relocation() : Relocation(relocInfo::none) {}
// Intentionally public non-virtual destructor, even though polymorphic. We
// never heap allocate a Relocation, so never delete through a base pointer.
// RelocationHolder depends on the destructor for all relocation types being
// trivial, so this must not be virtual (and hence non-trivial).
~Relocation() = default;
relocInfo::relocType type() const { return _rtype; }
// Copy this relocation into holder.
virtual void copy_into(RelocationHolder& holder) const;
// is it a call instruction?
virtual bool is_call() { return false; }
@ -818,17 +876,20 @@ class Relocation {
// certain inlines must be deferred until class Relocation is defined:
inline RelocationHolder::RelocationHolder() {
// initialize the vtbl, just to keep things type-safe
new(*this) Relocation(relocInfo::none);
inline RelocationHolder::RelocationHolder() :
RelocationHolder(Construct<Relocation>())
{}
inline RelocationHolder::RelocationHolder(const RelocationHolder& from) {
from.reloc()->copy_into(*this);
}
inline RelocationHolder::RelocationHolder(Relocation* r) {
// wordwise copy from r (ok if it copies garbage after r)
for (int i = 0; i < _relocbuf_size; i++) {
_relocbuf[i] = ((void**)r)[i];
}
inline RelocationHolder& RelocationHolder::operator=(const RelocationHolder& from) {
// All Relocation types are trivially destructible (verified in .cpp file),
// so we don't need to destruct our old value before copying over it.
// If not for that we would need to decide what to do about self-assignment.
from.reloc()->copy_into(*this);
return *this;
}
relocInfo::relocType RelocationHolder::type() const {
@ -876,29 +937,29 @@ class DataRelocation : public Relocation {
};
class post_call_nop_Relocation : public Relocation {
friend class RelocIterator;
friend class RelocationHolder;
public:
post_call_nop_Relocation() : Relocation(relocInfo::post_call_nop_type) { }
static RelocationHolder spec() {
RelocationHolder rh = newHolder();
new(rh) post_call_nop_Relocation();
return rh;
return RelocationHolder::construct<post_call_nop_Relocation>();
}
void copy_into(RelocationHolder& holder) const;
};
class entry_guard_Relocation : public Relocation {
friend class RelocIterator;
friend class RelocationHolder;
public:
entry_guard_Relocation() : Relocation(relocInfo::entry_guard_type) { }
static RelocationHolder spec() {
RelocationHolder rh = newHolder();
new(rh) entry_guard_Relocation();
return rh;
return RelocationHolder::construct<entry_guard_Relocation>();
}
void copy_into(RelocationHolder& holder) const;
};
// A CallRelocation always points at a call instruction.
@ -923,9 +984,7 @@ class oop_Relocation : public DataRelocation {
// an oop in the CodeBlob's oop pool
static RelocationHolder spec(int oop_index, int offset = 0) {
assert(oop_index > 0, "must be a pool-resident oop");
RelocationHolder rh = newHolder();
new(rh) oop_Relocation(oop_index, offset);
return rh;
return RelocationHolder::construct<oop_Relocation>(oop_index, offset);
}
// an oop in the instruction stream
static RelocationHolder spec_for_immediate() {
@ -935,11 +994,11 @@ class oop_Relocation : public DataRelocation {
"Must return true so we will search for oops as roots etc. in the code.");
const int oop_index = 0;
const int offset = 0; // if you want an offset, use the oop pool
RelocationHolder rh = newHolder();
new(rh) oop_Relocation(oop_index, offset);
return rh;
return RelocationHolder::construct<oop_Relocation>(oop_index, offset);
}
void copy_into(RelocationHolder& holder) const;
private:
jint _oop_index; // if > 0, index into CodeBlob::oop_at
jint _offset; // byte offset to apply to the oop itself
@ -947,7 +1006,7 @@ class oop_Relocation : public DataRelocation {
oop_Relocation(int oop_index, int offset)
: DataRelocation(relocInfo::oop_type), _oop_index(oop_index), _offset(offset) { }
friend class RelocIterator;
friend class RelocationHolder;
oop_Relocation() : DataRelocation(relocInfo::oop_type) {}
public:
@ -980,19 +1039,17 @@ class metadata_Relocation : public DataRelocation {
// an metadata in the CodeBlob's metadata pool
static RelocationHolder spec(int metadata_index, int offset = 0) {
assert(metadata_index > 0, "must be a pool-resident metadata");
RelocationHolder rh = newHolder();
new(rh) metadata_Relocation(metadata_index, offset);
return rh;
return RelocationHolder::construct<metadata_Relocation>(metadata_index, offset);
}
// an metadata in the instruction stream
static RelocationHolder spec_for_immediate() {
const int metadata_index = 0;
const int offset = 0; // if you want an offset, use the metadata pool
RelocationHolder rh = newHolder();
new(rh) metadata_Relocation(metadata_index, offset);
return rh;
return RelocationHolder::construct<metadata_Relocation>(metadata_index, offset);
}
void copy_into(RelocationHolder& holder) const;
private:
jint _metadata_index; // if > 0, index into nmethod::metadata_at
jint _offset; // byte offset to apply to the metadata itself
@ -1000,7 +1057,7 @@ class metadata_Relocation : public DataRelocation {
metadata_Relocation(int metadata_index, int offset)
: DataRelocation(relocInfo::metadata_type), _metadata_index(metadata_index), _offset(offset) { }
friend class RelocIterator;
friend class RelocationHolder;
metadata_Relocation() : DataRelocation(relocInfo::metadata_type) { }
// Fixes a Metadata pointer in the code. Most platforms embeds the
@ -1035,11 +1092,11 @@ class virtual_call_Relocation : public CallRelocation {
// The oop_limit helps find the last associated set-oop.
// (See comments at the top of this file.)
static RelocationHolder spec(address cached_value, jint method_index = 0) {
RelocationHolder rh = newHolder();
new(rh) virtual_call_Relocation(cached_value, method_index);
return rh;
return RelocationHolder::construct<virtual_call_Relocation>(cached_value, method_index);
}
void copy_into(RelocationHolder& holder) const;
private:
address _cached_value; // location of set-value instruction
jint _method_index; // resolved method for a Java call
@ -1051,7 +1108,7 @@ class virtual_call_Relocation : public CallRelocation {
assert(cached_value != NULL, "first oop address must be specified");
}
friend class RelocIterator;
friend class RelocationHolder;
virtual_call_Relocation() : CallRelocation(relocInfo::virtual_call_type) { }
public:
@ -1074,11 +1131,11 @@ class virtual_call_Relocation : public CallRelocation {
class opt_virtual_call_Relocation : public CallRelocation {
public:
static RelocationHolder spec(int method_index = 0) {
RelocationHolder rh = newHolder();
new(rh) opt_virtual_call_Relocation(method_index);
return rh;
return RelocationHolder::construct<opt_virtual_call_Relocation>(method_index);
}
void copy_into(RelocationHolder& holder) const;
private:
jint _method_index; // resolved method for a Java call
@ -1086,7 +1143,7 @@ class opt_virtual_call_Relocation : public CallRelocation {
: CallRelocation(relocInfo::opt_virtual_call_type),
_method_index(method_index) { }
friend class RelocIterator;
friend class RelocationHolder;
opt_virtual_call_Relocation() : CallRelocation(relocInfo::opt_virtual_call_type) {}
public:
@ -1106,11 +1163,11 @@ class opt_virtual_call_Relocation : public CallRelocation {
class static_call_Relocation : public CallRelocation {
public:
static RelocationHolder spec(int method_index = 0) {
RelocationHolder rh = newHolder();
new(rh) static_call_Relocation(method_index);
return rh;
return RelocationHolder::construct<static_call_Relocation>(method_index);
}
void copy_into(RelocationHolder& holder) const;
private:
jint _method_index; // resolved method for a Java call
@ -1118,7 +1175,7 @@ class static_call_Relocation : public CallRelocation {
: CallRelocation(relocInfo::static_call_type),
_method_index(method_index) { }
friend class RelocIterator;
friend class RelocationHolder;
static_call_Relocation() : CallRelocation(relocInfo::static_call_type) {}
public:
@ -1137,11 +1194,11 @@ class static_call_Relocation : public CallRelocation {
class static_stub_Relocation : public Relocation {
public:
static RelocationHolder spec(address static_call) {
RelocationHolder rh = newHolder();
new(rh) static_stub_Relocation(static_call);
return rh;
return RelocationHolder::construct<static_stub_Relocation>(static_call);
}
void copy_into(RelocationHolder& holder) const;
private:
address _static_call; // location of corresponding static_call
@ -1149,7 +1206,7 @@ class static_stub_Relocation : public Relocation {
: Relocation(relocInfo::static_stub_type),
_static_call(static_call) { }
friend class RelocIterator;
friend class RelocationHolder;
static_stub_Relocation() : Relocation(relocInfo::static_stub_type) { }
public:
@ -1166,29 +1223,27 @@ class runtime_call_Relocation : public CallRelocation {
public:
static RelocationHolder spec() {
RelocationHolder rh = newHolder();
new(rh) runtime_call_Relocation();
return rh;
return RelocationHolder::construct<runtime_call_Relocation>();
}
private:
friend class RelocIterator;
runtime_call_Relocation() : CallRelocation(relocInfo::runtime_call_type) { }
void copy_into(RelocationHolder& holder) const;
public:
private:
friend class RelocationHolder;
runtime_call_Relocation() : CallRelocation(relocInfo::runtime_call_type) { }
};
class runtime_call_w_cp_Relocation : public CallRelocation {
public:
static RelocationHolder spec() {
RelocationHolder rh = newHolder();
new(rh) runtime_call_w_cp_Relocation();
return rh;
return RelocationHolder::construct<runtime_call_w_cp_Relocation>();
}
void copy_into(RelocationHolder& holder) const;
private:
friend class RelocIterator;
friend class RelocationHolder;
runtime_call_w_cp_Relocation()
: CallRelocation(relocInfo::runtime_call_w_cp_type),
_offset(-4) /* <0 = invalid */ { }
@ -1218,10 +1273,11 @@ class runtime_call_w_cp_Relocation : public CallRelocation {
class trampoline_stub_Relocation : public Relocation {
public:
static RelocationHolder spec(address static_call) {
RelocationHolder rh = newHolder();
return (new (rh) trampoline_stub_Relocation(static_call));
return RelocationHolder::construct<trampoline_stub_Relocation>(static_call);
}
void copy_into(RelocationHolder& holder) const;
private:
address _owner; // Address of the NativeCall that owns the trampoline.
@ -1229,7 +1285,7 @@ class trampoline_stub_Relocation : public Relocation {
: Relocation(relocInfo::trampoline_stub_type),
_owner(owner) { }
friend class RelocIterator;
friend class RelocationHolder;
trampoline_stub_Relocation() : Relocation(relocInfo::trampoline_stub_type) { }
public:
@ -1248,19 +1304,17 @@ class external_word_Relocation : public DataRelocation {
public:
static RelocationHolder spec(address target) {
assert(target != NULL, "must not be null");
RelocationHolder rh = newHolder();
new(rh) external_word_Relocation(target);
return rh;
return RelocationHolder::construct<external_word_Relocation>(target);
}
// Use this one where all 32/64 bits of the target live in the code stream.
// The target must be an intptr_t, and must be absolute (not relative).
static RelocationHolder spec_for_immediate() {
RelocationHolder rh = newHolder();
new(rh) external_word_Relocation(NULL);
return rh;
return RelocationHolder::construct<external_word_Relocation>(nullptr);
}
void copy_into(RelocationHolder& holder) const;
// Some address looking values aren't safe to treat as relocations
// and should just be treated as constants.
static bool can_be_relocated(address target) {
@ -1274,7 +1328,7 @@ class external_word_Relocation : public DataRelocation {
external_word_Relocation(address target)
: DataRelocation(relocInfo::external_word_type), _target(target) { }
friend class RelocIterator;
friend class RelocationHolder;
external_word_Relocation() : DataRelocation(relocInfo::external_word_type) { }
public:
@ -1296,18 +1350,16 @@ class internal_word_Relocation : public DataRelocation {
public:
static RelocationHolder spec(address target) {
assert(target != NULL, "must not be null");
RelocationHolder rh = newHolder();
new(rh) internal_word_Relocation(target);
return rh;
return RelocationHolder::construct<internal_word_Relocation>(target);
}
// use this one where all the bits of the target can fit in the code stream:
static RelocationHolder spec_for_immediate() {
RelocationHolder rh = newHolder();
new(rh) internal_word_Relocation(NULL);
return rh;
return RelocationHolder::construct<internal_word_Relocation>(nullptr);
}
void copy_into(RelocationHolder& holder) const;
// default section -1 means self-relative
internal_word_Relocation(address target, int section = -1,
relocInfo::relocType type = relocInfo::internal_word_type)
@ -1317,7 +1369,7 @@ class internal_word_Relocation : public DataRelocation {
address _target; // address in CodeBlob
int _section; // section providing base address, if any
friend class RelocIterator;
friend class RelocationHolder;
internal_word_Relocation(relocInfo::relocType type = relocInfo::internal_word_type)
: DataRelocation(type) { }
@ -1341,11 +1393,11 @@ class internal_word_Relocation : public DataRelocation {
class section_word_Relocation : public internal_word_Relocation {
public:
static RelocationHolder spec(address target, int section) {
RelocationHolder rh = newHolder();
new(rh) section_word_Relocation(target, section);
return rh;
return RelocationHolder::construct<section_word_Relocation>(target, section);
}
void copy_into(RelocationHolder& holder) const;
section_word_Relocation(address target, int section)
: internal_word_Relocation(target, section, relocInfo::section_word_type) {
assert(target != NULL, "must not be null");
@ -1356,7 +1408,7 @@ class section_word_Relocation : public internal_word_Relocation {
void unpack_data();
private:
friend class RelocIterator;
friend class RelocationHolder;
section_word_Relocation() : internal_word_Relocation(relocInfo::section_word_type) { }
};
@ -1366,25 +1418,32 @@ class poll_Relocation : public Relocation {
void fix_relocation_after_move(const CodeBuffer* src, CodeBuffer* dest);
public:
poll_Relocation(relocInfo::relocType type = relocInfo::poll_type) : Relocation(type) { }
void copy_into(RelocationHolder& holder) const;
};
class poll_return_Relocation : public poll_Relocation {
public:
poll_return_Relocation() : poll_Relocation(relocInfo::relocInfo::poll_return_type) { }
void copy_into(RelocationHolder& holder) const;
};
// We know all the xxx_Relocation classes, so now we can define these:
#define EACH_CASE(name) \
inline name##_Relocation* RelocIterator::name##_reloc() { \
assert(type() == relocInfo::name##_type, "type must agree"); \
/* The purpose of the placed "new" is to re-use the same */ \
/* stack storage for each new iteration. */ \
name##_Relocation* r = new(_rh) name##_Relocation(); \
r->set_binding(this); \
r->name##_Relocation::unpack_data(); \
return r; \
#define EACH_CASE_AUX(Accessor, Reloc) \
inline Reloc* RelocIterator::Accessor() { \
static const RelocationHolder proto = RelocationHolder::construct<Reloc>(); \
assert(type() == proto.type(), "type must agree"); \
_rh = proto; \
Reloc* r = static_cast<Reloc*>(_rh.reloc()); \
r->set_binding(this); \
r->Reloc::unpack_data(); \
return r; \
}
#define EACH_CASE(name) \
EACH_CASE_AUX(PASTE_TOKENS(name, _reloc), PASTE_TOKENS(name, _Relocation))
APPLY_TO_RELOCATIONS(EACH_CASE);
#undef EACH_CASE_AUX
#undef EACH_CASE
inline RelocIterator::RelocIterator(CompiledMethod* nm, address begin, address limit) {