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 (file)
--- 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 (file)
--- 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 (file)
--- 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<int>(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<uint8_t*>(this) + offset;
   }
 
-  Instruction* NextInstruction() {
-    return this + kInstructionSize;
-  }
+  enum CheckAlignment { NO_CHECK, CHECK_ALIGNMENT };
 
-  Instruction* InstructionAtOffset(int64_t offset) {
-    ASSERT(IsAligned(reinterpret_cast<uintptr_t>(this) + offset,
-                     kInstructionSize));
-    return this + offset;
+  V8_INLINE Instruction* InstructionAtOffset(
+      int64_t offset,
+      CheckAlignment check = CHECK_ALIGNMENT) {
+    Address addr = reinterpret_cast<Address>(this) + offset;
+    // The FUZZ_disasm test relies on no check being done.
+    ASSERT(check == NO_CHECK || IsAddressAligned(addr, kInstructionSize));
+    return Cast(addr);
   }
 
-  template<typename T> static Instruction* Cast(T src) {
+  template<typename T> V8_INLINE static Instruction* Cast(T src) {
     return reinterpret_cast<Instruction*>(src);
   }
 
+  V8_INLINE ptrdiff_t DistanceTo(Instruction* target) {
+    return reinterpret_cast<Address>(target) - reinterpret_cast<Address>(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 (file)
--- 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 (file)
--- 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<Instruction*>(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(&parameters, pc_ + kDebugParamsOffset, sizeof(parameters));
-        message = reinterpret_cast<char const *>(pc_ + kDebugMessageOffset);
+        memcpy(&code,
+               pc_->InstructionAtOffset(kDebugCodeOffset),
+               sizeof(code));
+        memcpy(&parameters,
+               pc_->InstructionAtOffset(kDebugParamsOffset),
+               sizeof(parameters));
+        char const *message =
+            reinterpret_cast<char const*>(
+                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<const char*>(0);
 

diff --git a/src/a64/simulator-a64.h b/src/a64/simulator-a64.h
index de7330b..c327ab8 100644 (file)
--- 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;