Skip to content

Commit

Permalink
8289060: Undefined Behaviour in class VMReg
Browse files Browse the repository at this point in the history
Reviewed-by: jvernee, kvn
  • Loading branch information
Andrew Haley committed Jul 6, 2022
1 parent 82a8bd7 commit dfb24ae
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 20 deletions.
9 changes: 7 additions & 2 deletions src/hotspot/share/code/vmreg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
#include "asm/assembler.hpp"
#include "code/vmreg.hpp"

// First VMReg value that could refer to a stack slot
VMReg VMRegImpl::stack0 = (VMReg)(intptr_t)((ConcreteRegisterImpl::number_of_registers + 7) & ~7);
// First VMReg value that could refer to a stack slot. This is only
// used by SA and jvmti, but it's a leaky abstraction: SA and jvmti
// "know" that stack0 is an integer masquerading as a pointer. For the
// sake of those clients, we preserve this interface.
VMReg VMRegImpl::stack0 = (VMReg)VMRegImpl::stack_0()->value();

// VMRegs are 4 bytes wide on all platforms
const int VMRegImpl::stack_slot_size = 4;
Expand All @@ -49,4 +52,6 @@ void VMRegImpl::print_on(outputStream* st) const {
}
}

VMRegImpl all_VMRegs[ConcreteRegisterImpl::number_of_registers + 1];

void VMRegImpl::print() const { print_on(tty); }
46 changes: 29 additions & 17 deletions src/hotspot/share/code/vmreg.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,27 @@ friend class OptoReg;
BAD_REG = -1
};



// Despite being private, this field is exported to the
// serviceability agent and our friends. It's not really a pointer,
// but that's fine and dandy as long as no-one tries to dereference
// it.
static VMReg stack0;

static constexpr VMReg first();
// Names for registers
static const char *regName[];
static const int register_count;


public:

static VMReg as_VMReg(int val, bool bad_ok = false) { assert(val > BAD_REG || bad_ok, "invalid"); return (VMReg) (intptr_t) val; }
static constexpr VMReg stack_0() {
return first() + ((ConcreteRegisterImpl::number_of_registers + 7) & ~7);
}

static VMReg as_VMReg(int val, bool bad_ok = false) {
assert(val > BAD_REG || bad_ok, "invalid");
return val + first();
}

const char* name() {
if (is_reg()) {
Expand All @@ -78,9 +88,10 @@ friend class OptoReg;
return "STACKED REG";
}
}
static VMReg Bad() { return (VMReg) (intptr_t) BAD_REG; }
bool is_valid() const { return ((intptr_t) this) != BAD_REG; }
bool is_stack() const { return (intptr_t) this >= (intptr_t) stack0; }
intptr_t value() const { return this - first(); }
static VMReg Bad() { return BAD_REG+first(); }
bool is_valid() const { return value() != BAD_REG; }
bool is_stack() const { return this >= stack_0(); }
bool is_reg() const { return is_valid() && !is_stack(); }

// A concrete register is a value that returns true for is_reg() and is
Expand All @@ -98,21 +109,19 @@ friend class OptoReg;
// we don't try and get the VMReg number of a physical register that doesn't
// have an expressible part. That would be pd specific code
VMReg next() {
assert((is_reg() && value() < stack0->value() - 1) || is_stack(), "must be");
return (VMReg)(intptr_t)(value() + 1);
assert((is_reg() && this < stack_0() - 1) || is_stack(), "must be");
return this + 1;
}
VMReg next(int i) {
assert((is_reg() && value() < stack0->value() - i) || is_stack(), "must be");
return (VMReg)(intptr_t)(value() + i);
assert((is_reg() && this < stack_0() - i) || is_stack(), "must be");
return this + i;
}
VMReg prev() {
assert((is_stack() && value() > stack0->value()) || (is_reg() && value() != 0), "must be");
return (VMReg)(intptr_t)(value() - 1);
assert((is_stack() && this > stack_0()) || (is_reg() && value() != 0), "must be");
return this - 1;
}


intptr_t value() const {return (intptr_t) this; }

void print_on(outputStream* st) const;
void print() const;

Expand All @@ -131,12 +140,12 @@ friend class OptoReg;

// Convert register numbers to stack slots and vice versa
static VMReg stack2reg( int idx ) {
return (VMReg) (intptr_t) (stack0->value() + idx);
return stack_0() + idx;
}

uintptr_t reg2stack() {
assert( is_stack(), "Not a stack-based register" );
return value() - stack0->value();
return this - stack_0();
}

static void set_regName();
Expand All @@ -145,6 +154,9 @@ friend class OptoReg;

};

extern VMRegImpl all_VMRegs[ConcreteRegisterImpl::number_of_registers + 1] INTERNAL_VISIBILITY;
inline constexpr VMReg VMRegImpl::first() { return all_VMRegs + 1; }

//---------------------------VMRegPair-------------------------------------------
// Pairs of 32-bit registers for arguments.
// SharedRuntime::java_calling_convention will overwrite the structs with
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/optoreg.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class OptoReg {
}

static OptoReg::Name stack0() {
return VMRegImpl::stack0->value();
return VMRegImpl::stack_0()->value();
}

static const char* regname(OptoReg::Name n) {
Expand Down

1 comment on commit dfb24ae

@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.