Revert "Implement RetransformClasses"
This reverts commit a6c5e97a4395352bc8684e6af9cecb62b80c316c.
Reason for revert: Accidently introduces double-free bug in RedefineClasses.
Change-Id: I021336c4fcf0cfb304915b0ffc5eaba5f91fdd5e
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index 28ea267..6af51c4 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -242,12 +242,14 @@
}
}
+// TODO This should handle doing multiple classes at once so we need to do less cleanup when things
+// go wrong.
jvmtiError Redefiner::RedefineClasses(ArtJvmTiEnv* env,
art::Runtime* runtime,
art::Thread* self,
jint class_count,
const jvmtiClassDefinition* definitions,
- /*out*/std::string* error_msg) {
+ std::string* error_msg) {
if (env == nullptr) {
*error_msg = "env was null!";
return ERR(INVALID_ENVIRONMENT);
@@ -261,83 +263,46 @@
*error_msg = "null definitions!";
return ERR(NULL_POINTER);
}
- std::vector<ArtClassDefinition> def_vector;
- def_vector.reserve(class_count);
- for (jint i = 0; i < class_count; i++) {
- ArtClassDefinition def;
- def.dex_len = definitions[i].class_byte_count;
- def.dex_data = MakeJvmtiUniquePtr(env, const_cast<unsigned char*>(definitions[i].class_bytes));
- // We are definitely modified.
- def.modified = true;
- jvmtiError res = Transformer::FillInTransformationData(env, definitions[i].klass, &def);
- if (res != OK) {
- return res;
- }
- def_vector.push_back(std::move(def));
- }
- // Call all the transformation events.
- jvmtiError res = Transformer::RetransformClassesDirect(env,
- self,
- &def_vector);
- if (res != OK) {
- // Something went wrong with transformation!
- return res;
- }
- return RedefineClassesDirect(env, runtime, self, def_vector, error_msg);
-}
-
-jvmtiError Redefiner::RedefineClassesDirect(ArtJvmTiEnv* env,
- art::Runtime* runtime,
- art::Thread* self,
- const std::vector<ArtClassDefinition>& definitions,
- std::string* error_msg) {
- DCHECK(env != nullptr);
- if (definitions.size() == 0) {
- // We don't actually need to do anything. Just return OK.
- return OK;
- }
// Stop JIT for the duration of this redefine since the JIT might concurrently compile a method we
// are going to redefine.
art::jit::ScopedJitSuspend suspend_jit;
// Get shared mutator lock so we can lock all the classes.
art::ScopedObjectAccess soa(self);
std::vector<Redefiner::ClassRedefinition> redefinitions;
- redefinitions.reserve(definitions.size());
+ redefinitions.reserve(class_count);
Redefiner r(runtime, self, error_msg);
- for (const ArtClassDefinition& def : definitions) {
- // Only try to transform classes that have been modified.
- if (def.modified) {
- jvmtiError res = r.AddRedefinition(env, def);
- if (res != OK) {
- return res;
- }
+ for (jint i = 0; i < class_count; i++) {
+ jvmtiError res = r.AddRedefinition(env, definitions[i]);
+ if (res != OK) {
+ return res;
}
}
return r.Run();
}
-jvmtiError Redefiner::AddRedefinition(ArtJvmTiEnv* env, const ArtClassDefinition& def) {
+jvmtiError Redefiner::AddRedefinition(ArtJvmTiEnv* env, const jvmtiClassDefinition& def) {
std::string original_dex_location;
jvmtiError ret = OK;
if ((ret = GetClassLocation(env, def.klass, &original_dex_location))) {
*error_msg_ = "Unable to get original dex file location!";
return ret;
}
- char* generic_ptr_unused = nullptr;
- char* signature_ptr = nullptr;
- if ((ret = env->GetClassSignature(def.klass, &signature_ptr, &generic_ptr_unused)) != OK) {
- *error_msg_ = "Unable to get class signature!";
- return ret;
- }
- JvmtiUniquePtr generic_unique_ptr(MakeJvmtiUniquePtr(env, generic_ptr_unused));
- JvmtiUniquePtr signature_unique_ptr(MakeJvmtiUniquePtr(env, signature_ptr));
std::unique_ptr<art::MemMap> map(MoveDataToMemMap(original_dex_location,
- def.dex_len,
- def.dex_data.get(),
+ def.class_byte_count,
+ def.class_bytes,
error_msg_));
std::ostringstream os;
+ char* generic_ptr_unused = nullptr;
+ char* signature_ptr = nullptr;
+ if (env->GetClassSignature(def.klass, &signature_ptr, &generic_ptr_unused) != OK) {
+ *error_msg_ = "A jclass passed in does not seem to be valid";
+ return ERR(INVALID_CLASS);
+ }
+ // These will make sure we deallocate the signature.
+ JvmtiUniquePtr sig_unique_ptr(MakeJvmtiUniquePtr(env, signature_ptr));
+ JvmtiUniquePtr generic_unique_ptr(MakeJvmtiUniquePtr(env, generic_ptr_unused));
if (map.get() == nullptr) {
- os << "Failed to create anonymous mmap for modified dex file of class " << def.name
+ os << "Failed to create anonymous mmap for modified dex file of class " << signature_ptr
<< "in dex file " << original_dex_location << " because: " << *error_msg_;
*error_msg_ = os.str();
return ERR(OUT_OF_MEMORY);
@@ -354,7 +319,7 @@
/*verify_checksum*/true,
error_msg_));
if (dex_file.get() == nullptr) {
- os << "Unable to load modified dex file for " << def.name << ": " << *error_msg_;
+ os << "Unable to load modified dex file for " << signature_ptr << ": " << *error_msg_;
*error_msg_ = os.str();
return ERR(INVALID_CLASS_FORMAT);
}
@@ -1024,16 +989,17 @@
// Performs updates to class that will allow us to verify it.
void Redefiner::ClassRedefinition::UpdateClass(art::ObjPtr<art::mirror::Class> mclass,
art::ObjPtr<art::mirror::DexCache> new_dex_cache) {
- DCHECK_EQ(dex_file_->NumClassDefs(), 1u);
- const art::DexFile::ClassDef& class_def = dex_file_->GetClassDef(0);
- UpdateMethods(mclass, new_dex_cache, class_def);
+ const art::DexFile::ClassDef* class_def = art::OatFile::OatDexFile::FindClassDef(
+ *dex_file_, class_sig_.c_str(), art::ComputeModifiedUtf8Hash(class_sig_.c_str()));
+ DCHECK(class_def != nullptr);
+ UpdateMethods(mclass, new_dex_cache, *class_def);
UpdateFields(mclass);
// Update the class fields.
// Need to update class last since the ArtMethod gets its DexFile from the class (which is needed
// to call GetReturnTypeDescriptor and GetParameterTypeList above).
mclass->SetDexCache(new_dex_cache.Ptr());
- mclass->SetDexClassDefIndex(dex_file_->GetIndexForClassDef(class_def));
+ mclass->SetDexClassDefIndex(dex_file_->GetIndexForClassDef(*class_def));
mclass->SetDexTypeIndex(dex_file_->GetIndexForTypeId(*dex_file_->FindTypeId(class_sig_.c_str())));
}