Skip to content
This repository was archived by the owner on Feb 29, 2024. It is now read-only.

Commit de805f2

Browse files
committed
Use a more descriptive error message when an expression tries to call methods on a class that is not loaded yet.
Before: "The instance method ... is undefined for the type ..." After: "Java class ... has not been loaded yet" ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=156081390
1 parent 62afbac commit de805f2

File tree

4 files changed

+73
-30
lines changed

4 files changed

+73
-30
lines changed

src/agent/jvm_readers_factory.cc

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -330,27 +330,43 @@ JvmReadersFactory::FindLocalInstanceMethods(
330330
evaluators_->method_locals->GetLocalVariables(method_);
331331

332332
if (method->local_instance == nullptr) {
333-
return {}; // This is a static method, no matches.
333+
// The current class is expected to be loaded. So, we must be in a
334+
// static method with no instance methods.
335+
return {};
334336
}
335337

336-
return FindInstanceMethods(
338+
std::vector<ClassMetadataReader::Method> methods;
339+
FormatMessageModel unused_error_message;
340+
bool unused_success = FindInstanceMethods(
337341
method->local_instance->GetStaticType().object_signature,
338-
method_name);
342+
method_name,
343+
&methods,
344+
&unused_error_message);
345+
DCHECK(unused_success);
346+
347+
return methods;
339348
}
340349

341350

342-
std::vector<ClassMetadataReader::Method>
343-
JvmReadersFactory::FindInstanceMethods(
351+
bool JvmReadersFactory::FindInstanceMethods(
344352
const string& class_signature,
345-
const string& method_name) {
353+
const string& method_name,
354+
std::vector<ClassMetadataReader::Method>* methods,
355+
FormatMessageModel* error_message) {
346356
JniLocalRef cls =
347357
evaluators_->class_indexer->FindClassBySignature(class_signature);
348358
if (cls == nullptr) {
349-
LOG(ERROR) << "Local instance class not found: " << class_signature;
350-
return {};
359+
LOG(ERROR) << "Instance class not found: " << class_signature;
360+
*error_message = {
361+
ClassNotLoaded,
362+
{ TypeNameFromJObjectSignature(class_signature) }
363+
};
364+
return false;
351365
}
352366

353-
return FindClassMethods(static_cast<jclass>(cls.get()), false, method_name);
367+
*methods =
368+
FindClassMethods(static_cast<jclass>(cls.get()), false, method_name);
369+
return true;
354370
}
355371

356372

@@ -359,24 +375,28 @@ JvmReadersFactory::FindStaticMethods(
359375
const string& method_name) {
360376
JniLocalRef cls = GetMethodDeclaringClass(method_);
361377
if (cls == nullptr) {
378+
// This should not happen. The current class should always be loaded.
362379
return {};
363380
}
364381

365382
return FindClassMethods(static_cast<jclass>(cls.get()), true, method_name);
366383
}
367384

368385

369-
std::vector<ClassMetadataReader::Method>
370-
JvmReadersFactory::FindStaticMethods(
386+
bool JvmReadersFactory::FindStaticMethods(
371387
const string& class_name,
372-
const string& method_name) {
373-
FormatMessageModel error_message; // TODO(vlif): propagate this error.
374-
JniLocalRef cls = FindClassByName(class_name, &error_message);
388+
const string& method_name,
389+
std::vector<ClassMetadataReader::Method>* methods,
390+
FormatMessageModel* error_message) {
391+
JniLocalRef cls = FindClassByName(class_name, error_message);
375392
if (cls == nullptr) {
376-
return {};
393+
*error_message = { ClassNotLoaded, { class_name } };
394+
return false;
377395
}
378396

379-
return FindClassMethods(static_cast<jclass>(cls.get()), true, method_name);
397+
*methods =
398+
FindClassMethods(static_cast<jclass>(cls.get()), true, method_name);
399+
return true;
380400
}
381401

382402

src/agent/jvm_readers_factory.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,20 @@ class JvmReadersFactory : public ReadersFactory {
7575
std::vector<ClassMetadataReader::Method> FindLocalInstanceMethods(
7676
const string& method_name) override;
7777

78-
std::vector<ClassMetadataReader::Method> FindInstanceMethods(
78+
bool FindInstanceMethods(
7979
const string& class_signature,
80-
const string& method_name) override;
80+
const string& method_name,
81+
std::vector<ClassMetadataReader::Method>* methods,
82+
FormatMessageModel* error_message) override;
8183

8284
std::vector<ClassMetadataReader::Method> FindStaticMethods(
8385
const string& method_name) override;
8486

85-
std::vector<ClassMetadataReader::Method> FindStaticMethods(
87+
bool FindStaticMethods(
8688
const string& class_name,
87-
const string& method_name) override;
89+
const string& method_name,
90+
std::vector<ClassMetadataReader::Method>* methods,
91+
FormatMessageModel* error_message) override;
8892

8993
std::unique_ptr<ArrayReader> CreateArrayReader(
9094
const JSignature& array_signature) override;

src/agent/method_call_evaluator.cc

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,15 @@ void MethodCallEvaluator::MatchInstanceSourceMethod(
297297
return;
298298
}
299299

300-
std::vector<ClassMetadataReader::Method> instance_methods =
301-
readers_factory->FindInstanceMethods(
300+
std::vector<ClassMetadataReader::Method> instance_methods;
301+
if (!readers_factory->FindInstanceMethods(
302302
instance_source_->GetStaticType().object_signature,
303-
method_name_);
303+
method_name_,
304+
&instance_methods,
305+
error_message)) {
306+
*matched = true;
307+
return;
308+
}
304309

305310
MatchMethods(
306311
readers_factory,
@@ -327,8 +332,15 @@ void MethodCallEvaluator::MatchExplicitStaticMethod(
327332
ReadersFactory* readers_factory,
328333
bool* matched,
329334
FormatMessageModel* error_message) {
330-
std::vector<ClassMetadataReader::Method> static_methods =
331-
readers_factory->FindStaticMethods(possible_class_name_, method_name_);
335+
std::vector<ClassMetadataReader::Method> static_methods;
336+
if (!readers_factory->FindStaticMethods(
337+
possible_class_name_,
338+
method_name_,
339+
&static_methods,
340+
error_message)) {
341+
*matched = true;
342+
return;
343+
}
332344

333345
MatchMethods(
334346
readers_factory,

src/agent/readers_factory.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,20 +112,27 @@ class ReadersFactory {
112112

113113
// Finds signatures of all instance methods named "method_name" in the
114114
// specified class.
115-
virtual std::vector<ClassMetadataReader::Method> FindInstanceMethods(
115+
// Returns false if the class for class_signature is not loaded, true
116+
// otherwise.
117+
virtual bool FindInstanceMethods(
116118
const string& class_signature,
117-
const string& method_name) = 0;
119+
const string& method_name,
120+
std::vector<ClassMetadataReader::Method>* methods,
121+
FormatMessageModel* error_message) = 0;
118122

119123
// Finds signatures of all static methods named "method_name" in the current
120124
// class.
121125
virtual std::vector<ClassMetadataReader::Method> FindStaticMethods(
122126
const string& method_name) = 0;
123127

124128
// Finds signatures of all static methods named "method_name" in the current
125-
// class. Returns "MethodCaller" instances for each such method.
126-
virtual std::vector<ClassMetadataReader::Method> FindStaticMethods(
129+
// class. Populates "MethodCaller" instances for each such method.
130+
// Returns false if the class for class_name is not loaded, true otherwise.
131+
virtual bool FindStaticMethods(
127132
const string& class_name,
128-
const string& method_name) = 0;
133+
const string& method_name,
134+
std::vector<ClassMetadataReader::Method>* methods,
135+
FormatMessageModel* error_message) = 0;
129136

130137
// Creates an object to read native array in expression evaluation. Returns
131138
// nullptr if "array_signature" doesn't correspond to an array.

0 commit comments

Comments
 (0)