Correct exception behavior for default methods
Default methods are defined to throw an IncompatibleClassChangeError
(ICCE) when they are called and there is no "best" implementation.
Previously we would simply throw an ICCE during class loading as soon
as we noticed that this would happen if called. This makes us wait
until we actually attempt to execute the method. Furthermore, this
allows us to use other, non-conflicting, methods on the object as
normal.
Furthermore, this makes us correctly throw AbstractMethodErrors in
cases where all default implementations of a method are overridden by
abstract declarations.
Adds 3 tests for this new behavior.
Bug: 24618811
Change-Id: Id891958a81f9b3862b2ce5919636aabef7d3422e
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index cae62ef..d6a27b1 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -721,6 +721,83 @@
ArtMethod** out_imt)
SHARED_REQUIRES(Locks::mutator_lock_);
+ // Does anything needed to make sure that the compiler will not generate a direct invoke to this
+ // method. Should only be called on non-invokable methods.
+ void EnsureThrowsInvocationError(ArtMethod* method)
+ SHARED_REQUIRES(Locks::mutator_lock_);
+
+ // A wrapper class representing the result of a method translation used for linking methods and
+ // updating superclass default methods. For each method in a classes vtable there are 4 states it
+ // could be in:
+ // 1) No translation is necessary. In this case there is no MethodTranslation object for it. This
+ // is the standard case and is true when the method is not overridable by a default method,
+ // the class defines a concrete implementation of the method, the default method implementation
+ // remains the same, or an abstract method stayed abstract.
+ // 2) The method must be translated to a different default method. We note this with
+ // CreateTranslatedMethod.
+ // 3) The method must be replaced with a conflict method. This happens when a superclass
+ // implements an interface with a default method and this class implements an unrelated
+ // interface that also defines that default method. We note this with CreateConflictingMethod.
+ // 4) The method must be replaced with an abstract miranda method. This happens when a superclass
+ // implements an interface with a default method and this class implements a subinterface of
+ // the superclass's interface which declares the default method abstract. We note this with
+ // CreateAbstractMethod.
+ //
+ // When a method translation is unnecessary (case #1), we don't put it into the
+ // default_translation maps. So an instance of MethodTranslation must be in one of #2-#4.
+ class MethodTranslation {
+ public:
+ // This slot must become a default conflict method.
+ static MethodTranslation CreateConflictingMethod() {
+ return MethodTranslation(Type::kConflict, /*translation*/nullptr);
+ }
+
+ // This slot must become an abstract method.
+ static MethodTranslation CreateAbstractMethod() {
+ return MethodTranslation(Type::kAbstract, /*translation*/nullptr);
+ }
+
+ // Use the given method as the current value for this vtable slot during translation.
+ static MethodTranslation CreateTranslatedMethod(ArtMethod* new_method) {
+ return MethodTranslation(Type::kTranslation, new_method);
+ }
+
+ // Returns true if this is a method that must become a conflict method.
+ bool IsInConflict() const {
+ return type_ == Type::kConflict;
+ }
+
+ // Returns true if this is a method that must become an abstract method.
+ bool IsAbstract() const {
+ return type_ == Type::kAbstract;
+ }
+
+ // Returns true if this is a method that must become a different method.
+ bool IsTranslation() const {
+ return type_ == Type::kTranslation;
+ }
+
+ // Get the translated version of this method.
+ ArtMethod* GetTranslation() const {
+ DCHECK(IsTranslation());
+ DCHECK(translation_ != nullptr);
+ return translation_;
+ }
+
+ private:
+ enum class Type {
+ kTranslation,
+ kConflict,
+ kAbstract,
+ };
+
+ MethodTranslation(Type type, ArtMethod* translation)
+ : translation_(translation), type_(type) {}
+
+ ArtMethod* const translation_;
+ const Type type_;
+ };
+
// Links the virtual methods for the given class and records any default methods that will need to
// be updated later.
//
@@ -737,9 +814,10 @@
// scan, we therefore store the vtable index's that might need to be
// updated with the method they will turn into.
// TODO This whole default_translations thing is very dirty. There should be a better way.
- bool LinkVirtualMethods(Thread* self,
- Handle<mirror::Class> klass,
- /*out*/std::unordered_map<size_t, ArtMethod*>* default_translations)
+ bool LinkVirtualMethods(
+ Thread* self,
+ Handle<mirror::Class> klass,
+ /*out*/std::unordered_map<size_t, MethodTranslation>* default_translations)
SHARED_REQUIRES(Locks::mutator_lock_);
// Sets up the interface lookup table (IFTable) in the correct order to allow searching for
@@ -749,6 +827,13 @@
Handle<mirror::ObjectArray<mirror::Class>> interfaces)
SHARED_REQUIRES(Locks::mutator_lock_);
+
+ enum class DefaultMethodSearchResult {
+ kDefaultFound,
+ kAbstractFound,
+ kDefaultConflict
+ };
+
// Find the default method implementation for 'interface_method' in 'klass', if one exists.
//
// Arguments:
@@ -756,31 +841,31 @@
// * target_method - The method we are trying to find a default implementation for.
// * klass - The class we are searching for a definition of target_method.
// * out_default_method - The pointer we will store the found default method to on success.
- // * icce_message - A string we will store an appropriate IncompatibleClassChangeError message
- // into in case of failure. Note we must do it this way since we do not know
- // whether we can allocate the exception object, which could cause us to go to
- // sleep.
//
// Return value:
- // * True - There were no conflicting method implementations found in the class while searching
- // for target_method. The default method implementation is stored into out_default_method
- // if it was found. Otherwise *out_default_method will be set to nullptr.
- // * False - Conflicting method implementations were found when searching for target_method. The
- // value of *out_default_method is undefined and *icce_message is a string that should
- // be used to create an IncompatibleClassChangeError as soon as possible.
- bool FindDefaultMethodImplementation(Thread* self,
- ArtMethod* target_method,
- Handle<mirror::Class> klass,
- /*out*/ArtMethod** out_default_method,
- /*out*/std::string* icce_message) const
+ // * kDefaultFound - There were no conflicting method implementations found in the class while
+ // searching for target_method. The default method implementation is stored into
+ // out_default_method.
+ // * kAbstractFound - There were no conflicting method implementations found in the class while
+ // searching for target_method but no default implementation was found either.
+ // out_default_method is set to null and the method should be considered not
+ // implemented.
+ // * kDefaultConflict - Conflicting method implementations were found when searching for
+ // target_method. The value of *out_default_method is null.
+ DefaultMethodSearchResult FindDefaultMethodImplementation(
+ Thread* self,
+ ArtMethod* target_method,
+ Handle<mirror::Class> klass,
+ /*out*/ArtMethod** out_default_method) const
SHARED_REQUIRES(Locks::mutator_lock_);
// Sets the imt entries and fixes up the vtable for the given class by linking all the interface
// methods. See LinkVirtualMethods for an explanation of what default_translations is.
- bool LinkInterfaceMethods(Thread* self,
- Handle<mirror::Class> klass,
- const std::unordered_map<size_t, ArtMethod*>& default_translations,
- ArtMethod** out_imt)
+ bool LinkInterfaceMethods(
+ Thread* self,
+ Handle<mirror::Class> klass,
+ const std::unordered_map<size_t, MethodTranslation>& default_translations,
+ ArtMethod** out_imt)
SHARED_REQUIRES(Locks::mutator_lock_);
bool LinkStaticFields(Thread* self, Handle<mirror::Class> klass, size_t* class_size)