Skip to content

Commit fb66716

Browse files
committed
8331725: ubsan: pc may not always be the entry point for a VtableStub
Reviewed-by: kvn, mbaesken
1 parent fb9a227 commit fb66716

File tree

2 files changed

+33
-14
lines changed

2 files changed

+33
-14
lines changed

src/hotspot/share/code/vtableStubs.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,19 @@ inline uint VtableStubs::hash(bool is_vtable_stub, int vtable_index){
255255
}
256256

257257

258+
inline uint VtableStubs::unsafe_hash(address entry_point) {
259+
// The entrypoint may or may not be a VtableStub. Generate a hash as if it was.
260+
address vtable_stub_addr = entry_point - VtableStub::entry_offset();
261+
assert(CodeCache::contains(vtable_stub_addr), "assumed to always be the case");
262+
address vtable_type_addr = vtable_stub_addr + offset_of(VtableStub, _type);
263+
address vtable_index_addr = vtable_stub_addr + offset_of(VtableStub, _index);
264+
bool is_vtable_stub = *vtable_type_addr == static_cast<uint8_t>(VtableStub::Type::vtable_stub);
265+
int vtable_index;
266+
memcpy(&vtable_index, vtable_index_addr, sizeof(vtable_index));
267+
return hash(is_vtable_stub, vtable_index);
268+
}
269+
270+
258271
VtableStub* VtableStubs::lookup(bool is_vtable_stub, int vtable_index) {
259272
assert_lock_strong(VtableStubs_lock);
260273
unsigned hash = VtableStubs::hash(is_vtable_stub, vtable_index);
@@ -275,12 +288,15 @@ void VtableStubs::enter(bool is_vtable_stub, int vtable_index, VtableStub* s) {
275288
}
276289

277290
VtableStub* VtableStubs::entry_point(address pc) {
291+
// The pc may or may not be the entry point for a VtableStub. Use unsafe_hash
292+
// to generate the hash that would have been used if it was. The lookup in the
293+
// _table will only succeed if there is a VtableStub with an entry point at
294+
// the pc.
278295
MutexLocker ml(VtableStubs_lock, Mutex::_no_safepoint_check_flag);
279-
VtableStub* stub = (VtableStub*)(pc - VtableStub::entry_offset());
280-
uint hash = VtableStubs::hash(stub->is_vtable_stub(), stub->index());
296+
uint hash = VtableStubs::unsafe_hash(pc);
281297
VtableStub* s;
282-
for (s = Atomic::load(&_table[hash]); s != nullptr && s != stub; s = s->next()) {}
283-
return (s == stub) ? s : nullptr;
298+
for (s = Atomic::load(&_table[hash]); s != nullptr && s->entry_point() != pc; s = s->next()) {}
299+
return (s != nullptr && s->entry_point() == pc) ? s : nullptr;
284300
}
285301

286302
bool VtableStubs::contains(address pc) {

src/hotspot/share/code/vtableStubs.hpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "asm/macroAssembler.hpp"
2929
#include "code/vmreg.hpp"
3030
#include "memory/allStatic.hpp"
31-
#include "sanitizers/ub.hpp"
3231
#include "utilities/checkedCast.hpp"
3332

3433
// A VtableStub holds an individual code stub for a pair (vtable index, #args) for either itables or vtables
@@ -94,6 +93,7 @@ class VtableStubs : AllStatic {
9493
static VtableStub* lookup (bool is_vtable_stub, int vtable_index);
9594
static void enter (bool is_vtable_stub, int vtable_index, VtableStub* s);
9695
static inline uint hash (bool is_vtable_stub, int vtable_index);
96+
static inline uint unsafe_hash (address entry_point);
9797
static address find_stub (bool is_vtable_stub, int vtable_index);
9898
static void bookkeeping(MacroAssembler* masm, outputStream* out, VtableStub* s,
9999
address npe_addr, address ame_addr, bool is_vtable_stub,
@@ -119,6 +119,12 @@ class VtableStub {
119119
private:
120120
friend class VtableStubs;
121121

122+
enum class Type : uint8_t {
123+
itable_stub,
124+
vtable_stub,
125+
};
126+
127+
122128
static address _chunk; // For allocation
123129
static address _chunk_end; // For allocation
124130
static VMReg _receiver_location; // Where to find receiver
@@ -127,27 +133,27 @@ class VtableStub {
127133
const short _index; // vtable index
128134
short _ame_offset; // Where an AbstractMethodError might occur
129135
short _npe_offset; // Where a NullPointerException might occur
130-
bool _is_vtable_stub; // True if vtable stub, false, is itable stub
136+
Type _type; // Type, either vtable stub or itable stub
131137
/* code follows here */ // The vtableStub code
132138

133139
void* operator new(size_t size, int code_size) throw();
134140

135141
VtableStub(bool is_vtable_stub, short index)
136142
: _next(nullptr), _index(index), _ame_offset(-1), _npe_offset(-1),
137-
_is_vtable_stub(is_vtable_stub) {}
143+
_type(is_vtable_stub ? Type::vtable_stub : Type::itable_stub) {}
138144
VtableStub* next() const { return _next; }
139145
int index() const { return _index; }
140146
static VMReg receiver_location() { return _receiver_location; }
141147
void set_next(VtableStub* n) { _next = n; }
142148

143149
public:
144150
address code_begin() const { return (address)(this + 1); }
145-
address code_end() const { return code_begin() + VtableStubs::code_size_limit(_is_vtable_stub); }
151+
address code_end() const { return code_begin() + VtableStubs::code_size_limit(is_vtable_stub()); }
146152
address entry_point() const { return code_begin(); }
147153
static int entry_offset() { return sizeof(class VtableStub); }
148154

149155
bool matches(bool is_vtable_stub, int index) const {
150-
return _index == index && _is_vtable_stub == is_vtable_stub;
156+
return _index == index && this->is_vtable_stub() == is_vtable_stub;
151157
}
152158
bool contains(address pc) const { return code_begin() <= pc && pc < code_end(); }
153159

@@ -173,11 +179,8 @@ class VtableStub {
173179

174180
public:
175181
// Query
176-
bool is_itable_stub() { return !_is_vtable_stub; }
177-
// We reinterpret arbitrary memory as VtableStub. This does not cause failures because the lookup/equality
178-
// check will reject false objects. Disabling UBSan is a temporary workaround until JDK-8331725 is fixed.
179-
ATTRIBUTE_NO_UBSAN
180-
bool is_vtable_stub() { return _is_vtable_stub; }
182+
bool is_itable_stub() const { return _type == Type::itable_stub; }
183+
bool is_vtable_stub() const { return _type == Type::vtable_stub; }
181184
bool is_abstract_method_error(address epc) { return epc == code_begin()+_ame_offset; }
182185
bool is_null_pointer_exception(address epc) { return epc == code_begin()+_npe_offset; }
183186

0 commit comments

Comments
 (0)