From 2b03a1f24600c8c9558fb13d3f8bca1ef0f8ee40 Mon Sep 17 00:00:00 2001 From: Roland Levillain Date: Tue, 6 Jun 2017 16:09:59 +0100 Subject: Instrument ARM64 generated code to check the Marking Register. Generate run-time code in the Optimizing compiler checking that the Marking Register's value matches `self.tls32_.is.gc_marking` in debug mode (on target; and on host with JIT, or with AOT when compiling the core image). If a check fails, abort. Test: m test-art-target Test: m test-art-target with tree built with ART_USE_READ_BARRIER=false Test: ARM64 device boot test with libartd. Bug: 37707231 Change-Id: Ie9b322b22b3d26654a06821e1db71dbda3c43061 --- compiler/optimizing/code_generator_arm64.cc | 125 +++++++++++++++++++--------- compiler/optimizing/code_generator_arm64.h | 16 ++++ compiler/optimizing/codegen_test_utils.h | 42 +++++++++- compiler/optimizing/optimizing_compiler.cc | 9 +- 4 files changed, 141 insertions(+), 51 deletions(-) (limited to 'compiler/optimizing') diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 4999950600..3be774a421 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -1595,6 +1595,8 @@ void CodeGeneratorARM64::GenerateFrameEntry() { __ Str(wzr, MemOperand(sp, GetStackOffsetOfShouldDeoptimizeFlag())); } } + + MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void CodeGeneratorARM64::GenerateFrameExit() { @@ -3587,6 +3589,7 @@ void InstructionCodeGeneratorARM64::HandleGoto(HInstruction* got, HBasicBlock* s } if (block->IsEntryBlock() && (previous != nullptr) && previous->IsSuspendCheck()) { GenerateSuspendCheck(previous->AsSuspendCheck(), nullptr); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } if (!codegen_->GoesToNextBlock(block, successor)) { __ B(codegen_->GetLabelOf(successor)); @@ -4391,6 +4394,7 @@ void LocationsBuilderARM64::VisitInvokeUnresolved(HInvokeUnresolved* invoke) { void InstructionCodeGeneratorARM64::VisitInvokeUnresolved(HInvokeUnresolved* invoke) { codegen_->GenerateInvokeUnresolvedRuntimeCall(invoke); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::HandleInvoke(HInvoke* invoke) { @@ -4459,6 +4463,8 @@ void InstructionCodeGeneratorARM64::VisitInvokeInterface(HInvokeInterface* invok DCHECK(!codegen_->IsLeafMethod()); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); } + + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::VisitInvokeVirtual(HInvokeVirtual* invoke) { @@ -4626,6 +4632,7 @@ void LocationsBuilderARM64::VisitInvokePolymorphic(HInvokePolymorphic* invoke) { void InstructionCodeGeneratorARM64::VisitInvokePolymorphic(HInvokePolymorphic* invoke) { codegen_->GenerateInvokePolymorphicCall(invoke); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } vixl::aarch64::Label* CodeGeneratorARM64::NewPcRelativeMethodPatch( @@ -4801,27 +4808,37 @@ void InstructionCodeGeneratorARM64::VisitInvokeStaticOrDirect(HInvokeStaticOrDir DCHECK(!invoke->IsStaticWithExplicitClinitCheck()); if (TryGenerateIntrinsicCode(invoke, codegen_)) { + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); return; } - // Ensure that between the BLR (emitted by GenerateStaticOrDirectCall) and RecordPcInfo there - // are no pools emitted. - EmissionCheckScope guard(GetVIXLAssembler(), kInvokeCodeMarginSizeInBytes); - LocationSummary* locations = invoke->GetLocations(); - codegen_->GenerateStaticOrDirectCall( - invoke, locations->HasTemps() ? locations->GetTemp(0) : Location::NoLocation()); + { + // Ensure that between the BLR (emitted by GenerateStaticOrDirectCall) and RecordPcInfo there + // are no pools emitted. + EmissionCheckScope guard(GetVIXLAssembler(), kInvokeCodeMarginSizeInBytes); + LocationSummary* locations = invoke->GetLocations(); + codegen_->GenerateStaticOrDirectCall( + invoke, locations->HasTemps() ? locations->GetTemp(0) : Location::NoLocation()); + } + + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void InstructionCodeGeneratorARM64::VisitInvokeVirtual(HInvokeVirtual* invoke) { if (TryGenerateIntrinsicCode(invoke, codegen_)) { + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); return; } - // Ensure that between the BLR (emitted by GenerateVirtualCall) and RecordPcInfo there - // are no pools emitted. - EmissionCheckScope guard(GetVIXLAssembler(), kInvokeCodeMarginSizeInBytes); - codegen_->GenerateVirtualCall(invoke, invoke->GetLocations()->GetTemp(0)); - DCHECK(!codegen_->IsLeafMethod()); + { + // Ensure that between the BLR (emitted by GenerateVirtualCall) and RecordPcInfo there + // are no pools emitted. + EmissionCheckScope guard(GetVIXLAssembler(), kInvokeCodeMarginSizeInBytes); + codegen_->GenerateVirtualCall(invoke, invoke->GetLocations()->GetTemp(0)); + DCHECK(!codegen_->IsLeafMethod()); + } + + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } HLoadClass::LoadKind CodeGeneratorARM64::GetSupportedLoadClassKind( @@ -4895,6 +4912,7 @@ void InstructionCodeGeneratorARM64::VisitLoadClass(HLoadClass* cls) NO_THREAD_SA HLoadClass::LoadKind load_kind = cls->GetLoadKind(); if (load_kind == HLoadClass::LoadKind::kRuntimeCall) { codegen_->GenerateLoadClassRuntimeCall(cls); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); return; } DCHECK(!cls->NeedsAccessCheck()); @@ -4995,6 +5013,7 @@ void InstructionCodeGeneratorARM64::VisitLoadClass(HLoadClass* cls) NO_THREAD_SA } else { __ Bind(slow_path->GetExitLabel()); } + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } } @@ -5113,6 +5132,7 @@ void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) NO_THREAD codegen_->AddSlowPath(slow_path); __ Cbz(out.X(), slow_path->GetEntryLabel()); __ Bind(slow_path->GetExitLabel()); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); return; } case HLoadString::LoadKind::kJitTableAddress: { @@ -5137,6 +5157,7 @@ void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) NO_THREAD __ Mov(calling_convention.GetRegisterAt(0).W(), load->GetStringIndex().index_); codegen_->InvokeRuntime(kQuickResolveString, load, load->GetDexPc()); CheckEntrypointTypes(); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::VisitLongConstant(HLongConstant* constant) { @@ -5164,6 +5185,7 @@ void InstructionCodeGeneratorARM64::VisitMonitorOperation(HMonitorOperation* ins } else { CheckEntrypointTypes(); } + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::VisitMul(HMul* mul) { @@ -5260,6 +5282,7 @@ void InstructionCodeGeneratorARM64::VisitNewArray(HNewArray* instruction) { CodeGenerator::GetArrayAllocationEntrypoint(instruction->GetLoadClass()->GetClass()); codegen_->InvokeRuntime(entrypoint, instruction, instruction->GetDexPc()); CheckEntrypointTypes(); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::VisitNewInstance(HNewInstance* instruction) { @@ -5296,6 +5319,7 @@ void InstructionCodeGeneratorARM64::VisitNewInstance(HNewInstance* instruction) codegen_->InvokeRuntime(instruction->GetEntrypoint(), instruction, instruction->GetDexPc()); CheckEntrypointTypes(); } + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::VisitNot(HNot* instruction) { @@ -5644,6 +5668,7 @@ void InstructionCodeGeneratorARM64::VisitSuspendCheck(HSuspendCheck* instruction return; } GenerateSuspendCheck(instruction, nullptr); + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void LocationsBuilderARM64::VisitThrow(HThrow* instruction) { @@ -6021,6 +6046,7 @@ void InstructionCodeGeneratorARM64::GenerateGcRootFieldLoad( // Note that GC roots are not affected by heap poisoning, thus we // do not have to unpoison `root_reg` here. } + codegen_->MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void CodeGeneratorARM64::GenerateFieldLoadWithBakerReadBarrier(HInstruction* instruction, @@ -6074,22 +6100,25 @@ void CodeGeneratorARM64::GenerateFieldLoadWithBakerReadBarrier(HInstruction* ins obj.GetCode()); vixl::aarch64::Label* cbnz_label = NewBakerReadBarrierPatch(custom_data); - EmissionCheckScope guard(GetVIXLAssembler(), - (kPoisonHeapReferences ? 4u : 3u) * vixl::aarch64::kInstructionSize); - vixl::aarch64::Label return_address; - __ adr(lr, &return_address); - __ Bind(cbnz_label); - __ cbnz(mr, static_cast(0)); // Placeholder, patched at link-time. - static_assert(BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET == (kPoisonHeapReferences ? -8 : -4), - "Field LDR must be 1 instruction (4B) before the return address label; " - " 2 instructions (8B) for heap poisoning."); - Register ref_reg = RegisterFrom(ref, Primitive::kPrimNot); - __ ldr(ref_reg, MemOperand(base.X(), offset)); - if (needs_null_check) { - MaybeRecordImplicitNullCheck(instruction); + { + EmissionCheckScope guard(GetVIXLAssembler(), + (kPoisonHeapReferences ? 4u : 3u) * vixl::aarch64::kInstructionSize); + vixl::aarch64::Label return_address; + __ adr(lr, &return_address); + __ Bind(cbnz_label); + __ cbnz(mr, static_cast(0)); // Placeholder, patched at link-time. + static_assert(BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET == (kPoisonHeapReferences ? -8 : -4), + "Field LDR must be 1 instruction (4B) before the return address label; " + " 2 instructions (8B) for heap poisoning."); + Register ref_reg = RegisterFrom(ref, Primitive::kPrimNot); + __ ldr(ref_reg, MemOperand(base.X(), offset)); + if (needs_null_check) { + MaybeRecordImplicitNullCheck(instruction); + } + GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); + __ Bind(&return_address); } - GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); - __ Bind(&return_address); + MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__, /* temp_loc */ LocationFrom(ip1)); return; } @@ -6158,19 +6187,22 @@ void CodeGeneratorARM64::GenerateArrayLoadWithBakerReadBarrier(HInstruction* ins vixl::aarch64::Label* cbnz_label = NewBakerReadBarrierPatch(custom_data); __ Add(temp.X(), obj.X(), Operand(data_offset)); - EmissionCheckScope guard(GetVIXLAssembler(), - (kPoisonHeapReferences ? 4u : 3u) * vixl::aarch64::kInstructionSize); - vixl::aarch64::Label return_address; - __ adr(lr, &return_address); - __ Bind(cbnz_label); - __ cbnz(mr, static_cast(0)); // Placeholder, patched at link-time. - static_assert(BAKER_MARK_INTROSPECTION_ARRAY_LDR_OFFSET == (kPoisonHeapReferences ? -8 : -4), - "Array LDR must be 1 instruction (4B) before the return address label; " - " 2 instructions (8B) for heap poisoning."); - __ ldr(ref_reg, MemOperand(temp.X(), index_reg.X(), LSL, scale_factor)); - DCHECK(!needs_null_check); // The thunk cannot handle the null check. - GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); - __ Bind(&return_address); + { + EmissionCheckScope guard(GetVIXLAssembler(), + (kPoisonHeapReferences ? 4u : 3u) * vixl::aarch64::kInstructionSize); + vixl::aarch64::Label return_address; + __ adr(lr, &return_address); + __ Bind(cbnz_label); + __ cbnz(mr, static_cast(0)); // Placeholder, patched at link-time. + static_assert(BAKER_MARK_INTROSPECTION_ARRAY_LDR_OFFSET == (kPoisonHeapReferences ? -8 : -4), + "Array LDR must be 1 instruction (4B) before the return address label; " + " 2 instructions (8B) for heap poisoning."); + __ ldr(ref_reg, MemOperand(temp.X(), index_reg.X(), LSL, scale_factor)); + DCHECK(!needs_null_check); // The thunk cannot handle the null check. + GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); + __ Bind(&return_address); + } + MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__, /* temp_loc */ LocationFrom(ip1)); return; } @@ -6247,6 +6279,7 @@ void CodeGeneratorARM64::GenerateReferenceLoadWithBakerReadBarrier(HInstruction* GenerateRawReferenceLoad( instruction, ref, obj, offset, index, scale_factor, needs_null_check, use_load_acquire); __ Bind(slow_path->GetExitLabel()); + MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void CodeGeneratorARM64::UpdateReferenceFieldWithBakerReadBarrier(HInstruction* instruction, @@ -6303,6 +6336,7 @@ void CodeGeneratorARM64::UpdateReferenceFieldWithBakerReadBarrier(HInstruction* // Fast path: the GC is not marking: nothing to do (the field is // up-to-date, and we don't need to load the reference). __ Bind(slow_path->GetExitLabel()); + MaybeGenerateMarkingRegisterCheck(/* code */ __LINE__); } void CodeGeneratorARM64::GenerateRawReferenceLoad(HInstruction* instruction, @@ -6381,6 +6415,19 @@ void CodeGeneratorARM64::GenerateRawReferenceLoad(HInstruction* instruction, GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); } +void CodeGeneratorARM64::MaybeGenerateMarkingRegisterCheck(int code, Location temp_loc) { + // The following condition is a compile-time one, so it does not have a run-time cost. + if (kEmitCompilerReadBarrier && kUseBakerReadBarrier && kIsDebugBuild) { + // The following condition is a run-time one; it is executed after the + // previous compile-time test, to avoid penalizing non-debug builds. + if (GetCompilerOptions().EmitRunTimeChecksInDebugMode()) { + UseScratchRegisterScope temps(GetVIXLAssembler()); + Register temp = temp_loc.IsValid() ? WRegisterFrom(temp_loc) : temps.AcquireW(); + GetAssembler()->GenerateMarkingRegisterCheck(temp, code); + } + } +} + void CodeGeneratorARM64::GenerateReadBarrierSlow(HInstruction* instruction, Location out, Location ref, diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h index 584eead81b..c3392097a2 100644 --- a/compiler/optimizing/code_generator_arm64.h +++ b/compiler/optimizing/code_generator_arm64.h @@ -687,6 +687,22 @@ class CodeGeneratorARM64 : public CodeGenerator { bool needs_null_check, bool use_load_acquire); + // Emit code checking the status of the Marking Register, and + // aborting the program if MR does not match the value stored in the + // art::Thread object. Code is only emitted in debug mode and if + // CompilerOptions::EmitRunTimeChecksInDebugMode returns true. + // + // Argument `code` is used to identify the different occurrences of + // MaybeGenerateMarkingRegisterCheck in the code generator, and is + // passed to the BRK instruction. + // + // If `temp_loc` is a valid location, it is expected to be a + // register and will be used as a temporary to generate code; + // otherwise, a temporary will be fetched from the core register + // scratch pool. + virtual void MaybeGenerateMarkingRegisterCheck(int code, + Location temp_loc = Location::NoLocation()); + // Generate a read barrier for a heap reference within `instruction` // using a slow path. // diff --git a/compiler/optimizing/codegen_test_utils.h b/compiler/optimizing/codegen_test_utils.h index cada2e679b..bdd105fce7 100644 --- a/compiler/optimizing/codegen_test_utils.h +++ b/compiler/optimizing/codegen_test_utils.h @@ -103,6 +103,40 @@ class TestCodeGeneratorARMVIXL : public arm::CodeGeneratorARMVIXL { }; #endif +#ifdef ART_ENABLE_CODEGEN_arm64 +// Special ARM64 code generator for codegen testing in a limited code +// generation environment (i.e. with no runtime support). +// +// Note: If we want to exercise certains HIR constructions +// (e.g. reference field load in Baker read barrier configuration) in +// codegen tests in the future, we should also: +// - save the Thread Register (X19) and possibly the Marking Register +// (X20) before entering the generated function (both registers are +// callee-save in AAPCS64); +// - set these registers to meaningful values before or upon entering +// the generated function (so that generated code using them is +// correct); +// - restore their original values before leaving the generated +// function. +class TestCodeGeneratorARM64 : public arm64::CodeGeneratorARM64 { + public: + TestCodeGeneratorARM64(HGraph* graph, + const Arm64InstructionSetFeatures& isa_features, + const CompilerOptions& compiler_options) + : arm64::CodeGeneratorARM64(graph, isa_features, compiler_options) {} + + void MaybeGenerateMarkingRegisterCheck(int codem ATTRIBUTE_UNUSED, + Location temp_loc ATTRIBUTE_UNUSED) OVERRIDE { + // When turned on, the marking register checks in + // CodeGeneratorARM64::MaybeGenerateMarkingRegisterCheck expect the + // Thread Register and the Marking Register to be set to + // meaningful values. This is not the case in codegen testing, so + // just disable them entirely here (by doing nothing in this + // method). + } +}; +#endif + #ifdef ART_ENABLE_CODEGEN_x86 class TestCodeGeneratorX86 : public x86::CodeGeneratorX86 { public: @@ -263,7 +297,8 @@ static void RunCode(CodegenTargetConfig target_config, bool has_result, Expected expected) { CompilerOptions compiler_options; - std::unique_ptr codegen(target_config.CreateCodeGenerator(graph, compiler_options)); + std::unique_ptr codegen(target_config.CreateCodeGenerator(graph, + compiler_options)); RunCode(codegen.get(), graph, hook_before_codegen, has_result, expected); } @@ -280,9 +315,8 @@ CodeGenerator* create_codegen_arm_vixl32(HGraph* graph, const CompilerOptions& c CodeGenerator* create_codegen_arm64(HGraph* graph, const CompilerOptions& compiler_options) { std::unique_ptr features_arm64( Arm64InstructionSetFeatures::FromCppDefines()); - return new (graph->GetArena()) arm64::CodeGeneratorARM64(graph, - *features_arm64.get(), - compiler_options); + return new (graph->GetArena()) + TestCodeGeneratorARM64(graph, *features_arm64.get(), compiler_options); } #endif diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index 51101f104a..a6c33b461d 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -22,8 +22,6 @@ #include -#include "android-base/strings.h" - #ifdef ART_ENABLE_CODEGEN_arm64 #include "instruction_simplifier_arm64.h" #endif @@ -1111,12 +1109,7 @@ Compiler* CreateOptimizingCompiler(CompilerDriver* driver) { bool IsCompilingWithCoreImage() { const std::string& image = Runtime::Current()->GetImageLocation(); - // TODO: This is under-approximating... - if (android::base::EndsWith(image, "core.art") || - android::base::EndsWith(image, "core-optimizing.art")) { - return true; - } - return false; + return CompilerDriver::IsCoreImageFilename(image); } bool EncodeArtMethodInInlineInfo(ArtMethod* method ATTRIBUTE_UNUSED) { -- cgit v1.2.3-59-g8ed1b