Revert "Add verifier fallback for JVMTI Get/SetLocalVariable"
This reverts commit e48fd0b4780efadc6b3433fe7a56aa5be2a84325.
Reason for revert: Fails libjdwp tests. In particular:
org.apache.harmony.jpda.tests.jdwp.StackFrame_SetValuesTest
Bug: 131711256
Change-Id: Id46da7c0d26769f8f4bd469cdfb8049f6812295a
diff --git a/openjdkjvmti/ti_method.cc b/openjdkjvmti/ti_method.cc
index 4c6ea21..408ce69 100644
--- a/openjdkjvmti/ti_method.cc
+++ b/openjdkjvmti/ti_method.cc
@@ -31,33 +31,22 @@
#include "ti_method.h"
-#include <initializer_list>
#include <type_traits>
-#include <variant>
-#include "android-base/macros.h"
#include "arch/context.h"
#include "art_jvmti.h"
#include "art_method-inl.h"
#include "base/enums.h"
-#include "base/globals.h"
-#include "base/macros.h"
#include "base/mutex-inl.h"
#include "deopt_manager.h"
#include "dex/code_item_accessors-inl.h"
-#include "dex/code_item_accessors.h"
#include "dex/dex_file_annotations.h"
#include "dex/dex_file_types.h"
-#include "dex/dex_instruction.h"
-#include "dex/dex_instruction_iterator.h"
#include "dex/modifiers.h"
-#include "dex/primitive.h"
#include "events-inl.h"
#include "gc_root-inl.h"
-#include "handle.h"
#include "jit/jit.h"
#include "jni/jni_internal.h"
-#include "jvmti.h"
#include "mirror/class-inl.h"
#include "mirror/class_loader.h"
#include "mirror/object-inl.h"
@@ -67,18 +56,13 @@
#include "obj_ptr.h"
#include "runtime_callbacks.h"
#include "scoped_thread_state_change-inl.h"
-#include "scoped_thread_state_change.h"
#include "stack.h"
#include "thread-current-inl.h"
#include "thread.h"
#include "thread_list.h"
-#include "ti_logging.h"
#include "ti_stack.h"
#include "ti_thread.h"
#include "ti_phase.h"
-#include "verifier/register_line-inl.h"
-#include "verifier/reg_type-inl.h"
-#include "verifier/method_verifier-inl.h"
namespace openjdkjvmti {
@@ -542,21 +526,10 @@
class CommonLocalVariableClosure : public art::Closure {
public:
- // The verifier isn't always able to be as specific as the local-variable-table. We can only get
- // 32-bit, 64-bit or reference.
- enum class VerifierPrimitiveType {
- k32BitValue, // float, int, short, char, boolean, byte
- k64BitValue, // double, long
- kReferenceValue, // Object
- kZeroValue, // null or zero constant. Might be either k32BitValue or kReferenceValue
- };
+ CommonLocalVariableClosure(jint depth, jint slot)
+ : result_(ERR(INTERNAL)), depth_(depth), slot_(slot) {}
- using SlotType = std::variant<art::Primitive::Type, VerifierPrimitiveType>;
-
- CommonLocalVariableClosure(jvmtiEnv* jvmti, jint depth, jint slot)
- : jvmti_(jvmti), result_(ERR(INTERNAL)), depth_(depth), slot_(slot) {}
-
- void Run(art::Thread* self) override REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ void Run(art::Thread* self) override REQUIRES(art::Locks::mutator_lock_) {
art::Locks::mutator_lock_->AssertSharedHeld(art::Thread::Current());
bool needs_instrument;
{
@@ -587,7 +560,7 @@
return;
}
std::string descriptor;
- SlotType slot_type{ art::Primitive::kPrimVoid };
+ art::Primitive::Type slot_type = art::Primitive::kPrimVoid;
jvmtiError err = GetSlotType(method, pc, &descriptor, &slot_type);
if (err != OK) {
result_ = err;
@@ -614,190 +587,56 @@
virtual jvmtiError Execute(art::ArtMethod* method, art::StackVisitor& visitor)
REQUIRES_SHARED(art::Locks::mutator_lock_) = 0;
virtual jvmtiError GetTypeError(art::ArtMethod* method,
- SlotType type,
+ art::Primitive::Type type,
const std::string& descriptor)
REQUIRES_SHARED(art::Locks::mutator_lock_) = 0;
jvmtiError GetSlotType(art::ArtMethod* method,
uint32_t dex_pc,
/*out*/std::string* descriptor,
- /*out*/SlotType* type)
- REQUIRES_SHARED(art::Locks::mutator_lock_);
-
- jvmtiError InferSlotTypeFromVerifier(art::ArtMethod* method,
- uint32_t dex_pc,
- /*out*/ std::string* descriptor,
- /*out*/ SlotType* type)
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- art::Thread* self = art::Thread::Current();
- art::StackHandleScope<2> hs(self);
- std::unique_ptr<art::verifier::MethodVerifier> verifier(
- art::verifier::MethodVerifier::CalculateVerificationInfo(
- self,
- method,
- hs.NewHandle(method->GetDexCache()),
- hs.NewHandle(method->GetDeclaringClass()->GetClassLoader())));
- if (verifier == nullptr) {
- JVMTI_LOG(WARNING, jvmti_) << "Unable to extract verification information from "
- << method->PrettyMethod() << " due to hard verification failures! "
- << "How did this method even get loaded!";
- return ERR(INTERNAL);
- }
- art::verifier::RegisterLine* line = verifier->GetRegLine(dex_pc);
- if (line == nullptr) {
- JVMTI_LOG(WARNING, jvmti_) << "Unable to determine register line at dex-pc " << dex_pc
- << " for method " << method->PrettyMethod();
+ /*out*/art::Primitive::Type* type)
+ REQUIRES(art::Locks::mutator_lock_) {
+ const art::DexFile* dex_file = method->GetDexFile();
+ if (dex_file == nullptr) {
return ERR(OPAQUE_FRAME);
}
- const art::verifier::RegType& rt = line->GetRegisterType(verifier.get(), slot_);
- if (rt.IsUndefined()) {
- return ERR(INVALID_SLOT);
- } else if (rt.IsNonZeroReferenceTypes() || rt.IsNull()) {
- *descriptor = (rt.HasClass() ? rt.GetDescriptor() : "Ljava/lang/Object;");
- *type = VerifierPrimitiveType::kReferenceValue;
- return OK;
- } else if (rt.IsZero()) {
- *descriptor = "I";
- *type = VerifierPrimitiveType::kZeroValue;
- return OK;
- } else if (rt.IsCategory1Types()) {
- *descriptor = "I";
- *type = VerifierPrimitiveType::k32BitValue;
- return OK;
- } else if (rt.IsCategory2Types() && rt.IsLowHalf()) {
- *descriptor = "J";
- *type = VerifierPrimitiveType::k64BitValue;
- return OK;
- } else {
- // The slot doesn't have a type. Must not be valid here.
- return ERR(INVALID_SLOT);
+ art::CodeItemDebugInfoAccessor accessor(method->DexInstructionDebugInfo());
+ if (!accessor.HasCodeItem()) {
+ return ERR(OPAQUE_FRAME);
}
- }
-
- constexpr VerifierPrimitiveType SquashType(SlotType t) {
- if (std::holds_alternative<art::Primitive::Type>(t)) {
- switch (std::get<art::Primitive::Type>(t)) {
- // 32-bit primitives
- case art::Primitive::kPrimByte:
- case art::Primitive::kPrimChar:
- case art::Primitive::kPrimInt:
- case art::Primitive::kPrimShort:
- case art::Primitive::kPrimBoolean:
- case art::Primitive::kPrimFloat:
- return VerifierPrimitiveType::k32BitValue;
- // 64-bit primitives
- case art::Primitive::kPrimLong:
- case art::Primitive::kPrimDouble:
- return VerifierPrimitiveType::k64BitValue;
- case art::Primitive::kPrimNot:
- return VerifierPrimitiveType::kReferenceValue;
- case art::Primitive::kPrimVoid:
- LOG(FATAL) << "Got kPrimVoid";
- UNREACHABLE();
+ bool found = false;
+ *type = art::Primitive::kPrimVoid;
+ descriptor->clear();
+ auto visitor = [&](const art::DexFile::LocalInfo& entry) {
+ if (!found &&
+ entry.start_address_ <= dex_pc &&
+ entry.end_address_ > dex_pc &&
+ entry.reg_ == slot_) {
+ found = true;
+ *type = art::Primitive::GetType(entry.descriptor_[0]);
+ *descriptor = entry.descriptor_;
}
- } else {
- return std::get<VerifierPrimitiveType>(t);
+ };
+ if (!accessor.DecodeDebugLocalInfo(method->IsStatic(), method->GetDexMethodIndex(), visitor) ||
+ !found) {
+ // Something went wrong with decoding the debug information. It might as well not be there.
+ return ERR(INVALID_SLOT);
}
+ return OK;
}
- jvmtiEnv* jvmti_;
jvmtiError result_;
jint depth_;
jint slot_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(CommonLocalVariableClosure);
};
-std::ostream& operator<<(std::ostream& os,
- CommonLocalVariableClosure::VerifierPrimitiveType state) {
- switch (state) {
- case CommonLocalVariableClosure::VerifierPrimitiveType::k32BitValue:
- return os << "32BitValue";
- case CommonLocalVariableClosure::VerifierPrimitiveType::k64BitValue:
- return os << "64BitValue";
- case CommonLocalVariableClosure::VerifierPrimitiveType::kReferenceValue:
- return os << "ReferenceValue";
- case CommonLocalVariableClosure::VerifierPrimitiveType::kZeroValue:
- return os << "ZeroValue";
- }
-}
-
-std::ostream& operator<<(std::ostream& os, CommonLocalVariableClosure::SlotType state) {
- if (std::holds_alternative<art::Primitive::Type>(state)) {
- return os << "Primitive::Type[" << std::get<art::Primitive::Type>(state) << "]";
- } else {
- return os << "VerifierPrimitiveType["
- << std::get<CommonLocalVariableClosure::VerifierPrimitiveType>(state) << "]";
- }
-}
-
-jvmtiError CommonLocalVariableClosure::GetSlotType(art::ArtMethod* method,
- uint32_t dex_pc,
- /*out*/ std::string* descriptor,
- /*out*/ SlotType* type) {
- const art::DexFile* dex_file = method->GetDexFile();
- if (dex_file == nullptr) {
- return ERR(OPAQUE_FRAME);
- }
- art::CodeItemDebugInfoAccessor accessor(method->DexInstructionDebugInfo());
- if (!accessor.HasCodeItem()) {
- return ERR(OPAQUE_FRAME);
- }
- bool found = false;
- *type = art::Primitive::kPrimVoid;
- descriptor->clear();
- auto visitor = [&](const art::DexFile::LocalInfo& entry) {
- if (!found && entry.start_address_ <= dex_pc && entry.end_address_ > dex_pc &&
- entry.reg_ == slot_) {
- found = true;
- *type = art::Primitive::GetType(entry.descriptor_[0]);
- *descriptor = entry.descriptor_;
- }
- };
- if (!accessor.DecodeDebugLocalInfo(method->IsStatic(), method->GetDexMethodIndex(), visitor) ||
- !found) {
- // Something went wrong with decoding the debug information. It might as well not be there.
- // Try to find the type with the verifier.
- // TODO This is very slow.
- return InferSlotTypeFromVerifier(method, dex_pc, descriptor, type);
- } else if (art::kIsDebugBuild) {
- std::string type_unused;
- SlotType verifier_type{ art::Primitive::kPrimVoid };
- DCHECK_EQ(InferSlotTypeFromVerifier(method, dex_pc, &type_unused, &verifier_type), OK)
- << method->PrettyMethod() << " failed to verify!";
- if (*type == SlotType{ art::Primitive::kPrimNot }) {
- // We cannot distinguish between a constant 0 and a null reference so we return that it is a
- // 32bit value (Due to the way references are read by the interpreter this is safe even if
- // it's modified, the value will remain null). This is not ideal since it prevents modifying
- // locals in some circumstances but generally is not a big deal (since one can just modify it
- // later once it's been determined to be a reference by a later instruction).
- DCHECK(verifier_type == SlotType { VerifierPrimitiveType::kZeroValue } ||
- verifier_type == SlotType { VerifierPrimitiveType::kReferenceValue })
- << "Verifier disagrees on type of slot! debug: " << *type
- << " verifier: " << verifier_type;
- } else if (verifier_type == SlotType { VerifierPrimitiveType::kZeroValue }) {
- DCHECK(VerifierPrimitiveType::k32BitValue == SquashType(*type) ||
- VerifierPrimitiveType::kReferenceValue == SquashType(*type))
- << "Verifier disagrees on type of slot! debug: " << *type
- << " verifier: " << verifier_type;
- } else {
- DCHECK_EQ(SquashType(verifier_type), SquashType(*type))
- << "Verifier disagrees on type of slot! debug: " << *type
- << " verifier: " << verifier_type;
- }
- }
- return OK;
-}
-
class GetLocalVariableClosure : public CommonLocalVariableClosure {
public:
- GetLocalVariableClosure(jvmtiEnv* jvmti,
- jint depth,
+ GetLocalVariableClosure(jint depth,
jint slot,
art::Primitive::Type type,
jvalue* val)
- : CommonLocalVariableClosure(jvmti, depth, slot),
+ : CommonLocalVariableClosure(depth, slot),
type_(type),
val_(val),
obj_val_(nullptr) {}
@@ -817,61 +656,22 @@
}
protected:
- jvmtiError
- GetTypeError(art::ArtMethod* method, SlotType slot_type, const std::string& descriptor) override
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- jvmtiError res = GetTypeErrorInner(method, slot_type, descriptor);
- if (res == ERR(TYPE_MISMATCH)) {
- JVMTI_LOG(INFO, jvmti_) << "Unable to Get local variable in slot " << slot_ << ". Expected"
- << " slot to be of type compatible with " << SlotType { type_ }
- << " but slot is " << slot_type;
- } else if (res != OK) {
- JVMTI_LOG(INFO, jvmti_) << "Unable to get local variable in slot " << slot_ << ".";
- }
- return res;
- }
-
- jvmtiError GetTypeErrorInner(art::ArtMethod* method ATTRIBUTE_UNUSED,
- SlotType slot_type,
- const std::string& descriptor ATTRIBUTE_UNUSED)
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- switch (type_) {
- case art::Primitive::kPrimFloat:
- case art::Primitive::kPrimInt: {
- if (std::holds_alternative<VerifierPrimitiveType>(slot_type)) {
- return (slot_type == SlotType { VerifierPrimitiveType::k32BitValue } ||
- slot_type == SlotType { VerifierPrimitiveType::kZeroValue })
- ? OK
- : ERR(TYPE_MISMATCH);
- } else if (type_ == art::Primitive::kPrimFloat ||
- slot_type == SlotType { art::Primitive::kPrimFloat }) {
- // Check that we are actually a float.
- return (SlotType { type_ } == slot_type) ? OK : ERR(TYPE_MISMATCH);
- } else {
- // Some smaller int type.
- return SquashType(slot_type) == SquashType(SlotType { type_ }) ? OK : ERR(TYPE_MISMATCH);
- }
- }
- case art::Primitive::kPrimLong:
- case art::Primitive::kPrimDouble: {
- // todo
- if (std::holds_alternative<VerifierPrimitiveType>(slot_type)) {
- return (slot_type == SlotType { VerifierPrimitiveType::k64BitValue })
- ? OK
- : ERR(TYPE_MISMATCH);
- } else {
- return slot_type == SlotType { type_ } ? OK : ERR(TYPE_MISMATCH);
- }
- }
- case art::Primitive::kPrimNot:
- return (SquashType(slot_type) == VerifierPrimitiveType::kReferenceValue ||
- SquashType(slot_type) == VerifierPrimitiveType::kZeroValue)
- ? OK
- : ERR(TYPE_MISMATCH);
- case art::Primitive::kPrimShort:
- case art::Primitive::kPrimChar:
+ jvmtiError GetTypeError(art::ArtMethod* method ATTRIBUTE_UNUSED,
+ art::Primitive::Type slot_type,
+ const std::string& descriptor ATTRIBUTE_UNUSED)
+ override REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ switch (slot_type) {
case art::Primitive::kPrimByte:
+ case art::Primitive::kPrimChar:
+ case art::Primitive::kPrimInt:
+ case art::Primitive::kPrimShort:
case art::Primitive::kPrimBoolean:
+ return type_ == art::Primitive::kPrimInt ? OK : ERR(TYPE_MISMATCH);
+ case art::Primitive::kPrimLong:
+ case art::Primitive::kPrimFloat:
+ case art::Primitive::kPrimDouble:
+ case art::Primitive::kPrimNot:
+ return type_ == slot_type ? OK : ERR(TYPE_MISMATCH);
case art::Primitive::kPrimVoid:
LOG(FATAL) << "Unexpected primitive type " << slot_type;
UNREACHABLE();
@@ -935,7 +735,7 @@
jobject obj_val_;
};
-jvmtiError MethodUtil::GetLocalVariableGeneric(jvmtiEnv* env,
+jvmtiError MethodUtil::GetLocalVariableGeneric(jvmtiEnv* env ATTRIBUTE_UNUSED,
jthread thread,
jint depth,
jint slot,
@@ -953,7 +753,7 @@
art::Locks::thread_list_lock_->ExclusiveUnlock(self);
return err;
}
- GetLocalVariableClosure c(env, depth, slot, type, val);
+ GetLocalVariableClosure c(depth, slot, type, val);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
if (!target->RequestSynchronousCheckpoint(&c)) {
return ERR(THREAD_NOT_ALIVE);
@@ -964,100 +764,49 @@
class SetLocalVariableClosure : public CommonLocalVariableClosure {
public:
- SetLocalVariableClosure(jvmtiEnv* jvmti,
- art::Thread* caller,
+ SetLocalVariableClosure(art::Thread* caller,
jint depth,
jint slot,
art::Primitive::Type type,
jvalue val)
- : CommonLocalVariableClosure(jvmti, depth, slot), caller_(caller), type_(type), val_(val) {}
+ : CommonLocalVariableClosure(depth, slot), caller_(caller), type_(type), val_(val) {}
protected:
- jvmtiError
- GetTypeError(art::ArtMethod* method, SlotType slot_type, const std::string& descriptor) override
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- jvmtiError res = GetTypeErrorInner(method, slot_type, descriptor);
- if (res != OK) {
- if (res == ERR(TYPE_MISMATCH)) {
- std::ostringstream desc_exp;
- std::ostringstream desc_set;
- if (type_ == art::Primitive::kPrimNot) {
- desc_exp << " (type: " << descriptor << ")";
- art::ObjPtr<art::mirror::Object> new_val(art::Thread::Current()->DecodeJObject(val_.l));
- desc_set << " (type: "
- << (new_val.IsNull() ? "NULL" : new_val->GetClass()->PrettyDescriptor()) << ")";
- }
- JVMTI_LOG(INFO, jvmti_) << "Unable to Set local variable in slot " << slot_ << ". Expected"
- << " slot to be of type compatible with " << SlotType{ type_ }
- << desc_set.str() << " but slot is " << slot_type << desc_exp.str();
- } else {
- JVMTI_LOG(INFO, jvmti_) << "Unable to set local variable in slot " << slot_ << ". "
- << err_.str();
- }
- }
- return res;
- }
-
- jvmtiError
- GetTypeErrorInner(art::ArtMethod* method, SlotType slot_type, const std::string& descriptor)
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- switch (SquashType(SlotType{ type_ })) {
- case VerifierPrimitiveType::k32BitValue: {
- if (slot_type == SlotType{ VerifierPrimitiveType::kZeroValue }) {
- if (val_.i == 0) {
- return OK;
- } else {
- err_ << "Cannot determine if slot " << slot_ << " is a null reference or 32bit "
- << "constant. Cannot allow writing to slot.";
- return ERR(INTERNAL);
- }
- } else if (SquashType(slot_type) != VerifierPrimitiveType::k32BitValue) {
- return ERR(TYPE_MISMATCH);
- } else if (slot_type == SlotType { VerifierPrimitiveType::k32BitValue } ||
- slot_type == SlotType { type_ }) {
- return OK;
- } else if (type_ == art::Primitive::kPrimFloat ||
- slot_type == SlotType { art::Primitive::kPrimFloat }) {
- // we should have hit the get == type_ above
- return ERR(TYPE_MISMATCH);
- } else {
- // Some smaller type then int.
- return OK;
- }
- }
- case VerifierPrimitiveType::k64BitValue: {
- if (slot_type == SlotType { VerifierPrimitiveType::k64BitValue } ||
- slot_type == SlotType { type_ }) {
- return OK;
- } else {
- return ERR(TYPE_MISMATCH);
- }
- }
- case VerifierPrimitiveType::kReferenceValue: {
- if (SquashType(slot_type) != VerifierPrimitiveType::kReferenceValue &&
- SquashType(slot_type) != VerifierPrimitiveType::kZeroValue) {
+ jvmtiError GetTypeError(art::ArtMethod* method,
+ art::Primitive::Type slot_type,
+ const std::string& descriptor)
+ override REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ switch (slot_type) {
+ case art::Primitive::kPrimNot: {
+ if (type_ != art::Primitive::kPrimNot) {
return ERR(TYPE_MISMATCH);
} else if (val_.l == nullptr) {
return OK;
- } else if (slot_type == SlotType { VerifierPrimitiveType::kZeroValue }) {
- err_ << "Cannot determine if slot " << slot_ << " is a null "
- << "reference or 32bit constant. Cannot allow writing to slot.";
- return ERR(INTERNAL);
} else {
art::ClassLinker* cl = art::Runtime::Current()->GetClassLinker();
- art::ObjPtr<art::mirror::Class> set_class = caller_->DecodeJObject(val_.l)->GetClass();
+ art::ObjPtr<art::mirror::Class> set_class =
+ caller_->DecodeJObject(val_.l)->GetClass();
art::ObjPtr<art::mirror::ClassLoader> loader =
method->GetDeclaringClass()->GetClassLoader();
art::ObjPtr<art::mirror::Class> slot_class =
cl->LookupClass(caller_, descriptor.c_str(), loader);
- DCHECK(!slot_class.IsNull()) << descriptor << " slot: " << slot_type;
+ DCHECK(!slot_class.IsNull());
return slot_class->IsAssignableFrom(set_class) ? OK : ERR(TYPE_MISMATCH);
}
}
- case VerifierPrimitiveType::kZeroValue: {
- LOG(FATAL) << "Illegal result from SquashType of art::Primitive::Type " << type_;
+ case art::Primitive::kPrimByte:
+ case art::Primitive::kPrimChar:
+ case art::Primitive::kPrimInt:
+ case art::Primitive::kPrimShort:
+ case art::Primitive::kPrimBoolean:
+ return type_ == art::Primitive::kPrimInt ? OK : ERR(TYPE_MISMATCH);
+ case art::Primitive::kPrimLong:
+ case art::Primitive::kPrimFloat:
+ case art::Primitive::kPrimDouble:
+ return type_ == slot_type ? OK : ERR(TYPE_MISMATCH);
+ case art::Primitive::kPrimVoid:
+ LOG(FATAL) << "Unexpected primitive type " << slot_type;
UNREACHABLE();
- }
}
}
@@ -1108,10 +857,9 @@
art::Thread* caller_;
art::Primitive::Type type_;
jvalue val_;
- std::ostringstream err_;
};
-jvmtiError MethodUtil::SetLocalVariableGeneric(jvmtiEnv* env,
+jvmtiError MethodUtil::SetLocalVariableGeneric(jvmtiEnv* env ATTRIBUTE_UNUSED,
jthread thread,
jint depth,
jint slot,
@@ -1132,7 +880,7 @@
art::Locks::thread_list_lock_->ExclusiveUnlock(self);
return err;
}
- SetLocalVariableClosure c(env, self, depth, slot, type, val);
+ SetLocalVariableClosure c(self, depth, slot, type, val);
// RequestSynchronousCheckpoint releases the thread_list_lock_ as a part of its execution.
if (!target->RequestSynchronousCheckpoint(&c)) {
return ERR(THREAD_NOT_ALIVE);