Fix class status check in `InitializeClassVisitor`.

And address other late comments on
    https://android-review.googlesource.com/2163021 .

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Test: run-gtests.sh
Test: testrunner.py --target --optimizing
Bug: 162110941
Change-Id: I9eb49179633f94044b2060231babd156787bbd14

10 files changed, 41 insertions, 36 deletions

diff --git a/compiler/driver/compiler_options.cc b/compiler/driver/compiler_options.cc
index 87dbfee412..8b415c641d 100644
--- a/compiler/driver/compiler_options.cc
+++ b/compiler/driver/compiler_options.cc
@@ -135,8 +135,8 @@ bool CompilerOptions::IsImageClass(const char* descriptor) const {
   return image_classes_.find(std::string_view(descriptor)) != image_classes_.end();
 }
 
-bool CompilerOptions::IsPreloadedClass(const char* pretty_descriptor) const {
-  return preloaded_classes_.find(std::string_view(pretty_descriptor)) != preloaded_classes_.end();
+bool CompilerOptions::IsPreloadedClass(std::string_view pretty_descriptor) const {
+  return preloaded_classes_.find(pretty_descriptor) != preloaded_classes_.end();
 }
 
 bool CompilerOptions::ShouldCompileWithClinitCheck(ArtMethod* method) const {
diff --git a/compiler/driver/compiler_options.h b/compiler/driver/compiler_options.h
index abdb01b372..36ecf88199 100644
--- a/compiler/driver/compiler_options.h
+++ b/compiler/driver/compiler_options.h
@@ -292,7 +292,7 @@ class CompilerOptions final {
 
   // Returns whether the given `pretty_descriptor` is in the list of preloaded
   // classes. `pretty_descriptor` should be the result of calling `PrettyDescriptor`.
-  EXPORT bool IsPreloadedClass(const char* pretty_descriptor) const;
+  EXPORT bool IsPreloadedClass(std::string_view pretty_descriptor) const;
 
   bool ParseCompilerOptions(const std::vector<std::string>& options,
                             bool ignore_unrecognized,
diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h
index 3ac4bd7dcc..aec7b45a1a 100644
--- a/compiler/optimizing/code_generator.h
+++ b/compiler/optimizing/code_generator.h
@@ -59,15 +59,15 @@ static int32_t constexpr kPrimIntMax = 0x7fffffff;
 // Maximum value for a primitive long.
 static int64_t constexpr kPrimLongMax = INT64_C(0x7fffffffffffffff);
 
-constexpr size_t status_lsb_position = SubtypeCheckBits::BitStructSizeOf();
-constexpr size_t status_byte_offset =
-    mirror::Class::StatusOffset().SizeValue() + (status_lsb_position / kBitsPerByte);
-constexpr uint32_t shifted_visibly_initialized_value =
-    enum_cast<uint32_t>(ClassStatus::kVisiblyInitialized) << (status_lsb_position % kBitsPerByte);
-constexpr uint32_t shifted_initializing_value =
-    enum_cast<uint32_t>(ClassStatus::kInitializing) << (status_lsb_position % kBitsPerByte);
-constexpr uint32_t shifted_initialized_value =
-    enum_cast<uint32_t>(ClassStatus::kInitialized) << (status_lsb_position % kBitsPerByte);
+constexpr size_t kClassStatusLsbPosition = SubtypeCheckBits::BitStructSizeOf();
+constexpr size_t kClassStatusByteOffset =
+    mirror::Class::StatusOffset().SizeValue() + (kClassStatusLsbPosition / kBitsPerByte);
+constexpr uint32_t kShiftedVisiblyInitializedValue = enum_cast<uint32_t>(
+    ClassStatus::kVisiblyInitialized) << (kClassStatusLsbPosition % kBitsPerByte);
+constexpr uint32_t kShiftedInitializingValue =
+    enum_cast<uint32_t>(ClassStatus::kInitializing) << (kClassStatusLsbPosition % kBitsPerByte);
+constexpr uint32_t kShiftedInitializedValue =
+    enum_cast<uint32_t>(ClassStatus::kInitialized) << (kClassStatusLsbPosition % kBitsPerByte);
 
 class Assembler;
 class CodeGenerationData;
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 988809ee48..226e5343e2 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -1350,18 +1350,18 @@ void CodeGeneratorARM64::GenerateFrameEntry() {
     // We don't emit a read barrier here to save on code size. We rely on the
     // resolution trampoline to do a suspend check before re-entering this code.
     __ Ldr(temp1, MemOperand(kArtMethodRegister, ArtMethod::DeclaringClassOffset().Int32Value()));
-    __ Ldrb(temp2, HeapOperand(temp1, status_byte_offset));
-    __ Cmp(temp2, shifted_visibly_initialized_value);
+    __ Ldrb(temp2, HeapOperand(temp1, kClassStatusByteOffset));
+    __ Cmp(temp2, kShiftedVisiblyInitializedValue);
     __ B(hs, &frame_entry_label_);
 
     // Check if we're initialized and jump to code that does a memory barrier if
     // so.
-    __ Cmp(temp2, shifted_initialized_value);
+    __ Cmp(temp2, kShiftedInitializedValue);
     __ B(hs, &memory_barrier);
 
     // Check if we're initializing and the thread initializing is the one
     // executing the code.
-    __ Cmp(temp2, shifted_initializing_value);
+    __ Cmp(temp2, kShiftedInitializingValue);
     __ B(lo, &resolution);
 
     __ Ldr(temp1, HeapOperand(temp1, mirror::Class::ClinitThreadIdOffset().Int32Value()));
@@ -2080,8 +2080,8 @@ void InstructionCodeGeneratorARM64::GenerateClassInitializationCheck(SlowPathCod
   // size, load only the high byte of the field and compare with 0xf0.
   // Note: The same code size could be achieved with LDR+MNV(asr #24)+CBNZ but benchmarks
   // show that this pattern is slower (tested on little cores).
-  __ Ldrb(temp, HeapOperand(class_reg, status_byte_offset));
-  __ Cmp(temp, shifted_visibly_initialized_value);
+  __ Ldrb(temp, HeapOperand(class_reg, kClassStatusByteOffset));
+  __ Cmp(temp, kShiftedVisiblyInitializedValue);
   __ B(lo, slow_path->GetEntryLabel());
   __ Bind(slow_path->GetExitLabel());
 }
diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc
index 7c1a9b21f2..979db6ed75 100644
--- a/compiler/optimizing/code_generator_arm_vixl.cc
+++ b/compiler/optimizing/code_generator_arm_vixl.cc
@@ -2354,18 +2354,18 @@ void CodeGeneratorARMVIXL::GenerateFrameEntry() {
     // We don't emit a read barrier here to save on code size. We rely on the
     // resolution trampoline to do a suspend check before re-entering this code.
     __ Ldr(temp1, MemOperand(kMethodRegister, ArtMethod::DeclaringClassOffset().Int32Value()));
-    __ Ldrb(temp2, MemOperand(temp1, status_byte_offset));
-    __ Cmp(temp2, shifted_visibly_initialized_value);
+    __ Ldrb(temp2, MemOperand(temp1, kClassStatusByteOffset));
+    __ Cmp(temp2, kShiftedVisiblyInitializedValue);
     __ B(cs, &frame_entry_label_);
 
     // Check if we're initialized and jump to code that does a memory barrier if
     // so.
-    __ Cmp(temp2, shifted_initialized_value);
+    __ Cmp(temp2, kShiftedInitializedValue);
     __ B(cs, &memory_barrier);
 
     // Check if we're initializing and the thread initializing is the one
     // executing the code.
-    __ Cmp(temp2, shifted_initializing_value);
+    __ Cmp(temp2, kShiftedInitializingValue);
     __ B(lo, &resolution);
 
     __ Ldr(temp1, MemOperand(temp1, mirror::Class::ClinitThreadIdOffset().Int32Value()));
@@ -7862,8 +7862,8 @@ void InstructionCodeGeneratorARMVIXL::GenerateClassInitializationCheck(
     LoadClassSlowPathARMVIXL* slow_path, vixl32::Register class_reg) {
   UseScratchRegisterScope temps(GetVIXLAssembler());
   vixl32::Register temp = temps.Acquire();
-  __ Ldrb(temp, MemOperand(class_reg, status_byte_offset));
-  __ Cmp(temp, shifted_visibly_initialized_value);
+  __ Ldrb(temp, MemOperand(class_reg, kClassStatusByteOffset));
+  __ Cmp(temp, kShiftedVisiblyInitializedValue);
   __ B(lo, slow_path->GetEntryLabel());
   __ Bind(slow_path->GetExitLabel());
 }
diff --git a/compiler/optimizing/code_generator_riscv64.cc b/compiler/optimizing/code_generator_riscv64.cc
index 9b499a08a2..75d5db83a0 100644
--- a/compiler/optimizing/code_generator_riscv64.cc
+++ b/compiler/optimizing/code_generator_riscv64.cc
@@ -137,8 +137,8 @@ static RegisterSet OneRegInReferenceOutSaveEverythingCallerSaves() {
 template <ClassStatus kStatus>
 static constexpr int64_t ShiftedSignExtendedClassStatusValue() {
   // This is used only for status values that have the highest bit set.
-  static_assert(CLZ(enum_cast<uint32_t>(kStatus)) == status_lsb_position);
-  constexpr uint32_t kShiftedStatusValue = enum_cast<uint32_t>(kStatus) << status_lsb_position;
+  static_assert(CLZ(enum_cast<uint32_t>(kStatus)) == kClassStatusLsbPosition);
+  constexpr uint32_t kShiftedStatusValue = enum_cast<uint32_t>(kStatus) << kClassStatusLsbPosition;
   static_assert(kShiftedStatusValue >= 0x80000000u);
   return static_cast<int64_t>(kShiftedStatusValue) - (INT64_C(1) << 32);
 }
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 4329e40efc..2f6dde3df7 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -1390,17 +1390,18 @@ void CodeGeneratorX86::GenerateFrameEntry() {
     NearLabel continue_execution, resolution;
     // We'll use EBP as temporary.
     __ pushl(EBP);
+    __ cfi().AdjustCFAOffset(4);
     // Check if we're visibly initialized.
 
     // We don't emit a read barrier here to save on code size. We rely on the
     // resolution trampoline to do a suspend check before re-entering this code.
     __ movl(EBP, Address(kMethodRegisterArgument, ArtMethod::DeclaringClassOffset().Int32Value()));
-    __ cmpb(Address(EBP,  status_byte_offset), Immediate(shifted_visibly_initialized_value));
+    __ cmpb(Address(EBP, kClassStatusByteOffset), Immediate(kShiftedVisiblyInitializedValue));
     __ j(kAboveEqual, &continue_execution);
 
     // Check if we're initializing and the thread initializing is the one
     // executing the code.
-    __ cmpb(Address(EBP,  status_byte_offset), Immediate(shifted_initializing_value));
+    __ cmpb(Address(EBP, kClassStatusByteOffset), Immediate(kShiftedInitializingValue));
     __ j(kBelow, &resolution);
 
     __ movl(EBP, Address(EBP, mirror::Class::ClinitThreadIdOffset().Int32Value()));
@@ -1409,13 +1410,16 @@ void CodeGeneratorX86::GenerateFrameEntry() {
     __ Bind(&resolution);
 
     __ popl(EBP);
+    __ cfi().AdjustCFAOffset(-4);
     // Jump to the resolution stub.
     ThreadOffset32 entrypoint_offset =
         GetThreadOffset<kX86PointerSize>(kQuickQuickResolutionTrampoline);
     __ fs()->jmp(Address::Absolute(entrypoint_offset));
 
     __ Bind(&continue_execution);
+    __ cfi().AdjustCFAOffset(4);  // Undo the `-4` adjustment above. We get here with EBP pushed.
     __ popl(EBP);
+    __ cfi().AdjustCFAOffset(-4);
   }
 
   __ Bind(&frame_entry_label_);
@@ -7510,7 +7514,7 @@ void InstructionCodeGeneratorX86::VisitClinitCheck(HClinitCheck* check) {
 
 void InstructionCodeGeneratorX86::GenerateClassInitializationCheck(
     SlowPathCode* slow_path, Register class_reg) {
-  __ cmpb(Address(class_reg,  status_byte_offset), Immediate(shifted_visibly_initialized_value));
+  __ cmpb(Address(class_reg, kClassStatusByteOffset), Immediate(kShiftedVisiblyInitializedValue));
   __ j(kBelow, slow_path->GetEntryLabel());
   __ Bind(slow_path->GetExitLabel());
 }
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index 5b4f1b8b25..5c9ee8fea4 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -1839,13 +1839,14 @@ void CodeGeneratorX86_64::GenerateFrameEntry() {
     __ movl(CpuRegister(TMP),
             Address(CpuRegister(kMethodRegisterArgument),
                     ArtMethod::DeclaringClassOffset().Int32Value()));
-    __ cmpb(Address(CpuRegister(TMP),  status_byte_offset),
-            Immediate(shifted_visibly_initialized_value));
+    __ cmpb(Address(CpuRegister(TMP), kClassStatusByteOffset),
+            Immediate(kShiftedVisiblyInitializedValue));
     __ j(kAboveEqual, &frame_entry_label_);
 
     // Check if we're initializing and the thread initializing is the one
     // executing the code.
-    __ cmpb(Address(CpuRegister(TMP),  status_byte_offset), Immediate(shifted_initializing_value));
+    __ cmpb(Address(CpuRegister(TMP), kClassStatusByteOffset),
+            Immediate(kShiftedInitializingValue));
     __ j(kBelow, &resolution);
 
     __ movl(CpuRegister(TMP),
@@ -6596,7 +6597,7 @@ void ParallelMoveResolverX86_64::RestoreScratch(int reg) {
 
 void InstructionCodeGeneratorX86_64::GenerateClassInitializationCheck(
     SlowPathCode* slow_path, CpuRegister class_reg) {
-  __ cmpb(Address(class_reg,  status_byte_offset), Immediate(shifted_visibly_initialized_value));
+  __ cmpb(Address(class_reg, kClassStatusByteOffset), Immediate(kShiftedVisiblyInitializedValue));
   __ j(kBelow, slow_path->GetEntryLabel());
   __ Bind(slow_path->GetExitLabel());
 }
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index 8ba590c7da..c696191491 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -2419,9 +2419,9 @@ class InitializeClassVisitor : public CompilationVisitor {
       self->GetJniEnv()->AssertLocalsEmpty();
     }
 
-    if (!klass->IsVisiblyInitialized() &&
+    if (!klass->IsInitialized() &&
         (is_boot_image || is_boot_image_extension) &&
-        !compiler_options.IsPreloadedClass(PrettyDescriptor(descriptor).c_str())) {
+        !compiler_options.IsPreloadedClass(PrettyDescriptor(descriptor))) {
       klass->SetInBootImageAndNotInPreloadedClasses();
     }
 
diff --git a/libdexfile/dex/modifiers.h b/libdexfile/dex/modifiers.h
index 1307e0eae5..e4fc74a3e7 100644
--- a/libdexfile/dex/modifiers.h
+++ b/libdexfile/dex/modifiers.h
@@ -57,7 +57,7 @@ static constexpr uint32_t kAccSkipHiddenapiChecks =   0x00100000;  // class (run
 // declaring-class/super-class are to be considered obsolete, meaning they should not be used by.
 static constexpr uint32_t kAccObsoleteObject =        0x00200000;  // class (runtime)
 // Set during boot image compilation to indicate that the class is
-// not initialized at compile tile and not in the list of preloaded classes.
+// not initialized at compile time and not in the list of preloaded classes.
 static constexpr uint32_t kAccInBootImageAndNotInPreloadedClasses = 0x00400000;  // 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.