diff options
author | 2018-07-20 16:43:56 +0100 | |
---|---|---|
committer | 2018-08-02 17:41:21 +0100 | |
commit | a9f303c089aa2b2fc82d97201352945678ef54ae (patch) | |
tree | 0df0eb5294a3ee72aea8ca670762c02ca9ffa8dd | |
parent | 1bfd891d06e276d602b4a6ccf1a9f70967195218 (diff) |
Rewrite Class init entrypoint to take a Class arg.
Fixes invalid type index being passed to the entrypoint for
class init check across dex files when the target type does
not have a TypeId in the compilation unit's DexFile.
The size of the aosp_taimen-userdebug prebuilts:
- before:
arm/boot*.oat: 16782748
arm64/boot*.oat: 19764400
oat/arm64/services.odex: 20162432
- after:
arm/boot*.oat: 16811692 (+28.3KiB, +0.17%)
arm64/boot*.oat: 19801032 (+35.8KiB, +0.19%)
oat/arm64/services.odex: 20232208 (+68.1KiB, +0.35%)
This increase comes from doing two runtime calls instead of
one for HLoadClass/kBssEntry that MustGenerateClinitCheck().
Test: Additional test in 476-clinit-inline-static-invoke
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing --jit
Test: Pixel 2 XL boots.
Test: testrunner.py --target --optimizing --jit
Test: testrunner.py --jvm
Bug: 111433619
Change-Id: I2fccd6944480ab4dac514f60d38e72c1014ae7b2
21 files changed, 249 insertions, 186 deletions
diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index b0a05da0b1..f6a104b7c2 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -737,14 +737,12 @@ void CodeGenerator::CreateLoadClassRuntimeCallLocationSummary(HLoadClass* cls, void CodeGenerator::GenerateLoadClassRuntimeCall(HLoadClass* cls) { DCHECK_EQ(cls->GetLoadKind(), HLoadClass::LoadKind::kRuntimeCall); + DCHECK(!cls->MustGenerateClinitCheck()); LocationSummary* locations = cls->GetLocations(); MoveConstant(locations->GetTemp(0), cls->GetTypeIndex().index_); if (cls->NeedsAccessCheck()) { CheckEntrypointTypes<kQuickInitializeTypeAndVerifyAccess, void*, uint32_t>(); InvokeRuntime(kQuickInitializeTypeAndVerifyAccess, cls, cls->GetDexPc()); - } else if (cls->MustGenerateClinitCheck()) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - InvokeRuntime(kQuickInitializeStaticStorage, cls, cls->GetDexPc()); } else { CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); InvokeRuntime(kQuickInitializeType, cls, cls->GetDexPc()); diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 8a5cbcade0..5659ebf62c 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -307,35 +307,41 @@ class DivZeroCheckSlowPathARM64 : public SlowPathCodeARM64 { class LoadClassSlowPathARM64 : public SlowPathCodeARM64 { public: - LoadClassSlowPathARM64(HLoadClass* cls, - HInstruction* at, - uint32_t dex_pc, - bool do_clinit) - : SlowPathCodeARM64(at), - cls_(cls), - dex_pc_(dex_pc), - do_clinit_(do_clinit) { + LoadClassSlowPathARM64(HLoadClass* cls, HInstruction* at) + : SlowPathCodeARM64(at), cls_(cls) { DCHECK(at->IsLoadClass() || at->IsClinitCheck()); + DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); } void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { LocationSummary* locations = instruction_->GetLocations(); Location out = locations->Out(); - CodeGeneratorARM64* arm64_codegen = down_cast<CodeGeneratorARM64*>(codegen); + const uint32_t dex_pc = instruction_->GetDexPc(); + bool must_resolve_type = instruction_->IsLoadClass() && cls_->MustResolveTypeOnSlowPath(); + bool must_do_clinit = instruction_->IsClinitCheck() || cls_->MustGenerateClinitCheck(); + CodeGeneratorARM64* arm64_codegen = down_cast<CodeGeneratorARM64*>(codegen); __ Bind(GetEntryLabel()); SaveLiveRegisters(codegen, locations); InvokeRuntimeCallingConvention calling_convention; - dex::TypeIndex type_index = cls_->GetTypeIndex(); - __ Mov(calling_convention.GetRegisterAt(0).W(), type_index.index_); - QuickEntrypointEnum entrypoint = do_clinit_ ? kQuickInitializeStaticStorage - : kQuickInitializeType; - arm64_codegen->InvokeRuntime(entrypoint, instruction_, dex_pc_, this); - if (do_clinit_) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - } else { + if (must_resolve_type) { + DCHECK(IsSameDexFile(cls_->GetDexFile(), arm64_codegen->GetGraph()->GetDexFile())); + dex::TypeIndex type_index = cls_->GetTypeIndex(); + __ Mov(calling_convention.GetRegisterAt(0).W(), type_index.index_); + arm64_codegen->InvokeRuntime(kQuickInitializeType, instruction_, dex_pc, this); CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); + // If we also must_do_clinit, the resolved type is now in the correct register. + } else { + DCHECK(must_do_clinit); + Location source = instruction_->IsLoadClass() ? out : locations->InAt(0); + arm64_codegen->MoveLocation(LocationFrom(calling_convention.GetRegisterAt(0)), + source, + cls_->GetType()); + } + if (must_do_clinit) { + arm64_codegen->InvokeRuntime(kQuickInitializeStaticStorage, instruction_, dex_pc, this); + CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, mirror::Class*>(); } // Move the class to the desired location. @@ -354,12 +360,6 @@ class LoadClassSlowPathARM64 : public SlowPathCodeARM64 { // The class this slow path will load. HLoadClass* const cls_; - // The dex PC of `at_`. - const uint32_t dex_pc_; - - // Whether to initialize the class. - const bool do_clinit_; - DISALLOW_COPY_AND_ASSIGN(LoadClassSlowPathARM64); }; @@ -3194,8 +3194,8 @@ void LocationsBuilderARM64::VisitClinitCheck(HClinitCheck* check) { void InstructionCodeGeneratorARM64::VisitClinitCheck(HClinitCheck* check) { // We assume the class is not null. - SlowPathCodeARM64* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathARM64( - check->GetLoadClass(), check, check->GetDexPc(), true); + SlowPathCodeARM64* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathARM64(check->GetLoadClass(), check); codegen_->AddSlowPath(slow_path); GenerateClassInitializationCheck(slow_path, InputRegisterAt(check, 0)); } @@ -5192,8 +5192,8 @@ void InstructionCodeGeneratorARM64::VisitLoadClass(HLoadClass* cls) NO_THREAD_SA bool do_clinit = cls->MustGenerateClinitCheck(); if (generate_null_check || do_clinit) { DCHECK(cls->CanCallRuntime()); - SlowPathCodeARM64* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathARM64( - cls, cls, cls->GetDexPc(), do_clinit); + SlowPathCodeARM64* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathARM64(cls, cls); codegen_->AddSlowPath(slow_path); if (generate_null_check) { __ Cbz(out, slow_path->GetEntryLabel()); diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index 836a989f22..c274dfcc7a 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -501,29 +501,39 @@ class BoundsCheckSlowPathARMVIXL : public SlowPathCodeARMVIXL { class LoadClassSlowPathARMVIXL : public SlowPathCodeARMVIXL { public: - LoadClassSlowPathARMVIXL(HLoadClass* cls, HInstruction* at, uint32_t dex_pc, bool do_clinit) - : SlowPathCodeARMVIXL(at), cls_(cls), dex_pc_(dex_pc), do_clinit_(do_clinit) { + LoadClassSlowPathARMVIXL(HLoadClass* cls, HInstruction* at) + : SlowPathCodeARMVIXL(at), cls_(cls) { DCHECK(at->IsLoadClass() || at->IsClinitCheck()); + DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); } void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { LocationSummary* locations = instruction_->GetLocations(); Location out = locations->Out(); + const uint32_t dex_pc = instruction_->GetDexPc(); + bool must_resolve_type = instruction_->IsLoadClass() && cls_->MustResolveTypeOnSlowPath(); + bool must_do_clinit = instruction_->IsClinitCheck() || cls_->MustGenerateClinitCheck(); CodeGeneratorARMVIXL* arm_codegen = down_cast<CodeGeneratorARMVIXL*>(codegen); __ Bind(GetEntryLabel()); SaveLiveRegisters(codegen, locations); InvokeRuntimeCallingConventionARMVIXL calling_convention; - dex::TypeIndex type_index = cls_->GetTypeIndex(); - __ Mov(calling_convention.GetRegisterAt(0), type_index.index_); - QuickEntrypointEnum entrypoint = do_clinit_ ? kQuickInitializeStaticStorage - : kQuickInitializeType; - arm_codegen->InvokeRuntime(entrypoint, instruction_, dex_pc_, this); - if (do_clinit_) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - } else { + if (must_resolve_type) { + DCHECK(IsSameDexFile(cls_->GetDexFile(), arm_codegen->GetGraph()->GetDexFile())); + dex::TypeIndex type_index = cls_->GetTypeIndex(); + __ Mov(calling_convention.GetRegisterAt(0), type_index.index_); + arm_codegen->InvokeRuntime(kQuickInitializeType, instruction_, dex_pc, this); CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); + // If we also must_do_clinit, the resolved type is now in the correct register. + } else { + DCHECK(must_do_clinit); + Location source = instruction_->IsLoadClass() ? out : locations->InAt(0); + arm_codegen->Move32(LocationFrom(calling_convention.GetRegisterAt(0)), source); + } + if (must_do_clinit) { + arm_codegen->InvokeRuntime(kQuickInitializeStaticStorage, instruction_, dex_pc, this); + CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, mirror::Class*>(); } // Move the class to the desired location. @@ -541,12 +551,6 @@ class LoadClassSlowPathARMVIXL : public SlowPathCodeARMVIXL { // The class this slow path will load. HLoadClass* const cls_; - // The dex PC of `at_`. - const uint32_t dex_pc_; - - // Whether to initialize the class. - const bool do_clinit_; - DISALLOW_COPY_AND_ASSIGN(LoadClassSlowPathARMVIXL); }; @@ -7508,8 +7512,7 @@ void InstructionCodeGeneratorARMVIXL::VisitLoadClass(HLoadClass* cls) NO_THREAD_ if (generate_null_check || cls->MustGenerateClinitCheck()) { DCHECK(cls->CanCallRuntime()); LoadClassSlowPathARMVIXL* slow_path = - new (codegen_->GetScopedAllocator()) LoadClassSlowPathARMVIXL( - cls, cls, cls->GetDexPc(), cls->MustGenerateClinitCheck()); + new (codegen_->GetScopedAllocator()) LoadClassSlowPathARMVIXL(cls, cls); codegen_->AddSlowPath(slow_path); if (generate_null_check) { __ CompareAndBranchIfZero(out, slow_path->GetEntryLabel()); @@ -7555,10 +7558,7 @@ void LocationsBuilderARMVIXL::VisitClinitCheck(HClinitCheck* check) { void InstructionCodeGeneratorARMVIXL::VisitClinitCheck(HClinitCheck* check) { // We assume the class is not null. LoadClassSlowPathARMVIXL* slow_path = - new (codegen_->GetScopedAllocator()) LoadClassSlowPathARMVIXL(check->GetLoadClass(), - check, - check->GetDexPc(), - /* do_clinit */ true); + new (codegen_->GetScopedAllocator()) LoadClassSlowPathARMVIXL(check->GetLoadClass(), check); codegen_->AddSlowPath(slow_path); GenerateClassInitializationCheck(slow_path, InputRegisterAt(check, 0)); } diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index 4aed2c091c..60bbf4c9f0 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -222,35 +222,41 @@ class DivZeroCheckSlowPathMIPS : public SlowPathCodeMIPS { class LoadClassSlowPathMIPS : public SlowPathCodeMIPS { public: - LoadClassSlowPathMIPS(HLoadClass* cls, - HInstruction* at, - uint32_t dex_pc, - bool do_clinit) - : SlowPathCodeMIPS(at), - cls_(cls), - dex_pc_(dex_pc), - do_clinit_(do_clinit) { + LoadClassSlowPathMIPS(HLoadClass* cls, HInstruction* at) + : SlowPathCodeMIPS(at), cls_(cls) { DCHECK(at->IsLoadClass() || at->IsClinitCheck()); + DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); } void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { LocationSummary* locations = instruction_->GetLocations(); Location out = locations->Out(); + const uint32_t dex_pc = instruction_->GetDexPc(); + bool must_resolve_type = instruction_->IsLoadClass() && cls_->MustResolveTypeOnSlowPath(); + bool must_do_clinit = instruction_->IsClinitCheck() || cls_->MustGenerateClinitCheck(); + CodeGeneratorMIPS* mips_codegen = down_cast<CodeGeneratorMIPS*>(codegen); - InvokeRuntimeCallingConvention calling_convention; - DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); __ Bind(GetEntryLabel()); SaveLiveRegisters(codegen, locations); - dex::TypeIndex type_index = cls_->GetTypeIndex(); - __ LoadConst32(calling_convention.GetRegisterAt(0), type_index.index_); - QuickEntrypointEnum entrypoint = do_clinit_ ? kQuickInitializeStaticStorage - : kQuickInitializeType; - mips_codegen->InvokeRuntime(entrypoint, instruction_, dex_pc_, this); - if (do_clinit_) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - } else { + InvokeRuntimeCallingConvention calling_convention; + if (must_resolve_type) { + DCHECK(IsSameDexFile(cls_->GetDexFile(), mips_codegen->GetGraph()->GetDexFile())); + dex::TypeIndex type_index = cls_->GetTypeIndex(); + __ LoadConst32(calling_convention.GetRegisterAt(0), type_index.index_); + mips_codegen->InvokeRuntime(kQuickInitializeType, instruction_, dex_pc, this); CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); + // If we also must_do_clinit, the resolved type is now in the correct register. + } else { + DCHECK(must_do_clinit); + Location source = instruction_->IsLoadClass() ? out : locations->InAt(0); + mips_codegen->MoveLocation(Location::RegisterLocation(calling_convention.GetRegisterAt(0)), + source, + cls_->GetType()); + } + if (must_do_clinit) { + mips_codegen->InvokeRuntime(kQuickInitializeStaticStorage, instruction_, dex_pc, this); + CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, mirror::Class*>(); } // Move the class to the desired location. @@ -272,12 +278,6 @@ class LoadClassSlowPathMIPS : public SlowPathCodeMIPS { // The class this slow path will load. HLoadClass* const cls_; - // The dex PC of `at_`. - const uint32_t dex_pc_; - - // Whether to initialize the class. - const bool do_clinit_; - DISALLOW_COPY_AND_ASSIGN(LoadClassSlowPathMIPS); }; @@ -3598,11 +3598,8 @@ void LocationsBuilderMIPS::VisitClinitCheck(HClinitCheck* check) { void InstructionCodeGeneratorMIPS::VisitClinitCheck(HClinitCheck* check) { // We assume the class is not null. - SlowPathCodeMIPS* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS( - check->GetLoadClass(), - check, - check->GetDexPc(), - true); + SlowPathCodeMIPS* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS(check->GetLoadClass(), check); codegen_->AddSlowPath(slow_path); GenerateClassInitializationCheck(slow_path, check->GetLocations()->InAt(0).AsRegister<Register>()); @@ -8277,8 +8274,8 @@ void InstructionCodeGeneratorMIPS::VisitLoadClass(HLoadClass* cls) NO_THREAD_SAF if (generate_null_check || cls->MustGenerateClinitCheck()) { DCHECK(cls->CanCallRuntime()); - SlowPathCodeMIPS* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS( - cls, cls, cls->GetDexPc(), cls->MustGenerateClinitCheck()); + SlowPathCodeMIPS* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS(cls, cls); codegen_->AddSlowPath(slow_path); if (generate_null_check) { __ Beqz(out, slow_path->GetEntryLabel()); diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 75169139cd..81d86a9a3f 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -175,35 +175,41 @@ class DivZeroCheckSlowPathMIPS64 : public SlowPathCodeMIPS64 { class LoadClassSlowPathMIPS64 : public SlowPathCodeMIPS64 { public: - LoadClassSlowPathMIPS64(HLoadClass* cls, - HInstruction* at, - uint32_t dex_pc, - bool do_clinit) - : SlowPathCodeMIPS64(at), - cls_(cls), - dex_pc_(dex_pc), - do_clinit_(do_clinit) { + LoadClassSlowPathMIPS64(HLoadClass* cls, HInstruction* at) + : SlowPathCodeMIPS64(at), cls_(cls) { DCHECK(at->IsLoadClass() || at->IsClinitCheck()); + DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); } void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { LocationSummary* locations = instruction_->GetLocations(); Location out = locations->Out(); + const uint32_t dex_pc = instruction_->GetDexPc(); + bool must_resolve_type = instruction_->IsLoadClass() && cls_->MustResolveTypeOnSlowPath(); + bool must_do_clinit = instruction_->IsClinitCheck() || cls_->MustGenerateClinitCheck(); + CodeGeneratorMIPS64* mips64_codegen = down_cast<CodeGeneratorMIPS64*>(codegen); - InvokeRuntimeCallingConvention calling_convention; - DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); __ Bind(GetEntryLabel()); SaveLiveRegisters(codegen, locations); - dex::TypeIndex type_index = cls_->GetTypeIndex(); - __ LoadConst32(calling_convention.GetRegisterAt(0), type_index.index_); - QuickEntrypointEnum entrypoint = do_clinit_ ? kQuickInitializeStaticStorage - : kQuickInitializeType; - mips64_codegen->InvokeRuntime(entrypoint, instruction_, dex_pc_, this); - if (do_clinit_) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - } else { + InvokeRuntimeCallingConvention calling_convention; + if (must_resolve_type) { + DCHECK(IsSameDexFile(cls_->GetDexFile(), mips64_codegen->GetGraph()->GetDexFile())); + dex::TypeIndex type_index = cls_->GetTypeIndex(); + __ LoadConst32(calling_convention.GetRegisterAt(0), type_index.index_); + mips64_codegen->InvokeRuntime(kQuickInitializeType, instruction_, dex_pc, this); CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); + // If we also must_do_clinit, the resolved type is now in the correct register. + } else { + DCHECK(must_do_clinit); + Location source = instruction_->IsLoadClass() ? out : locations->InAt(0); + mips64_codegen->MoveLocation(Location::RegisterLocation(calling_convention.GetRegisterAt(0)), + source, + cls_->GetType()); + } + if (must_do_clinit) { + mips64_codegen->InvokeRuntime(kQuickInitializeStaticStorage, instruction_, dex_pc, this); + CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, mirror::Class*>(); } // Move the class to the desired location. @@ -225,12 +231,6 @@ class LoadClassSlowPathMIPS64 : public SlowPathCodeMIPS64 { // The class this slow path will load. HLoadClass* const cls_; - // The dex PC of `at_`. - const uint32_t dex_pc_; - - // Whether to initialize the class. - const bool do_clinit_; - DISALLOW_COPY_AND_ASSIGN(LoadClassSlowPathMIPS64); }; @@ -3153,11 +3153,8 @@ void LocationsBuilderMIPS64::VisitClinitCheck(HClinitCheck* check) { void InstructionCodeGeneratorMIPS64::VisitClinitCheck(HClinitCheck* check) { // We assume the class is not null. - SlowPathCodeMIPS64* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS64( - check->GetLoadClass(), - check, - check->GetDexPc(), - true); + SlowPathCodeMIPS64* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS64(check->GetLoadClass(), check); codegen_->AddSlowPath(slow_path); GenerateClassInitializationCheck(slow_path, check->GetLocations()->InAt(0).AsRegister<GpuRegister>()); @@ -6315,8 +6312,8 @@ void InstructionCodeGeneratorMIPS64::VisitLoadClass(HLoadClass* cls) NO_THREAD_S if (generate_null_check || cls->MustGenerateClinitCheck()) { DCHECK(cls->CanCallRuntime()); - SlowPathCodeMIPS64* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS64( - cls, cls, cls->GetDexPc(), cls->MustGenerateClinitCheck()); + SlowPathCodeMIPS64* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathMIPS64(cls, cls); codegen_->AddSlowPath(slow_path); if (generate_null_check) { __ Beqzc(out, slow_path->GetEntryLabel()); diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 30436eef9c..83ce734797 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -255,36 +255,42 @@ class LoadStringSlowPathX86 : public SlowPathCode { class LoadClassSlowPathX86 : public SlowPathCode { public: - LoadClassSlowPathX86(HLoadClass* cls, - HInstruction* at, - uint32_t dex_pc, - bool do_clinit) - : SlowPathCode(at), cls_(cls), dex_pc_(dex_pc), do_clinit_(do_clinit) { + LoadClassSlowPathX86(HLoadClass* cls, HInstruction* at) + : SlowPathCode(at), cls_(cls) { DCHECK(at->IsLoadClass() || at->IsClinitCheck()); + DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); } void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { LocationSummary* locations = instruction_->GetLocations(); + Location out = locations->Out(); + const uint32_t dex_pc = instruction_->GetDexPc(); + bool must_resolve_type = instruction_->IsLoadClass() && cls_->MustResolveTypeOnSlowPath(); + bool must_do_clinit = instruction_->IsClinitCheck() || cls_->MustGenerateClinitCheck(); + CodeGeneratorX86* x86_codegen = down_cast<CodeGeneratorX86*>(codegen); __ Bind(GetEntryLabel()); SaveLiveRegisters(codegen, locations); InvokeRuntimeCallingConvention calling_convention; - dex::TypeIndex type_index = cls_->GetTypeIndex(); - __ movl(calling_convention.GetRegisterAt(0), Immediate(type_index.index_)); - x86_codegen->InvokeRuntime(do_clinit_ ? kQuickInitializeStaticStorage - : kQuickInitializeType, - instruction_, - dex_pc_, - this); - if (do_clinit_) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - } else { + if (must_resolve_type) { + DCHECK(IsSameDexFile(cls_->GetDexFile(), x86_codegen->GetGraph()->GetDexFile())); + dex::TypeIndex type_index = cls_->GetTypeIndex(); + __ movl(calling_convention.GetRegisterAt(0), Immediate(type_index.index_)); + x86_codegen->InvokeRuntime(kQuickInitializeType, instruction_, dex_pc, this); CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); + // If we also must_do_clinit, the resolved type is now in the correct register. + } else { + DCHECK(must_do_clinit); + Location source = instruction_->IsLoadClass() ? out : locations->InAt(0); + x86_codegen->Move32(Location::RegisterLocation(calling_convention.GetRegisterAt(0)), source); + } + if (must_do_clinit) { + x86_codegen->InvokeRuntime(kQuickInitializeStaticStorage, instruction_, dex_pc, this); + CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, mirror::Class*>(); } // Move the class to the desired location. - Location out = locations->Out(); if (out.IsValid()) { DCHECK(out.IsRegister() && !locations->GetLiveRegisters()->ContainsCoreRegister(out.reg())); x86_codegen->Move32(out, Location::RegisterLocation(EAX)); @@ -299,12 +305,6 @@ class LoadClassSlowPathX86 : public SlowPathCode { // The class this slow path will load. HLoadClass* const cls_; - // The dex PC of `at_`. - const uint32_t dex_pc_; - - // Whether to initialize the class. - const bool do_clinit_; - DISALLOW_COPY_AND_ASSIGN(LoadClassSlowPathX86); }; @@ -6588,8 +6588,7 @@ void InstructionCodeGeneratorX86::VisitLoadClass(HLoadClass* cls) NO_THREAD_SAFE if (generate_null_check || cls->MustGenerateClinitCheck()) { DCHECK(cls->CanCallRuntime()); - SlowPathCode* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86( - cls, cls, cls->GetDexPc(), cls->MustGenerateClinitCheck()); + SlowPathCode* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86(cls, cls); codegen_->AddSlowPath(slow_path); if (generate_null_check) { @@ -6636,8 +6635,8 @@ void LocationsBuilderX86::VisitClinitCheck(HClinitCheck* check) { void InstructionCodeGeneratorX86::VisitClinitCheck(HClinitCheck* check) { // We assume the class to not be null. - SlowPathCode* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86( - check->GetLoadClass(), check, check->GetDexPc(), true); + SlowPathCode* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86(check->GetLoadClass(), check); codegen_->AddSlowPath(slow_path); GenerateClassInitializationCheck(slow_path, check->GetLocations()->InAt(0).AsRegister<Register>()); diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index 0d7837e70f..aabf2e0be4 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -239,34 +239,41 @@ class BoundsCheckSlowPathX86_64 : public SlowPathCode { class LoadClassSlowPathX86_64 : public SlowPathCode { public: - LoadClassSlowPathX86_64(HLoadClass* cls, - HInstruction* at, - uint32_t dex_pc, - bool do_clinit) - : SlowPathCode(at), cls_(cls), dex_pc_(dex_pc), do_clinit_(do_clinit) { + LoadClassSlowPathX86_64(HLoadClass* cls, HInstruction* at) + : SlowPathCode(at), cls_(cls) { DCHECK(at->IsLoadClass() || at->IsClinitCheck()); + DCHECK_EQ(instruction_->IsLoadClass(), cls_ == instruction_); } void EmitNativeCode(CodeGenerator* codegen) OVERRIDE { LocationSummary* locations = instruction_->GetLocations(); + Location out = locations->Out(); + const uint32_t dex_pc = instruction_->GetDexPc(); + bool must_resolve_type = instruction_->IsLoadClass() && cls_->MustResolveTypeOnSlowPath(); + bool must_do_clinit = instruction_->IsClinitCheck() || cls_->MustGenerateClinitCheck(); + CodeGeneratorX86_64* x86_64_codegen = down_cast<CodeGeneratorX86_64*>(codegen); __ Bind(GetEntryLabel()); - SaveLiveRegisters(codegen, locations); // Custom calling convention: RAX serves as both input and output. - __ movl(CpuRegister(RAX), Immediate(cls_->GetTypeIndex().index_)); - x86_64_codegen->InvokeRuntime(do_clinit_ ? kQuickInitializeStaticStorage : kQuickInitializeType, - instruction_, - dex_pc_, - this); - if (do_clinit_) { - CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, uint32_t>(); - } else { + if (must_resolve_type) { + DCHECK(IsSameDexFile(cls_->GetDexFile(), x86_64_codegen->GetGraph()->GetDexFile())); + dex::TypeIndex type_index = cls_->GetTypeIndex(); + __ movl(CpuRegister(RAX), Immediate(type_index.index_)); + x86_64_codegen->InvokeRuntime(kQuickInitializeType, instruction_, dex_pc, this); CheckEntrypointTypes<kQuickInitializeType, void*, uint32_t>(); + // If we also must_do_clinit, the resolved type is now in the correct register. + } else { + DCHECK(must_do_clinit); + Location source = instruction_->IsLoadClass() ? out : locations->InAt(0); + x86_64_codegen->Move(Location::RegisterLocation(RAX), source); + } + if (must_do_clinit) { + x86_64_codegen->InvokeRuntime(kQuickInitializeStaticStorage, instruction_, dex_pc, this); + CheckEntrypointTypes<kQuickInitializeStaticStorage, void*, mirror::Class*>(); } - Location out = locations->Out(); // Move the class to the desired location. if (out.IsValid()) { DCHECK(out.IsRegister() && !locations->GetLiveRegisters()->ContainsCoreRegister(out.reg())); @@ -283,12 +290,6 @@ class LoadClassSlowPathX86_64 : public SlowPathCode { // The class this slow path will load. HLoadClass* const cls_; - // The dex PC of `at_`. - const uint32_t dex_pc_; - - // Whether to initialize the class. - const bool do_clinit_; - DISALLOW_COPY_AND_ASSIGN(LoadClassSlowPathX86_64); }; @@ -5927,8 +5928,8 @@ void InstructionCodeGeneratorX86_64::VisitLoadClass(HLoadClass* cls) NO_THREAD_S if (generate_null_check || cls->MustGenerateClinitCheck()) { DCHECK(cls->CanCallRuntime()); - SlowPathCode* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86_64( - cls, cls, cls->GetDexPc(), cls->MustGenerateClinitCheck()); + SlowPathCode* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86_64(cls, cls); codegen_->AddSlowPath(slow_path); if (generate_null_check) { __ testl(out, out); @@ -5973,8 +5974,8 @@ void InstructionCodeGeneratorX86_64::VisitLoadMethodType(HLoadMethodType* load) void InstructionCodeGeneratorX86_64::VisitClinitCheck(HClinitCheck* check) { // We assume the class to not be null. - SlowPathCode* slow_path = new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86_64( - check->GetLoadClass(), check, check->GetDexPc(), true); + SlowPathCode* slow_path = + new (codegen_->GetScopedAllocator()) LoadClassSlowPathX86_64(check->GetLoadClass(), check); codegen_->AddSlowPath(slow_path); GenerateClassInitializationCheck(slow_path, check->GetLocations()->InAt(0).AsRegister<CpuRegister>()); diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 16a7417301..8b9e1da0d3 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -6284,6 +6284,13 @@ class HLoadClass FINAL : public HInstruction { bool IsInBootImage() const { return GetPackedFlag<kFlagIsInBootImage>(); } bool MustGenerateClinitCheck() const { return GetPackedFlag<kFlagGenerateClInitCheck>(); } + bool MustResolveTypeOnSlowPath() const { + // Check that this instruction has a slow path. + DCHECK(GetLoadKind() != LoadKind::kRuntimeCall); // kRuntimeCall calls on main path. + DCHECK(GetLoadKind() == LoadKind::kBssEntry || MustGenerateClinitCheck()); + return GetLoadKind() == LoadKind::kBssEntry; + } + void MarkInBootImage() { SetPackedFlag<kFlagIsInBootImage>(true); } diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc index 831bccc90a..060613d349 100644 --- a/compiler/optimizing/prepare_for_register_allocation.cc +++ b/compiler/optimizing/prepare_for_register_allocation.cc @@ -150,7 +150,9 @@ void PrepareForRegisterAllocation::VisitClinitCheck(HClinitCheck* check) { if (can_merge_with_load_class && !load_class->HasUses()) { load_class->GetBlock()->RemoveInstruction(load_class); } - } else if (can_merge_with_load_class && !load_class->NeedsAccessCheck()) { + } else if (can_merge_with_load_class && + load_class->GetLoadKind() != HLoadClass::LoadKind::kRuntimeCall) { + DCHECK(!load_class->NeedsAccessCheck()); // Pass the initialization duty to the `HLoadClass` instruction, // and remove the instruction from the graph. DCHECK(load_class->HasEnvironment()); diff --git a/runtime/arch/mips/entrypoints_init_mips.cc b/runtime/arch/mips/entrypoints_init_mips.cc index 2b69c1753b..dca59cab34 100644 --- a/runtime/arch/mips/entrypoints_init_mips.cc +++ b/runtime/arch/mips/entrypoints_init_mips.cc @@ -192,7 +192,7 @@ void InitEntryPoints(JniEntryPoints* jpoints, QuickEntryPoints* qpoints) { qpoints->pCheckInstanceOf = art_quick_check_instance_of; static_assert(!IsDirectEntrypoint(kQuickCheckInstanceOf), "Non-direct C stub marked direct."); - // DexCache + // Resolution and initialization qpoints->pInitializeStaticStorage = art_quick_initialize_static_storage; static_assert(!IsDirectEntrypoint(kQuickInitializeStaticStorage), "Non-direct C stub marked direct."); diff --git a/runtime/entrypoints/quick/quick_default_externs.h b/runtime/entrypoints/quick/quick_default_externs.h index 938489b730..70e81734a7 100644 --- a/runtime/entrypoints/quick/quick_default_externs.h +++ b/runtime/entrypoints/quick/quick_default_externs.h @@ -33,8 +33,8 @@ class ArtMethod; // Cast entrypoints. extern "C" void art_quick_check_instance_of(art::mirror::Object*, art::mirror::Class*); -// DexCache entrypoints. -extern "C" void* art_quick_initialize_static_storage(uint32_t); +// Resolution and initialization entrypoints. +extern "C" void* art_quick_initialize_static_storage(art::mirror::Class*); extern "C" void* art_quick_initialize_type(uint32_t); extern "C" void* art_quick_initialize_type_and_verify_access(uint32_t); extern "C" void* art_quick_resolve_method_handle(uint32_t); diff --git a/runtime/entrypoints/quick/quick_default_init_entrypoints.h b/runtime/entrypoints/quick/quick_default_init_entrypoints.h index 5dcece4208..32b6be29a5 100644 --- a/runtime/entrypoints/quick/quick_default_init_entrypoints.h +++ b/runtime/entrypoints/quick/quick_default_init_entrypoints.h @@ -33,7 +33,7 @@ static void DefaultInitEntryPoints(JniEntryPoints* jpoints, QuickEntryPoints* qp // Alloc ResetQuickAllocEntryPoints(qpoints, /* is_marking */ true); - // DexCache + // Resolution and initialization qpoints->pInitializeStaticStorage = art_quick_initialize_static_storage; qpoints->pInitializeTypeAndVerifyAccess = art_quick_initialize_type_and_verify_access; qpoints->pInitializeType = art_quick_initialize_type; diff --git a/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc b/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc index 85d633f6a6..c46ea356c1 100644 --- a/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc @@ -95,7 +95,7 @@ static inline void StoreTypeInBss(ArtMethod* outer_method, static inline void StoreStringInBss(ArtMethod* outer_method, dex::StringIndex string_idx, ObjPtr<mirror::String> resolved_string) - REQUIRES_SHARED(Locks::mutator_lock_) __attribute__((optnone)) { + REQUIRES_SHARED(Locks::mutator_lock_) { const DexFile* dex_file = outer_method->GetDexFile(); DCHECK(dex_file != nullptr); const OatDexFile* oat_dex_file = dex_file->GetOatDexFile(); @@ -129,24 +129,22 @@ static ALWAYS_INLINE bool CanReferenceBss(ArtMethod* outer_method, ArtMethod* ca return outer_method->GetDexFile() == caller->GetDexFile(); } -extern "C" mirror::Class* artInitializeStaticStorageFromCode(uint32_t type_idx, Thread* self) +extern "C" mirror::Class* artInitializeStaticStorageFromCode(mirror::Class* klass, Thread* self) REQUIRES_SHARED(Locks::mutator_lock_) { // Called to ensure static storage base is initialized for direct static field reads and writes. // A class may be accessing another class' fields when it doesn't have access, as access has been // given by inheritance. ScopedQuickEntrypointChecks sqec(self); - auto caller_and_outer = GetCalleeSaveMethodCallerAndOuterMethod( - self, CalleeSaveType::kSaveEverythingForClinit); - ArtMethod* caller = caller_and_outer.caller; - ObjPtr<mirror::Class> result = ResolveVerifyAndClinit(dex::TypeIndex(type_idx), - caller, - self, - /* can_run_clinit */ true, - /* verify_access */ false); - if (LIKELY(result != nullptr) && CanReferenceBss(caller_and_outer.outer_method, caller)) { - StoreTypeInBss(caller_and_outer.outer_method, dex::TypeIndex(type_idx), result); + DCHECK(klass != nullptr); + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + StackHandleScope<1> hs(self); + Handle<mirror::Class> h_klass = hs.NewHandle(klass); + bool success = class_linker->EnsureInitialized( + self, h_klass, /* can_init_fields */ true, /* can_init_parents */ true); + if (UNLIKELY(!success)) { + return nullptr; } - return result.Ptr(); + return h_klass.Get(); } extern "C" mirror::Class* artInitializeTypeFromCode(uint32_t type_idx, Thread* self) diff --git a/runtime/entrypoints/quick/quick_entrypoints_list.h b/runtime/entrypoints/quick/quick_entrypoints_list.h index 4ce954c48a..014f1bd4cc 100644 --- a/runtime/entrypoints/quick/quick_entrypoints_list.h +++ b/runtime/entrypoints/quick/quick_entrypoints_list.h @@ -38,7 +38,7 @@ V(InstanceofNonTrivial, size_t, mirror::Object*, mirror::Class*) \ V(CheckInstanceOf, void, mirror::Object*, mirror::Class*) \ \ - V(InitializeStaticStorage, void*, uint32_t) \ + V(InitializeStaticStorage, void*, mirror::Class*) \ V(InitializeTypeAndVerifyAccess, void*, uint32_t) \ V(InitializeType, void*, uint32_t) \ V(ResolveMethodHandle, void*, uint32_t) \ diff --git a/runtime/oat.h b/runtime/oat.h index 4ad065a238..0a34ea0a24 100644 --- a/runtime/oat.h +++ b/runtime/oat.h @@ -32,8 +32,8 @@ class InstructionSetFeatures; class PACKED(4) OatHeader { public: static constexpr uint8_t kOatMagic[] = { 'o', 'a', 't', '\n' }; - // Last oat version changed reason: Encode frame info using varints. - static constexpr uint8_t kOatVersion[] = { '1', '5', '9', '\0' }; + // Last oat version changed reason: Pass Class reference to clinit entrypoint. + static constexpr uint8_t kOatVersion[] = { '1', '6', '0', '\0' }; static constexpr const char* kImageLocationKey = "image-location"; static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline"; diff --git a/test/683-clinit-inline-static-invoke/expected.txt b/test/683-clinit-inline-static-invoke/expected.txt new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/test/683-clinit-inline-static-invoke/expected.txt diff --git a/test/683-clinit-inline-static-invoke/info.txt b/test/683-clinit-inline-static-invoke/info.txt new file mode 100644 index 0000000000..32e5cdcb99 --- /dev/null +++ b/test/683-clinit-inline-static-invoke/info.txt @@ -0,0 +1,3 @@ +Regression test for a bug where the class initialization check for an inlined +call to a static method used a type index from the wrong dex file because the +current dex file does not have a TypeId for it. This was likely to crash. diff --git a/test/683-clinit-inline-static-invoke/multidex.jpp b/test/683-clinit-inline-static-invoke/multidex.jpp new file mode 100644 index 0000000000..b0d200ea38 --- /dev/null +++ b/test/683-clinit-inline-static-invoke/multidex.jpp @@ -0,0 +1,3 @@ +Main: + @@com.android.jack.annotations.ForceInMainDex + class Main diff --git a/test/683-clinit-inline-static-invoke/src-multidex/MyTimeZone.java b/test/683-clinit-inline-static-invoke/src-multidex/MyTimeZone.java new file mode 100644 index 0000000000..b74b310a45 --- /dev/null +++ b/test/683-clinit-inline-static-invoke/src-multidex/MyTimeZone.java @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import android.icu.util.TimeZone; + +public abstract class MyTimeZone extends TimeZone { + // Reference to MyTimeZone.getDefaultTimeZoneType() shall resolve + // to TimeZone.getDefaultTimeZoneType() which should be easily inlined. +} diff --git a/test/683-clinit-inline-static-invoke/src/Main.java b/test/683-clinit-inline-static-invoke/src/Main.java new file mode 100644 index 0000000000..b4ccfaa95b --- /dev/null +++ b/test/683-clinit-inline-static-invoke/src/Main.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class Main { + public static void main(String[] args) { + // The following is a simple static field getter that can be inlined, referenced + // through a subclass with the declaring class having no TypeId in current DexFile. + // When we inline this getter, we're left with HLoadClass+HClinitCheck which cannot + // be merged back to the InvokeStaticOrDirect for implicit class init check. + // The declaring class is in the boot image, so the LoadClass can load it using the + // .data.bimg.rel.ro section. However, the ClinitCheck entrypoint was previously + // taking a type index of the declaring class and since we did not have a valid + // TypeId in the current DexFile, we erroneously provided the type index from the + // declaring DexFile and that caused a crash. This was fixed by changing the + // ClinitCheck entrypoint to take the Class reference from LoadClass. + int dummy = MyTimeZone.getDefaultTimeZoneType(); + } +} diff --git a/test/knownfailures.json b/test/knownfailures.json index 4bb5f0787e..962c5cfc97 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -678,6 +678,11 @@ "description": ["Requires zip, which isn't available on device"] }, { + "tests": ["683-clinit-inline-static-invoke"], + "variant": "jvm", + "description": ["Uses android-specific boot image class."] + }, + { "tests": ["1941-dispose-stress", "522-checker-regression-monitor-exit"], "variant": "jvm", "bug": "b/73888836", |