Remove kAccMiranda and kAccDefaultConflicting.
Use a combination of other existing flags to mark miranda
and default conflicting methods.
Also change the value of identical flags kAccCriticalNative
and kAccPreCompiled to 0x00100000 to have the values of
kAccFastNative and kAccCriticalNative in consecutive bits.
The method access flag 0x00200000 is now free for future
use. The method access flag 0x00100000 is also potentially
usable for some methods the same way that the old
kAccMiranda flag was used despite sharing the same value
with both kAccCriticalNative and kAccPreCompiled.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 112676029
Change-Id: Ibfd20598f9b4af78e1e72f8331dc647d01dd8eb5
diff --git a/libdexfile/dex/modifiers.h b/libdexfile/dex/modifiers.h
index bdb3781..2c92fdd 100644
--- a/libdexfile/dex/modifiers.h
+++ b/libdexfile/dex/modifiers.h
@@ -58,35 +58,34 @@
// Used by a class to denote that this class and any objects with this as a
// declaring-class/super-class are to be considered obsolete, meaning they should not be used by.
static constexpr uint32_t kAccObsoleteObject = 0x00200000; // class (runtime)
-// This is set by the class linker during LinkInterfaceMethods. It is used by a method to represent
-// that it was copied from its declaring class into another class. All methods marked kAccMiranda
-// and kAccDefaultConflict will have this bit set. Any kAccDefault method contained in the methods_
-// array of a concrete class will also have this bit set.
-// We need copies of the original method because the method may end up in
-// different places in classes vtables, and the vtable index is set in ArtMethod.method_index.
-static constexpr uint32_t kAccCopied = 0x00100000; // method (runtime)
-static constexpr uint32_t kAccMiranda = 0x00200000; // method (runtime, not native)
+// This is set by the class linker during LinkInterfaceMethods. It is used by a method
+// to represent that it was copied from its declaring class into another class.
+// We need copies of the original method because the method may end up in different
+// places in classes vtables, and the vtable index is set in ArtMethod.method_index.
+//
+// Default methods copied to a sub-interface or a concrete class shall have this bit set.
+// Default conflict methods shall be marked as copied, abstract and default.
+// Miranda methods shall be marked as copied and abstract but not default.
+//
+// We do not have intrinsics for any default methods and therefore intrinsics are never
+// copied. We can therefore use a flag from the intrinsic flags range.
+static constexpr uint32_t kAccCopied = 0x01000000; // method (runtime)
static constexpr uint32_t kAccDefault = 0x00400000; // method (runtime)
// Native method flags are set when linking the methods based on the presence of the
// @dalvik.annotation.optimization.{Fast,Critical}Native annotations with build visibility.
// Reuse the values of kAccSkipAccessChecks and kAccMiranda which are not used for native methods.
static constexpr uint32_t kAccFastNative = 0x00080000; // method (runtime; native only)
-static constexpr uint32_t kAccCriticalNative = 0x00200000; // method (runtime; native only)
+static constexpr uint32_t kAccCriticalNative = 0x00100000; // method (runtime; native only)
// Set by the JIT when clearing profiling infos to denote that a method was previously warm.
static constexpr uint32_t kAccPreviouslyWarm = 0x00800000; // method (runtime)
-// This is set by the class linker during LinkInterfaceMethods. Prior to that point we do not know
-// if any particular method needs to be a default conflict. Used to figure out at runtime if
-// invoking this method will throw an exception.
-static constexpr uint32_t kAccDefaultConflict = 0x01000000; // method (runtime)
-
// Set by the verifier for a method we do not want the compiler to compile.
static constexpr uint32_t kAccCompileDontBother = 0x02000000; // method (runtime)
// Used in conjunction with kAccCompileDontBother to mark the method as pre
// compiled by the JIT compiler.
-static constexpr uint32_t kAccPreCompiled = 0x00200000; // method (runtime)
+static constexpr uint32_t kAccPreCompiled = 0x00100000; // method (runtime)
// Set by the verifier for a method that could not be verified to follow structured locking.
static constexpr uint32_t kAccMustCountLocks = 0x04000000; // method (runtime)
@@ -118,7 +117,7 @@
// Continuous sequence of bits used to hold the ordinal of an intrinsic method. Flags
// which overlap are not valid when kAccIntrinsic is set.
static constexpr uint32_t kAccIntrinsicBits = kAccHiddenapiBits |
- kAccSingleImplementation | kAccMustCountLocks | kAccCompileDontBother | kAccDefaultConflict |
+ kAccSingleImplementation | kAccMustCountLocks | kAccCompileDontBother | kAccCopied |
kAccPreviouslyWarm | kAccFastInterpreterToInterpreterInvoke;
// Valid (meaningful) bits for a field.
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 9edcfd0..bfcb455 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -149,8 +149,6 @@
void ArtMethod::ThrowInvocationTimeError() {
DCHECK(!IsInvokable());
- // NOTE: IsDefaultConflicting must be first since the actual method might or might not be abstract
- // due to the way we select it.
if (IsDefaultConflicting()) {
ThrowIncompatibleClassChangeErrorForMethodConflict(this);
} else {
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 31b81d4..ee2a47f 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -192,9 +192,11 @@
void SetNotIntrinsic() REQUIRES_SHARED(Locks::mutator_lock_);
bool IsCopied() const {
- static_assert((kAccCopied & (kAccIntrinsic | kAccIntrinsicBits)) == 0,
- "kAccCopied conflicts with intrinsic modifier");
- const bool copied = (GetAccessFlags() & kAccCopied) != 0;
+ // We do not have intrinsics for any default methods and therefore intrinsics are never copied.
+ // So we are using a flag from the intrinsic flags range and need to check `kAccIntrinsic` too.
+ static_assert((kAccCopied & kAccIntrinsicBits) != 0,
+ "kAccCopied deliberately overlaps intrinsic bits");
+ const bool copied = (GetAccessFlags() & (kAccIntrinsic | kAccCopied)) == kAccCopied;
// (IsMiranda() || IsDefaultConflicting()) implies copied
DCHECK(!(IsMiranda() || IsDefaultConflicting()) || copied)
<< "Miranda or default-conflict methods must always be copied.";
@@ -202,15 +204,31 @@
}
bool IsMiranda() const {
- // The kAccMiranda flag value is used with a different meaning for native methods and methods
- // marked kAccCompileDontBother, so we need to check these flags as well.
- return (GetAccessFlags() & (kAccNative | kAccMiranda | kAccCompileDontBother)) == kAccMiranda;
+ // Miranda methods are marked as copied and abstract but not default.
+ // We need to check the kAccIntrinsic too, see `IsCopied()`.
+ static constexpr uint32_t kMask = kAccIntrinsic | kAccCopied | kAccAbstract | kAccDefault;
+ static constexpr uint32_t kValue = kAccCopied | kAccAbstract;
+ return (GetAccessFlags() & kMask) == kValue;
+ }
+
+ // A default conflict method is a special sentinel method that stands for a conflict between
+ // multiple default methods. It cannot be invoked, throwing an IncompatibleClassChangeError
+ // if one attempts to do so.
+ bool IsDefaultConflicting() const {
+ // Default conflct methods are marked as copied, abstract and default.
+ // We need to check the kAccIntrinsic too, see `IsCopied()`.
+ static constexpr uint32_t kMask = kAccIntrinsic | kAccCopied | kAccAbstract | kAccDefault;
+ static constexpr uint32_t kValue = kAccCopied | kAccAbstract | kAccDefault;
+ return (GetAccessFlags() & kMask) == kValue;
}
// Returns true if invoking this method will not throw an AbstractMethodError or
// IncompatibleClassChangeError.
bool IsInvokable() const {
- return !IsAbstract() && !IsDefaultConflicting();
+ // Default conflicting methods are marked with `kAccAbstract` (as well as `kAccCopied`
+ // and `kAccDefault`) but they are not considered abstract, see `IsAbstract()`.
+ DCHECK_EQ((GetAccessFlags() & kAccAbstract) == 0, !IsDefaultConflicting() && !IsAbstract());
+ return (GetAccessFlags() & kAccAbstract) == 0;
}
bool IsPreCompiled() const {
@@ -253,16 +271,6 @@
AddAccessFlags(kAccCompileDontBother);
}
- // A default conflict method is a special sentinel method that stands for a conflict between
- // multiple default methods. It cannot be invoked, throwing an IncompatibleClassChangeError if one
- // attempts to do so.
- bool IsDefaultConflicting() const {
- if (IsIntrinsic()) {
- return false;
- }
- return (GetAccessFlags() & kAccDefaultConflict) != 0u;
- }
-
// This is set by the class linker.
bool IsDefault() const {
static_assert((kAccDefault & (kAccIntrinsic | kAccIntrinsicBits)) == 0,
@@ -301,7 +309,8 @@
}
bool IsAbstract() const {
- return (GetAccessFlags() & kAccAbstract) != 0;
+ // Default confliciting methods have `kAccAbstract` set but they are not actually abstract.
+ return (GetAccessFlags() & kAccAbstract) != 0 && !IsDefaultConflicting();
}
bool IsSynthetic() const {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index b98708e..11b5480 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -5121,9 +5121,9 @@
// Set class to be the concrete proxy class.
out->SetDeclaringClass(klass.Get());
- // Clear the abstract, default and conflict flags to ensure that defaults aren't picked in
+ // Clear the abstract and default flags to ensure that defaults aren't picked in
// preference to the invocation handler.
- const uint32_t kRemoveFlags = kAccAbstract | kAccDefault | kAccDefaultConflict;
+ const uint32_t kRemoveFlags = kAccAbstract | kAccDefault;
// Make the method final.
// Mark kAccCompileDontBother so that we don't take JIT samples for the method. b/62349349
const uint32_t kAddFlags = kAccFinal | kAccCompileDontBother;
@@ -7881,9 +7881,11 @@
ArtMethod* mir_method = miranda_methods_[i];
ArtMethod& new_method = *out;
new_method.CopyFrom(mir_method, pointer_size);
- new_method.SetAccessFlags(new_method.GetAccessFlags() | kAccMiranda | kAccCopied);
- DCHECK_NE(new_method.GetAccessFlags() & kAccAbstract, 0u)
- << "Miranda method should be abstract!";
+ uint32_t access_flags = new_method.GetAccessFlags();
+ DCHECK_EQ(access_flags & kAccIntrinsic, 0u) << "Miranda method should not be an intrinsic!";
+ DCHECK_EQ(access_flags & kAccDefault, 0u) << "Miranda method should not be a default method!";
+ DCHECK_NE(access_flags & kAccAbstract, 0u) << "Miranda method should be abstract!";
+ new_method.SetAccessFlags(access_flags | kAccCopied);
move_table_.emplace(mir_method, &new_method);
// Update the entry in the method array, as the array will be used for future lookups,
// where thread suspension is allowed.
@@ -7928,17 +7930,20 @@
ArtMethod& new_method = *out;
new_method.CopyFrom(conf_method, pointer_size);
// This is a type of default method (there are default method impls, just a conflict) so
- // mark this as a default, non-abstract method, since thats what it is. Also clear the
- // kAccSkipAccessChecks bit since this class hasn't been verified yet it shouldn't have
- // methods that are skipping access checks.
- // Also clear potential kAccSingleImplementation to avoid CHA trying to inline
- // the default method.
- DCHECK_EQ(new_method.GetAccessFlags() & kAccNative, 0u);
- constexpr uint32_t kSetFlags = kAccDefault | kAccDefaultConflict | kAccCopied;
- constexpr uint32_t kMaskFlags =
- ~(kAccAbstract | kAccSkipAccessChecks | kAccSingleImplementation);
- new_method.SetAccessFlags((new_method.GetAccessFlags() | kSetFlags) & kMaskFlags);
+ // mark this as a default. We use the `kAccAbstract` flag to distinguish it from invokable
+ // copied default method without using a separate access flag but the default conflicting
+ // method is technically not abstract and ArtMethod::IsAbstract() shall return false.
+ // Also clear the kAccSkipAccessChecks bit since this class hasn't been verified yet it
+ // shouldn't have methods that are skipping access checks. Also clear potential
+ // kAccSingleImplementation to avoid CHA trying to inline the default method.
+ uint32_t access_flags = new_method.GetAccessFlags();
+ DCHECK_EQ(access_flags & kAccNative, 0u);
+ DCHECK_EQ(access_flags & kAccIntrinsic, 0u);
+ constexpr uint32_t kSetFlags = kAccDefault | kAccAbstract | kAccCopied;
+ constexpr uint32_t kMaskFlags = ~(kAccSkipAccessChecks | kAccSingleImplementation);
+ new_method.SetAccessFlags((access_flags | kSetFlags) & kMaskFlags);
DCHECK(new_method.IsDefaultConflicting());
+ DCHECK(!new_method.IsAbstract());
// The actual method might or might not be marked abstract since we just copied it from a
// (possibly default) interface method. We need to set it entry point to be the bridge so
// that the compiler will not invoke the implementation of whatever method we copied from.
diff --git a/runtime/image.cc b/runtime/image.cc
index 61b8a0f..788c981 100644
--- a/runtime/image.cc
+++ b/runtime/image.cc
@@ -29,8 +29,8 @@
namespace art {
const uint8_t ImageHeader::kImageMagic[] = { 'a', 'r', 't', '\n' };
-// Last change: JNI transition without HandleScope.
-const uint8_t ImageHeader::kImageVersion[] = { '0', '9', '6', '\0' };
+// Last change: Remove kAccMiranda, kAccDefaultConflict.
+const uint8_t ImageHeader::kImageVersion[] = { '0', '9', '7', '\0' };
ImageHeader::ImageHeader(uint32_t image_reservation_size,
uint32_t component_count,
