From: svenpanne@chromium.org Date: Tue, 18 Mar 2014 07:13:55 +0000 (+0000) Subject: Robustified address calculations on A64. X-Git-Tag: upstream/4.7.83~10209 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d7844ec8cccd11e4e3077b1c20170abb1f9a59e2;p=platform%2Fupstream%2Fv8.git Robustified address calculations on A64. We no longer rely on the (adventurous) assumption that the class Instruction is empty, implying sizeof(Instruction) == 1. This will greatly simplify upcoming refactorings. R=rodolph.perfetta@gmail.com Review URL: https://codereview.chromium.org/201843003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20018 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/a64/disasm-a64.cc b/src/a64/disasm-a64.cc index 703595b..bc3cbcd 100644 --- a/src/a64/disasm-a64.cc +++ b/src/a64/disasm-a64.cc @@ -1607,8 +1607,8 @@ int Disassembler::SubstitutePCRelAddressField(Instruction* instr, offset = -offset; sign = '-'; } - STATIC_ASSERT(sizeof(*instr) == 1); - AppendToOutput("#%c0x%x (addr %p)", sign, offset, instr + offset); + AppendToOutput("#%c0x%x (addr %p)", sign, offset, + instr->InstructionAtOffset(offset, Instruction::NO_CHECK)); return 13; } @@ -1635,8 +1635,8 @@ int Disassembler::SubstituteBranchTargetField(Instruction* instr, offset = -offset; sign = '-'; } - STATIC_ASSERT(sizeof(*instr) == 1); - AppendToOutput("#%c0x%" PRIx64 " (addr %p)", sign, offset, instr + offset); + AppendToOutput("#%c0x%" PRIx64 " (addr %p)", sign, offset, + instr->InstructionAtOffset(offset), Instruction::NO_CHECK); return 8; } diff --git a/src/a64/instructions-a64.cc b/src/a64/instructions-a64.cc index be61229..17f4f2f 100644 --- a/src/a64/instructions-a64.cc +++ b/src/a64/instructions-a64.cc @@ -226,7 +226,7 @@ ptrdiff_t Instruction::ImmPCOffset() { Instruction* Instruction::ImmPCOffsetTarget() { - return this + ImmPCOffset(); + return InstructionAtOffset(ImmPCOffset()); } @@ -237,8 +237,7 @@ bool Instruction::IsValidImmPCOffset(ImmBranchType branch_type, bool Instruction::IsTargetInImmPCOffsetRange(Instruction* target) { - int offset = target - this; - return IsValidImmPCOffset(BranchType(), offset); + return IsValidImmPCOffset(BranchType(), DistanceTo(target)); } @@ -257,17 +256,17 @@ void Instruction::SetPCRelImmTarget(Instruction* target) { // ADRP is not supported, so 'this' must point to an ADR instruction. ASSERT(Mask(PCRelAddressingMask) == ADR); - Instr imm = Assembler::ImmPCRelAddress(target - this); + Instr imm = Assembler::ImmPCRelAddress(DistanceTo(target)); SetInstructionBits(Mask(~ImmPCRel_mask) | imm); } void Instruction::SetBranchImmTarget(Instruction* target) { - ASSERT(((target - this) & 3) == 0); + ASSERT(IsAligned(DistanceTo(target), kInstructionSize)); Instr branch_imm = 0; uint32_t imm_mask = 0; - int offset = (target - this) >> kInstructionSizeLog2; + ptrdiff_t offset = DistanceTo(target) >> kInstructionSizeLog2; switch (BranchType()) { case CondBranchType: { branch_imm = Assembler::ImmCondBranch(offset); @@ -296,8 +295,8 @@ void Instruction::SetBranchImmTarget(Instruction* target) { void Instruction::SetImmLLiteral(Instruction* source) { - ASSERT(((source - this) & 3) == 0); - int offset = (source - this) >> kLiteralEntrySizeLog2; + ASSERT(IsAligned(DistanceTo(source), kInstructionSize)); + ptrdiff_t offset = DistanceTo(source) >> kLiteralEntrySizeLog2; Instr imm = Assembler::ImmLLiteral(offset); Instr mask = ImmLLiteral_mask; diff --git a/src/a64/instructions-a64.h b/src/a64/instructions-a64.h index ccab39e..caeae9f 100644 --- a/src/a64/instructions-a64.h +++ b/src/a64/instructions-a64.h @@ -144,12 +144,12 @@ class Instruction { return InstructionBits() & mask; } - Instruction* following(int count = 1) { - return this + count * kInstructionSize; + V8_INLINE Instruction* following(int count = 1) { + return InstructionAtOffset(count * static_cast(kInstructionSize)); } - Instruction* preceding(int count = 1) { - return this - count * kInstructionSize; + V8_INLINE Instruction* preceding(int count = 1) { + return following(-count); } #define DEFINE_GETTER(Name, HighBit, LowBit, Func) \ @@ -367,20 +367,25 @@ class Instruction { return reinterpret_cast(this) + offset; } - Instruction* NextInstruction() { - return this + kInstructionSize; - } + enum CheckAlignment { NO_CHECK, CHECK_ALIGNMENT }; - Instruction* InstructionAtOffset(int64_t offset) { - ASSERT(IsAligned(reinterpret_cast(this) + offset, - kInstructionSize)); - return this + offset; + V8_INLINE Instruction* InstructionAtOffset( + int64_t offset, + CheckAlignment check = CHECK_ALIGNMENT) { + Address addr = reinterpret_cast
(this) + offset; + // The FUZZ_disasm test relies on no check being done. + ASSERT(check == NO_CHECK || IsAddressAligned(addr, kInstructionSize)); + return Cast(addr); } - template static Instruction* Cast(T src) { + template V8_INLINE static Instruction* Cast(T src) { return reinterpret_cast(src); } + V8_INLINE ptrdiff_t DistanceTo(Instruction* target) { + return reinterpret_cast
(target) - reinterpret_cast
(this); + } + void SetPCRelImmTarget(Instruction* target); void SetBranchImmTarget(Instruction* target); diff --git a/src/a64/macro-assembler-a64.cc b/src/a64/macro-assembler-a64.cc index e876201..ca61b49 100644 --- a/src/a64/macro-assembler-a64.cc +++ b/src/a64/macro-assembler-a64.cc @@ -5135,7 +5135,7 @@ InlineSmiCheckInfo::InlineSmiCheckInfo(Address info) reg_ = Register::XRegFromCode(reg_code); uint64_t smi_check_delta = DeltaBits::decode(payload); ASSERT(smi_check_delta != 0); - smi_check_ = inline_data - (smi_check_delta * kInstructionSize); + smi_check_ = inline_data->preceding(smi_check_delta); } } } diff --git a/src/a64/simulator-a64.cc b/src/a64/simulator-a64.cc index ca17105..8f7feee 100644 --- a/src/a64/simulator-a64.cc +++ b/src/a64/simulator-a64.cc @@ -807,7 +807,7 @@ void Simulator::CheckBreakpoints() { void Simulator::CheckBreakNext() { // If the current instruction is a BL, insert a breakpoint just after it. if (break_on_next_ && pc_->IsBranchAndLinkToRegister()) { - SetBreakpoint(pc_->NextInstruction()); + SetBreakpoint(pc_->following()); break_on_next_ = false; } } @@ -815,7 +815,7 @@ void Simulator::CheckBreakNext() { void Simulator::PrintInstructionsAt(Instruction* start, uint64_t count) { Instruction* end = start->InstructionAtOffset(count * kInstructionSize); - for (Instruction* pc = start; pc < end; pc = pc->NextInstruction()) { + for (Instruction* pc = start; pc < end; pc = pc->following()) { disassembler_decoder_->Decode(pc); } } @@ -996,7 +996,7 @@ void Simulator::VisitPCRelAddressing(Instruction* instr) { void Simulator::VisitUnconditionalBranch(Instruction* instr) { switch (instr->Mask(UnconditionalBranchMask)) { case BL: - set_lr(instr->NextInstruction()); + set_lr(instr->following()); // Fall through. case B: set_pc(instr->ImmPCOffsetTarget()); @@ -1019,7 +1019,7 @@ void Simulator::VisitUnconditionalBranchToRegister(Instruction* instr) { Instruction* target = reg(instr->Rn()); switch (instr->Mask(UnconditionalBranchToRegisterMask)) { case BLR: { - set_lr(instr->NextInstruction()); + set_lr(instr->following()); if (instr->Rn() == 31) { // BLR XZR is used as a guard for the constant pool. We should never hit // this, but if we do trap to allow debugging. @@ -3362,12 +3362,16 @@ void Simulator::VisitException(Instruction* instr) { // Read the arguments encoded inline in the instruction stream. uint32_t code; uint32_t parameters; - char const * message; - ASSERT(sizeof(*pc_) == 1); - memcpy(&code, pc_ + kDebugCodeOffset, sizeof(code)); - memcpy(¶meters, pc_ + kDebugParamsOffset, sizeof(parameters)); - message = reinterpret_cast(pc_ + kDebugMessageOffset); + memcpy(&code, + pc_->InstructionAtOffset(kDebugCodeOffset), + sizeof(code)); + memcpy(¶meters, + pc_->InstructionAtOffset(kDebugParamsOffset), + sizeof(parameters)); + char const *message = + reinterpret_cast( + pc_->InstructionAtOffset(kDebugMessageOffset)); // Always print something when we hit a debug point that breaks. // We are going to break, so printing something is not an issue in @@ -3415,14 +3419,13 @@ void Simulator::VisitException(Instruction* instr) { // The stop parameters are inlined in the code. Skip them: // - Skip to the end of the message string. - pc_ += kDebugMessageOffset + strlen(message) + 1; - // - Advance to the next aligned location. - pc_ = AlignUp(pc_, kInstructionSize); + size_t size = kDebugMessageOffset + strlen(message) + 1; + pc_ = pc_->InstructionAtOffset(RoundUp(size, kInstructionSize)); // - Verify that the unreachable marker is present. ASSERT(pc_->Mask(ExceptionMask) == HLT); ASSERT(pc_->ImmException() == kImmExceptionIsUnreachable); // - Skip past the unreachable marker. - set_pc(pc_->NextInstruction()); + set_pc(pc_->following()); // Check if the debugger should break. if (parameters & BREAK) Debug(); @@ -3615,8 +3618,9 @@ void Simulator::VisitException(Instruction* instr) { } else if (instr->ImmException() == kImmExceptionIsPrintf) { // Read the argument encoded inline in the instruction stream. uint32_t type; - ASSERT(sizeof(*pc_) == 1); - memcpy(&type, pc_ + kPrintfTypeOffset, sizeof(type)); + memcpy(&type, + pc_->InstructionAtOffset(kPrintfTypeOffset), + sizeof(type)); const char* format = reg(0); diff --git a/src/a64/simulator-a64.h b/src/a64/simulator-a64.h index de7330b..c327ab8 100644 --- a/src/a64/simulator-a64.h +++ b/src/a64/simulator-a64.h @@ -329,7 +329,7 @@ class Simulator : public DecoderVisitor { void increment_pc() { if (!pc_modified_) { - pc_ = pc_->NextInstruction(); + pc_ = pc_->following(); } pc_modified_ = false;