Updates:
Cc: [email protected] [email protected]
Comment #1 on issue 2047 by [email protected]:
test-debug/ThreadedDebugging is flaky
http://code.google.com/p/v8/issues/detail?id=2047
This seems to be a can of worms. Here are my findings:
1.) There is indeed a real bug that is uncovered by
test-debug/ThreadedDebugging: The PC of the unoptimized frame for the
function under test is correct, but gets translated to a wrong source
position, namely to the one of the statement immediately before it. This
can be fixed by:
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -8058,8 +8058,8 @@ int Code::SourcePosition(Address pc) {
// source.
RelocIterator it(this, RelocInfo::kPositionMask);
while (!it.done()) {
- // Only look at positions after the current pc.
- if (it.rinfo()->pc() < pc) {
+ // Only look at positions up to and including the current pc.
+ if (it.rinfo()->pc() <= pc) {
// Get position and distance.
int dist = static_cast<int>(pc - it.rinfo()->pc());
I believe that this change is the Right Thing To Do; however, it makes
other tests fail, mostly related to break points and stepping. Apparently
we're relying on the old behavior somewhere. It looks like the change makes
us step too far (or too often), but that might just be a red herring.
2.) A similar suspicious "<" instead of "<=" is in "void
BreakLocationIterator::FindBreakLocationFromAddress(Address pc)" in
debug.cc, where we apparently hope to find a PC that's an exact match ("if
(distance == 0) break;"), but the strictly-smaller condition prevents us
from ever finding it. Changing this on its own causes test failures, too;
when it's changed in addition to the above patch for (1) the failure count
stays the same.
3.) Looking one method further down in "void
BreakLocationIterator::FindBreakLocationFromPosition(int position)", it
strikes me as odd that we invert the search scope by looking only at
breakpoints that have positions that are ">=" the input. This might be on
purpose, though, I don't have enough context to be sure.
4.) mjsunit/debug-return-value assumes that when a "debugger;" statement
triggers a debug break, the frame's current PC can be mapped to
the "debugger;" statement's source position. This feels inconsistent to me:
I would expect the statement to behave like a break point, i.e. when it
triggers, the PC already points to the next instruction that will be
executed.
I will relax the CHECK in test-debug/ThreadedDebugging for now to make the
test pass reliably, and keep this bug open to track the fact that this
should be fixed properly.
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
