Revert^2 "Support clinit for app image during compilation"
Add some spot fixes for app image class initialization and re-enable
test 660 features.
Bug: 70735003
Test: test-art-host
This reverts commit abadf024efdc632f663d7fb503cd277b3f65fca2.
Change-Id: Id16fd3ada3eb1bd57ea60c3cdc4a0cf9835950d7
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index b4cd5d7..c0b2f3e 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -2122,6 +2122,8 @@
const char* descriptor = dex_file.StringDataByIdx(class_type_id.descriptor_idx_);
ScopedObjectAccessUnchecked soa(Thread::Current());
StackHandleScope<3> hs(soa.Self());
+ ClassLinker* const class_linker = manager_->GetClassLinker();
+ Runtime* const runtime = Runtime::Current();
const bool is_boot_image = manager_->GetCompiler()->GetCompilerOptions().IsBootImage();
const bool is_app_image = manager_->GetCompiler()->GetCompilerOptions().IsAppImage();
@@ -2135,7 +2137,7 @@
if (klass->IsVerified()) {
// Attempt to initialize the class but bail if we either need to initialize the super-class
// or static fields.
- manager_->GetClassLinker()->EnsureInitialized(soa.Self(), klass, false, false);
+ class_linker->EnsureInitialized(soa.Self(), klass, false, false);
old_status = klass->GetStatus();
if (!klass->IsInitialized()) {
// We don't want non-trivial class initialization occurring on multiple threads due to
@@ -2154,7 +2156,7 @@
bool is_superclass_initialized = !is_app_image ? true :
InitializeDependencies(klass, class_loader, soa.Self());
if (!is_app_image || (is_app_image && is_superclass_initialized)) {
- manager_->GetClassLinker()->EnsureInitialized(soa.Self(), klass, false, true);
+ class_linker->EnsureInitialized(soa.Self(), klass, false, true);
// It's OK to clear the exception here since the compiler is supposed to be fault
// tolerant and will silently not initialize classes that have exceptions.
soa.Self()->ClearException();
@@ -2182,10 +2184,14 @@
CHECK(is_app_image);
// The boot image case doesn't need to recursively initialize the dependencies with
// special logic since the class linker already does this.
+ // Optimization will be disabled in debuggable build, because in debuggable mode we
+ // want the <clinit> behavior to be observable for the debugger, so we don't do the
+ // <clinit> at compile time.
can_init_static_fields =
ClassLinker::kAppImageMayContainStrings &&
!soa.Self()->IsExceptionPending() &&
is_superclass_initialized &&
+ !manager_->GetCompiler()->GetCompilerOptions().GetDebuggable() &&
(manager_->GetCompiler()->GetCompilerOptions().InitializeAppImageClasses() ||
NoClinitInDependency(klass, soa.Self(), &class_loader));
// TODO The checking for clinit can be removed since it's already
@@ -2200,12 +2206,23 @@
// exclusive access to the runtime and the transaction. To achieve this, we could use
// a ReaderWriterMutex but we're holding the mutator lock so we fail mutex sanity
// checks in Thread::AssertThreadSuspensionIsAllowable.
- Runtime* const runtime = Runtime::Current();
+
+ // Resolve and initialize the exception type before enabling the transaction in case
+ // the transaction aborts and cannot resolve the type.
+ // TransactionAbortError is not initialized ant not in boot image, needed only by
+ // compiler and will be pruned by ImageWriter.
+ Handle<mirror::Class> exception_class =
+ hs.NewHandle(class_linker->FindClass(Thread::Current(),
+ Transaction::kAbortExceptionSignature,
+ class_loader));
+ bool exception_initialized =
+ class_linker->EnsureInitialized(soa.Self(), exception_class, true, true);
+ DCHECK(exception_initialized);
+
// Run the class initializer in transaction mode.
runtime->EnterTransactionMode(is_app_image, klass.Get());
- bool success = manager_->GetClassLinker()->EnsureInitialized(soa.Self(), klass, true,
- true);
+ bool success = class_linker->EnsureInitialized(soa.Self(), klass, true, true);
// TODO we detach transaction from runtime to indicate we quit the transactional
// mode which prevents the GC from visiting objects modified during the transaction.
// Ensure GC is not run so don't access freed objects when aborting transaction.
@@ -2239,10 +2256,12 @@
}
}
- if (!success) {
+ if (!success && is_boot_image) {
// On failure, still intern strings of static fields and seen in <clinit>, as these
// will be created in the zygote. This is separated from the transaction code just
// above as we will allocate strings, so must be allowed to suspend.
+ // We only need to intern strings for boot image because classes that failed to be
+ // initialized will not appear in app image.
if (&klass->GetDexFile() == manager_->GetDexFile()) {
InternStrings(klass, class_loader);
} else {
@@ -2258,8 +2277,7 @@
// If the class still isn't initialized, at least try some checks that initialization
// would do so they can be skipped at runtime.
- if (!klass->IsInitialized() &&
- manager_->GetClassLinker()->ValidateSuperClassDescriptors(klass)) {
+ if (!klass->IsInitialized() && class_linker->ValidateSuperClassDescriptors(klass)) {
old_status = ClassStatus::kSuperclassValidated;
} else {
soa.Self()->ClearException();
diff --git a/runtime/aot_class_linker.cc b/runtime/aot_class_linker.cc
index c9ca4c9..c151306 100644
--- a/runtime/aot_class_linker.cc
+++ b/runtime/aot_class_linker.cc
@@ -31,6 +31,16 @@
AotClassLinker::~AotClassLinker() {}
+bool AotClassLinker::CanAllocClass() {
+ // AllocClass doesn't work under transaction, so we abort.
+ if (Runtime::Current()->IsActiveTransaction()) {
+ Runtime::Current()->AbortTransactionAndThrowAbortError(
+ Thread::Current(), "Can't resolve type within transaction.");
+ return false;
+ }
+ return ClassLinker::CanAllocClass();
+}
+
// Wrap the original InitializeClass with creation of transaction when in strict mode.
bool AotClassLinker::InitializeClass(Thread* self,
Handle<mirror::Class> klass,
@@ -44,6 +54,13 @@
return ClassLinker::InitializeClass(self, klass, can_init_statics, can_init_parents);
}
+ // When in strict_mode, don't initialize a class if it belongs to boot but not initialized.
+ if (strict_mode_ && klass->IsBootStrapClassLoaded()) {
+ runtime->AbortTransactionAndThrowAbortError(self, "Can't resolve "
+ + klass->PrettyTypeOf() + " because it is an uninitialized boot class.");
+ return false;
+ }
+
// Don't initialize klass if it's superclass is not initialized, because superclass might abort
// the transaction and rolled back after klass's change is commited.
if (strict_mode_ && !klass->IsInterface() && klass->HasSuperClass()) {
@@ -64,9 +81,8 @@
// Exit Transaction if success.
runtime->ExitTransactionMode();
} else {
- // If not successfully initialized, the last transaction must abort. Don't rollback
- // immediately, leave the cleanup to compiler driver which needs abort message and exception.
- DCHECK(runtime->IsTransactionAborted());
+ // If not successfully initialized, don't rollback immediately, leave the cleanup to compiler
+ // driver which needs abort message and exception.
DCHECK(self->IsExceptionPending());
}
}
diff --git a/runtime/aot_class_linker.h b/runtime/aot_class_linker.h
index 6a8133e..74da33d 100644
--- a/runtime/aot_class_linker.h
+++ b/runtime/aot_class_linker.h
@@ -37,6 +37,13 @@
override
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Override AllocClass because aot compiler will need to perform a transaction check to determine
+ // can we allocate class from heap.
+ bool CanAllocClass()
+ override
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Roles::uninterruptible_);
+
bool InitializeClass(Thread *self,
Handle<mirror::Class> klass,
bool can_run_clinit,
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index c06e5e8..2bbcbb7 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3152,7 +3152,11 @@
// Interface object should get the right size here. Regular class will
// figure out the right size later and be replaced with one of the right
// size when the class becomes resolved.
- klass.Assign(AllocClass(self, SizeOfClassWithoutEmbeddedTables(dex_file, dex_class_def)));
+ if (CanAllocClass()) {
+ klass.Assign(AllocClass(self, SizeOfClassWithoutEmbeddedTables(dex_file, dex_class_def)));
+ } else {
+ return nullptr;
+ }
}
if (UNLIKELY(klass == nullptr)) {
self->AssertPendingOOMException();
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 3605ffb..8269aaa 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -758,6 +758,10 @@
std::string* error_msg)
REQUIRES_SHARED(Locks::mutator_lock_);
+ virtual bool CanAllocClass() REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!Locks::dex_lock_) {
+ return true;
+ }
+
private:
class LinkInterfaceMethodsHelper;
diff --git a/test/660-clinit/expected.txt b/test/660-clinit/expected.txt
index 9eb4941..ee1b479 100644
--- a/test/660-clinit/expected.txt
+++ b/test/660-clinit/expected.txt
@@ -1,4 +1,5 @@
JNI_OnLoad called
+hello world
A.a: 5
A.a: 10
B.b: 10
diff --git a/test/660-clinit/profile b/test/660-clinit/profile
index 0239f22..9eb4924 100644
--- a/test/660-clinit/profile
+++ b/test/660-clinit/profile
@@ -4,7 +4,10 @@
LA;
LB;
LC;
+LE;
LG;
LGs;
LObjectRef;
-
+LInvokeStatic;
+LClinitE;
+LPrint;
diff --git a/test/660-clinit/run b/test/660-clinit/run
index d24ef42..a0e79ee 100644
--- a/test/660-clinit/run
+++ b/test/660-clinit/run
@@ -14,4 +14,4 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-exec ${RUN} $@ --profile
+exec ${RUN} $@ --profile -Xcompiler-option --initialize-app-image-classes=true
diff --git a/test/660-clinit/src/Main.java b/test/660-clinit/src/Main.java
index f9b068e..5fb5fe5 100644
--- a/test/660-clinit/src/Main.java
+++ b/test/660-clinit/src/Main.java
@@ -24,19 +24,28 @@
if (!checkAppImageLoaded()) {
System.out.println("AppImage not loaded.");
}
+ if (!checkAppImageContains(ClInit.class)) {
+ System.out.println("ClInit class is not in app image!");
+ }
- expectNotPreInit(Day.class);
- expectNotPreInit(ClInit.class); // should pass
- expectNotPreInit(A.class); // should pass
- expectNotPreInit(B.class); // should fail
- expectNotPreInit(C.class); // should fail
- expectNotPreInit(G.class); // should fail
- expectNotPreInit(Gs.class); // should fail
- expectNotPreInit(Gss.class); // should fail
+ expectPreInit(ClInit.class);
+ expectPreInit(A.class);
+ expectPreInit(E.class);
+ expectNotPreInit(B.class);
+ expectNotPreInit(C.class);
+ expectNotPreInit(G.class);
+ expectNotPreInit(Gs.class);
+ expectNotPreInit(Gss.class);
+ expectPreInit(InvokeStatic.class);
+ expectNotPreInit(ClinitE.class);
expectNotPreInit(Add.class);
expectNotPreInit(Mul.class);
expectNotPreInit(ObjectRef.class);
+ expectNotPreInit(Print.class);
+
+ Print p = new Print();
+ Gs gs = new Gs();
A x = new A();
System.out.println("A.a: " + A.a);
@@ -62,6 +71,10 @@
System.out.println("a != 101");
}
+ try {
+ ClinitE e = new ClinitE();
+ } catch (Error err) { }
+
return;
}
@@ -154,6 +167,13 @@
}
}
+class E {
+ public static final int e;
+ static {
+ e = 100;
+ }
+}
+
class G {
static G g;
static int i;
@@ -182,9 +202,35 @@
}
}
+// test of INVOKE_STATIC instruction
+class InvokeStatic {
+ static int a;
+ static int b;
+ static {
+ a = Add.exec(10, 20);
+ b = Mul.exec(10, 20);
+ }
+}
+
// non-image
class Mul {
static int exec(int a, int b) {
return a * b;
}
}
+
+class ClinitE {
+ static {
+ if (Math.sin(3) < 0.5) {
+ // throw anyway, can't initialized
+ throw new ExceptionInInitializerError("Can't initialize this class!");
+ }
+ }
+}
+
+// fail because JNI
+class Print {
+ static {
+ System.out.println("hello world");
+ }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index ad0cbe8..cb0fc0f 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -738,9 +738,10 @@
},
{
"tests": "660-clinit",
- "variant": "no-image | no-prebuild | jvmti-stress | redefine-stress",
+ "variant": "no-image | no-prebuild | jvmti-stress | redefine-stress | interp-ac",
"description": ["Tests <clinit> for app images, which --no-image, --no-prebuild, ",
- "and --redefine-stress do not create"]
+ "and --redefine-stress do not create. Also avoid for ",
+ "verify-soft-fail (interp-ac) since it prevents initialization."]
},
{
"tests": ["961-default-iface-resolution-gen",