Shark exception processing improvements
Gary Benson
gbenson at redhat.com
Thu May 28 07:41:58 PDT 2009
Hi all,
When something in HotSpot throws an exception, it stores a pointer to
the exception object in a slot in the current Java thread. There are
various places in Shark where an exception can appear, and there are
several different ways to handle them, and currently these behaviours
are selected with boolean arguments that are not particularly self-
documenting:
check_pending_exception(true);
What the hell does that mean? This commit replaces all these dodgy
booleans with an enum with somewhat more meaningful values:
enum ExceptionAction {
EX_CHECK_NONE, // don't check for pending exceptions
EX_CHECK_NO_CATCH, // if there is a pending exception then throw it
EX_CHECK_FULL // if there is a pending exception then catch it
}; // if it has a handler or throw it otherwise
This allowed me to add exception handling to a bunch of methods that
previously delegated it to their callers. This fixed a couple of bugs,
namely that the monitorenter and monitorexit bytecodes did not check
for exceptions, and the whole-method-synchronization thing always
checked for exceptions, rather than only for the rare slow case.
Cheers,
Gary
--
http://gbenson.net/
-------------- next part --------------
diff -r f78f8e38f872 ChangeLog
--- a/ChangeLog Thu May 28 05:53:28 2009 -0400
+++ b/ChangeLog Thu May 28 10:22:16 2009 -0400
@@ -1,3 +1,37 @@
+2009-05-28 Gary Benson <gbenson at redhat.com>
+
+ * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
+ (SharkTopLevelBlock::ExceptionAction): New enum.
+ (SharkTopLevelBlock::check_pending_exception): Replace
+ boolean attempt_catch argument with an ExceptionAction.
+ (SharkTopLevelBlock::handle_exception): Likewise.
+ (SharkTopLevelBlock::call_vm): Add an ExceptionAction argument.
+ (SharkTopLevelBlock::acquire_lock): Likewise.
+ (SharkTopLevelBlock::release_lock): Likewise.
+ (SharkTopLevelBlock::call_vm_nocheck): Removed.
+
+ * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
+ (SharkTopLevelBlock::check_pending_exception): Replace
+ boolean attempt_catch argument with an ExceptionAction.
+ (SharkTopLevelBlock::handle_exception): Likewise.
+ (SharkTopLevelBlock::call_vm): Add an ExceptionAction argument.
+ (SharkTopLevelBlock::acquire_lock): Likewise.
+ (SharkTopLevelBlock::release_lock): Likewise.
+ (SharkTopLevelBlock::zero_check_value): Updated.
+ (SharkTopLevelBlock::check_bounds): Likewise.
+ (SharkTopLevelBlock::add_safepoint): Likewise.
+ (SharkTopLevelBlock::call_register_finalizer): Likewise.
+ (SharkTopLevelBlock::handle_return): Likewise.
+ (SharkTopLevelBlock::do_athrow): Likewise.
+ (SharkTopLevelBlock::do_call): Likewise.
+ (SharkTopLevelBlock::do_new): Likewise.
+ (SharkTopLevelBlock::do_newarray): Likewise.
+ (SharkTopLevelBlock::do_anewarray): Likewise.
+ (SharkTopLevelBlock::do_multianewarray): Likewise.
+ (SharkTopLevelBlock::acquire_method_lock): Likewise.
+ (SharkTopLevelBlock::do_monitorenter): Likewise.
+ (SharkTopLevelBlock::do_monitorexit): Likewise.
+
2009-05-28 Gary Benson <gbenson at redhat.com>
* ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
diff -r f78f8e38f872 ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp Thu May 28 05:53:28 2009 -0400
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp Thu May 28 10:22:16 2009 -0400
@@ -294,15 +294,16 @@
builder()->SetInsertPoint(zero_block);
if (value->is_jobject()) {
- call_vm_nocheck(
+ call_vm(
SharkRuntime::throw_NullPointerException(),
builder()->pointer_constant(__FILE__),
- LLVMValue::jint_constant(__LINE__));
+ LLVMValue::jint_constant(__LINE__),
+ EX_CHECK_NONE);
}
else {
builder()->CreateUnimplemented(__FILE__, __LINE__);
}
- handle_exception(function()->CreateGetPendingException());
+ handle_exception(function()->CreateGetPendingException(), EX_CHECK_FULL);
}
void SharkTopLevelBlock::check_bounds(SharkValue* array, SharkValue* index)
@@ -318,19 +319,22 @@
builder()->SetInsertPoint(out_of_bounds);
SharkState *saved_state = current_state()->copy();
- call_vm_nocheck(
+ call_vm(
SharkRuntime::throw_ArrayIndexOutOfBoundsException(),
builder()->pointer_constant(__FILE__),
LLVMValue::jint_constant(__LINE__),
- index->jint_value());
- handle_exception(function()->CreateGetPendingException());
+ index->jint_value(),
+ EX_CHECK_NONE);
+ handle_exception(function()->CreateGetPendingException(), EX_CHECK_FULL);
set_current_state(saved_state);
builder()->SetInsertPoint(in_bounds);
}
-void SharkTopLevelBlock::check_pending_exception(bool attempt_catch)
+void SharkTopLevelBlock::check_pending_exception(ExceptionAction action)
{
+ assert(action != EX_CHECK_NONE, "shouldn't be");
+
BasicBlock *exception = function()->CreateBlock("exception");
BasicBlock *no_exception = function()->CreateBlock("no_exception");
@@ -345,15 +349,16 @@
builder()->SetInsertPoint(exception);
builder()->CreateStore(LLVMValue::null(), pending_exception_addr);
SharkState *saved_state = current_state()->copy();
- handle_exception(pending_exception, attempt_catch);
+ handle_exception(pending_exception, action);
set_current_state(saved_state);
builder()->SetInsertPoint(no_exception);
}
-void SharkTopLevelBlock::handle_exception(Value* exception, bool attempt_catch)
+void SharkTopLevelBlock::handle_exception(Value* exception,
+ ExceptionAction action)
{
- if (attempt_catch && num_exceptions() != 0) {
+ if (action == EX_CHECK_FULL && num_exceptions() != 0) {
// Clear the stack and push the exception onto it.
// We do this now to protect it across the VM call
// we may be about to make.
@@ -392,11 +397,11 @@
LLVMValue::jint_constant(indexes[i]),
builder()->CreateStructGEP(options, i));
- Value *index = call_vm_nocheck(
+ Value *index = call_vm(
SharkRuntime::find_exception_handler(),
builder()->CreateStructGEP(options, 0),
- LLVMValue::jint_constant(num_options));
- check_pending_exception(false);
+ LLVMValue::jint_constant(num_options),
+ EX_CHECK_NO_CATCH);
// Jump to the exception handler, if found
BasicBlock *no_handler = function()->CreateBlock("no_handler");
@@ -451,7 +456,7 @@
do_safepoint, safepointed);
builder()->SetInsertPoint(do_safepoint);
- call_vm(SharkRuntime::safepoint());
+ call_vm(SharkRuntime::safepoint(), EX_CHECK_FULL);
BasicBlock *safepointed_block = builder()->GetInsertBlock();
builder()->CreateBr(safepointed);
@@ -504,7 +509,7 @@
do_call, done);
builder()->SetInsertPoint(do_call);
- call_vm(SharkRuntime::register_finalizer(), receiver);
+ call_vm(SharkRuntime::register_finalizer(), receiver, EX_CHECK_FULL);
BasicBlock *branch_block = builder()->GetInsertBlock();
builder()->CreateBr(done);
@@ -527,7 +532,7 @@
// If we're returning with an exception then that exception
// takes priority and the release_lock one will be ignored.
while (num_monitors())
- release_lock();
+ release_lock(EX_CHECK_NONE);
// Reload the exception we're throwing
if (exception)
@@ -724,7 +729,7 @@
{
SharkValue *exception = pop();
check_null(exception);
- handle_exception(exception->jobject_value());
+ handle_exception(exception->jobject_value(), EX_CHECK_FULL);
}
void SharkTopLevelBlock::do_goto()
@@ -1137,7 +1142,7 @@
current_state()->cache_after_Java_call(method);
// Check for pending exceptions
- check_pending_exception();
+ check_pending_exception(EX_CHECK_FULL);
}
void SharkTopLevelBlock::do_instance_check()
@@ -1451,7 +1456,8 @@
// The slow path
call_vm(
SharkRuntime::new_instance(),
- LLVMValue::jint_constant(iter()->get_klass_index()));
+ LLVMValue::jint_constant(iter()->get_klass_index()),
+ EX_CHECK_FULL);
slow_object = function()->CreateGetVMResult();
got_slow = builder()->GetInsertBlock();
@@ -1481,7 +1487,8 @@
call_vm(
SharkRuntime::newarray(),
LLVMValue::jint_constant(type),
- pop()->jint_value());
+ pop()->jint_value(),
+ EX_CHECK_FULL);
push(SharkValue::create_generic(
ciArrayKlass::make(ciType::make(type)),
@@ -1503,7 +1510,8 @@
call_vm(
SharkRuntime::anewarray(),
LLVMValue::jint_constant(iter()->get_klass_index()),
- pop()->jint_value());
+ pop()->jint_value(),
+ EX_CHECK_FULL);
push(SharkValue::create_generic(
array_klass, function()->CreateGetVMResult(), true));
@@ -1535,7 +1543,8 @@
SharkRuntime::multianewarray(),
LLVMValue::jint_constant(iter()->get_klass_index()),
LLVMValue::jint_constant(ndims),
- builder()->CreateStructGEP(dimensions, 0));
+ builder()->CreateStructGEP(dimensions, 0),
+ EX_CHECK_FULL);
// Now we can pop the dimensions off the stack
for (int i = 0; i < ndims; i++)
@@ -1550,28 +1559,27 @@
iter()->force_bci(start()); // for the decache in acquire_lock
if (target()->is_static()) {
SharkConstantPool constants(this);
- acquire_lock(constants.java_mirror());
+ acquire_lock(constants.java_mirror(), EX_CHECK_NO_CATCH);
}
else {
- acquire_lock(local(0)->jobject_value());
+ acquire_lock(local(0)->jobject_value(), EX_CHECK_NO_CATCH);
}
- check_pending_exception(false);
}
void SharkTopLevelBlock::do_monitorenter()
{
SharkValue *lockee = pop();
check_null(lockee);
- acquire_lock(lockee->jobject_value());
+ acquire_lock(lockee->jobject_value(), EX_CHECK_FULL);
}
void SharkTopLevelBlock::do_monitorexit()
{
pop(); // don't need this (monitors are block structured)
- release_lock();
+ release_lock(EX_CHECK_FULL);
}
-void SharkTopLevelBlock::acquire_lock(Value *lockee)
+void SharkTopLevelBlock::acquire_lock(Value *lockee, ExceptionAction ea)
{
BasicBlock *try_recursive = function()->CreateBlock("try_recursive");
BasicBlock *got_recursive = function()->CreateBlock("got_recursive");
@@ -1645,7 +1653,7 @@
// It's not a recursive case so we need to drop into the runtime
builder()->SetInsertPoint(not_recursive);
- call_vm_nocheck(SharkRuntime::monitorenter(), monitor_addr);
+ call_vm(SharkRuntime::monitorenter(), monitor_addr, ea);
BasicBlock *acquired_slow = builder()->GetInsertBlock();
builder()->CreateBr(lock_acquired);
@@ -1654,7 +1662,7 @@
current_state()->merge(fast_state, acquired_fast, acquired_slow);
}
-void SharkTopLevelBlock::release_lock()
+void SharkTopLevelBlock::release_lock(ExceptionAction ea)
{
BasicBlock *not_recursive = function()->CreateBlock("not_recursive");
BasicBlock *released_fast = function()->CreateBlock("released_fast");
@@ -1697,7 +1705,7 @@
// Need to drop into the runtime to release this one
builder()->SetInsertPoint(slow_path);
- call_vm_nocheck(SharkRuntime::monitorexit(), monitor_addr);
+ call_vm(SharkRuntime::monitorexit(), monitor_addr, ea);
BasicBlock *released_slow = builder()->GetInsertBlock();
builder()->CreateBr(lock_released);
diff -r f78f8e38f872 ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp Thu May 28 05:53:28 2009 -0400
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp Thu May 28 10:22:16 2009 -0400
@@ -236,98 +236,75 @@
int bci,
SharkState* saved_state,
llvm::BasicBlock* continue_block);
+ // Exceptions
+ enum ExceptionAction {
+ EX_CHECK_NONE, // don't check for pending exceptions
+ EX_CHECK_NO_CATCH, // if there is a pending exception then throw it
+ EX_CHECK_FULL // if there is a pending exception then catch it
+ }; // if it has a handler or throw it otherwise
+ void check_pending_exception(ExceptionAction action);
+ void handle_exception(llvm::Value* exception, ExceptionAction action);
// VM calls
private:
- llvm::CallInst* call_vm_nocheck(llvm::Constant* callee,
- llvm::Value** args_start,
- llvm::Value** args_end)
+ llvm::CallInst* call_vm(llvm::Constant* callee,
+ llvm::Value** args_start,
+ llvm::Value** args_end,
+ ExceptionAction ea)
{
current_state()->decache_for_VM_call();
function()->set_last_Java_frame();
llvm::CallInst *res = builder()->CreateCall(callee, args_start, args_end);
function()->reset_last_Java_frame();
current_state()->cache_after_VM_call();
- return res;
- }
-
- llvm::CallInst* call_vm(llvm::Constant* callee,
- llvm::Value** args_start,
- llvm::Value** args_end)
- {
- llvm::CallInst* res = call_vm_nocheck(callee, args_start, args_end);
- check_pending_exception();
+ if (ea != EX_CHECK_NONE)
+ check_pending_exception(ea);
return res;
}
public:
- llvm::CallInst* call_vm(llvm::Constant* callee)
+ llvm::CallInst* call_vm(llvm::Constant* callee,
+ ExceptionAction ea)
{
llvm::Value *args[] = {thread()};
- return call_vm(callee, args, args + 1);
- }
- llvm::CallInst* call_vm(llvm::Constant* callee,
- llvm::Value* arg1)
- {
- llvm::Value *args[] = {thread(), arg1};
- return call_vm(callee, args, args + 2);
+ return call_vm(callee, args, args + 1, ea);
}
llvm::CallInst* call_vm(llvm::Constant* callee,
llvm::Value* arg1,
- llvm::Value* arg2)
+ ExceptionAction ea)
{
- llvm::Value *args[] = {thread(), arg1, arg2};
- return call_vm(callee, args, args + 3);
+ llvm::Value *args[] = {thread(), arg1};
+ return call_vm(callee, args, args + 2, ea);
}
llvm::CallInst* call_vm(llvm::Constant* callee,
llvm::Value* arg1,
llvm::Value* arg2,
- llvm::Value* arg3)
+ ExceptionAction ea)
+ {
+ llvm::Value *args[] = {thread(), arg1, arg2};
+ return call_vm(callee, args, args + 3, ea);
+ }
+ llvm::CallInst* call_vm(llvm::Constant* callee,
+ llvm::Value* arg1,
+ llvm::Value* arg2,
+ llvm::Value* arg3,
+ ExceptionAction ea)
{
llvm::Value *args[] = {thread(), arg1, arg2, arg3};
- return call_vm(callee, args, args + 4);
- }
-
- llvm::CallInst* call_vm_nocheck(llvm::Constant* callee)
- {
- llvm::Value *args[] = {thread()};
- return call_vm_nocheck(callee, args, args + 1);
- }
- llvm::CallInst* call_vm_nocheck(llvm::Constant* callee,
- llvm::Value* arg1)
- {
- llvm::Value *args[] = {thread(), arg1};
- return call_vm_nocheck(callee, args, args + 2);
- }
- llvm::CallInst* call_vm_nocheck(llvm::Constant* callee,
- llvm::Value* arg1,
- llvm::Value* arg2)
- {
- llvm::Value *args[] = {thread(), arg1, arg2};
- return call_vm_nocheck(callee, args, args + 3);
- }
- llvm::CallInst* call_vm_nocheck(llvm::Constant* callee,
- llvm::Value* arg1,
- llvm::Value* arg2,
- llvm::Value* arg3)
- {
- llvm::Value *args[] = {thread(), arg1, arg2, arg3};
- return call_vm_nocheck(callee, args, args + 4);
+ return call_vm(callee, args, args + 4, ea);
}
// Synchronization
private:
- void acquire_lock(llvm::Value* lockee);
- void release_lock();
+ void acquire_lock(llvm::Value* lockee, ExceptionAction ea);
+ void release_lock(ExceptionAction ea);
public:
void acquire_method_lock();
- // Error checking
+ // Bounds checks
private:
void check_bounds(SharkValue* array, SharkValue* index);
- void check_pending_exception(bool attempt_catch = true);
- void handle_exception(llvm::Value* exception, bool attempt_catch = true);
// Safepoints
private:
More information about the distro-pkg-dev
mailing list