RelocInfo: fix source position decoding.

We used to rely on reading both POSITION and STATEMENT_POSITION to get
correct decoding of positions. This was error prone and made liveedit
unhappy.

Review URL: http://codereview.chromium.org/5277007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5905 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/assembler.cc

@@ -350,12 +350,8 @@ void RelocIterator::next() {
       Advance();
       // Check if we want source positions.
       if (mode_mask_ & RelocInfo::kPositionMask) {
-        // Check if we want this type of source position.
-        if (SetMode(DebugInfoModeFromTag(GetPositionTypeTag()))) {
-          // Finally read the data before returning.
-          ReadTaggedData();
-          return;
-        }
+        ReadTaggedData();
+        if (SetMode(DebugInfoModeFromTag(GetPositionTypeTag()))) return;
       }
     } else {
       ASSERT(tag == kDefaultTag);

src/assembler.h

@@ -419,7 +419,7 @@ class RelocIterator: public Malloced {
   // If the given mode is wanted, set it in rinfo_ and return true.
   // Else return false. Used for efficiently skipping unwanted modes.
   bool SetMode(RelocInfo::Mode mode) {
-    return (mode_mask_ & 1 << mode) ? (rinfo_.rmode_ = mode, true) : false;
+    return (mode_mask_ & (1 << mode)) ? (rinfo_.rmode_ = mode, true) : false;
   }
 
   byte* pos_;

src/runtime.cc

@@ -9887,7 +9887,7 @@ static MaybeObject* Runtime_GetFunctionCodePositionFromSource(Arguments args) {
 
   Handle<Code> code(function->code());
 
-  RelocIterator it(*code, 1 << RelocInfo::STATEMENT_POSITION);
+  RelocIterator it(*code, RelocInfo::ModeMask(RelocInfo::STATEMENT_POSITION));
   int closest_pc = 0;
   int distance = kMaxInt;
   while (!it.done()) {

test/cctest/SConscript

@@ -69,6 +69,7 @@ SOURCES = {
     'test-parsing.cc',
     'test-profile-generator.cc',
     'test-regexp.cc',
+    'test-reloc-info.cc',
     'test-serialize.cc',
     'test-sockets.cc',
     'test-spaces.cc',

test/cctest/test-reloc-info.cc

@@ -0,0 +1,109 @@
+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+#include "cctest.h"
+#include "assembler.h"
+
+namespace v8 {
+namespace internal {
+
+static void WriteRinfo(RelocInfoWriter* writer,
+                       byte* pc, RelocInfo::Mode mode, intptr_t data) {
+  RelocInfo rinfo(pc, mode, data);
+  writer->Write(&rinfo);
+}
+
+
+// Tests that writing both types of positions and then reading either
+// or both works as expected.
+TEST(Positions) {
+  const int instr_size = 10 << 10;
+  const int reloc_size = 10 << 10;
+  const int buf_size = instr_size + reloc_size;
+  SmartPointer<byte> buf(new byte[buf_size]);
+  byte* pc = *buf;
+  CodeDesc desc = { *buf, buf_size, instr_size, reloc_size, NULL };
+
+  RelocInfoWriter writer(*buf + buf_size, pc);
+  for (int i = 0, pos = 0; i < 100; i++, pc += i, pos += i) {
+    RelocInfo::Mode mode = (i % 2 == 0) ?
+        RelocInfo::STATEMENT_POSITION : RelocInfo::POSITION;
+    WriteRinfo(&writer, pc, mode, pos);
+  }
+
+  // Read only (non-statement) positions.
+  {
+    RelocIterator it(desc, RelocInfo::ModeMask(RelocInfo::POSITION));
+    pc = *buf;
+    for (int i = 0, pos = 0; i < 100; i++, pc += i, pos += i) {
+      RelocInfo::Mode mode = (i % 2 == 0) ?
+          RelocInfo::STATEMENT_POSITION : RelocInfo::POSITION;
+      if (mode == RelocInfo::POSITION) {
+        CHECK_EQ(pc, it.rinfo()->pc());
+        CHECK_EQ(mode, it.rinfo()->rmode());
+        CHECK_EQ(static_cast<intptr_t>(pos), it.rinfo()->data());
+        it.next();
+      }
+    }
+    CHECK(it.done());
+  }
+
+  // Read only statement positions.
+  {
+    RelocIterator it(desc, RelocInfo::ModeMask(RelocInfo::STATEMENT_POSITION));
+    pc = *buf;
+    for (int i = 0, pos = 0; i < 100; i++, pc += i, pos += i) {
+      RelocInfo::Mode mode = (i % 2 == 0) ?
+          RelocInfo::STATEMENT_POSITION : RelocInfo::POSITION;
+      if (mode == RelocInfo::STATEMENT_POSITION) {
+        CHECK_EQ(pc, it.rinfo()->pc());
+        CHECK_EQ(mode, it.rinfo()->rmode());
+        CHECK_EQ(static_cast<intptr_t>(pos), it.rinfo()->data());
+        it.next();
+      }
+    }
+    CHECK(it.done());
+  }
+
+  // Read both types of positions.
+  {
+    RelocIterator it(desc, RelocInfo::kPositionMask);
+    pc = *buf;
+    for (int i = 0, pos = 0; i < 100; i++, pc += i, pos += i) {
+      RelocInfo::Mode mode = (i % 2 == 0) ?
+          RelocInfo::STATEMENT_POSITION : RelocInfo::POSITION;
+      CHECK_EQ(pc, it.rinfo()->pc());
+      CHECK_EQ(mode, it.rinfo()->rmode());
+      CHECK_EQ(static_cast<intptr_t>(pos), it.rinfo()->data());
+      it.next();
+    }
+    CHECK(it.done());
+  }
+}
+
+} }  // namespace v8::internal