Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 311416 - [vm][cache] Stack frames missing in debug view
Summary: [vm][cache] Stack frames missing in debug view
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf-gdb (show other bugs)
Version: 7.0   Edit
Hardware: PC Linux
: P3 major (vote)
Target Milestone: 7.0   Edit
Assignee: Pawel Piech CLA
QA Contact: Marc Khouzam CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-03 14:37 EDT by Marc Khouzam CLA
Modified: 2010-07-28 15:25 EDT (History)
2 users (show)

See Also:
marc.khouzam: review+

Attachments
Snaptshot of missing frames (there is not main) (27.43 KB, image/png)
2010-05-03 14:37 EDT, Marc Khouzam CLA 		no flags Details
Snapshot after refreshing the view (51.92 KB, image/png)
2010-05-03 14:38 EDT, Marc Khouzam CLA 		marc.khouzam: iplog-
Details
Fix. (1.06 KB, patch)
2010-05-04 00:52 EDT, Pawel Piech CLA 		no flags Details | Diff
Tiny simplification (1005 bytes, patch)
2010-05-04 09:20 EDT, Marc Khouzam CLA 		marc.khouzam: iplog-
Details | Diff
Java doc fix (1.46 KB, patch)
2010-05-04 09:36 EDT, Marc Khouzam CLA 		marc.khouzam: iplog-
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Khouzam CLA 2010-05-03 14:37:24 EDT 
Created attachment 166845 [details]
Snaptshot of missing frames (there is not main)

I believe the fix to Bug 310992 is causing problems in the debug view.  When I step into a recursive method, I only see two stack frames in the debug view.  If I refresh the view, I see all the frames.

I think this only happens if I turn off the "Limit stack frames" preferences.
Comment 1 Marc Khouzam CLA 2010-05-03 14:38:19 EDT 
Created attachment 166846 [details]
Snapshot after refreshing the view

Snapshots of before and after a refresh.  We can see some missing frames.
Comment 2 Pawel Piech CLA 2010-05-04 00:52:38 EDT 
Created attachment 166902 [details]
Fix.
Comment 3 Pawel Piech CLA 2010-05-04 00:54:23 EDT 
(In reply to comment #0)
> I believe the fix to Bug 310992 is causing problems in the debug view.  When I
> step into a recursive method, I only see two stack frames in the debug view. 
> If I refresh the view, I see all the frames.
Fortunately it's a false alarm, the bug is a caching bug in MIStack.

Marc, please review.
Comment 4 Anton Leherbauer CLA 2010-05-04 03:03:30 EDT 
(In reply to comment #3)
> Fortunately it's a false alarm, the bug is a caching bug in MIStack.

Nice find.  Thanks, Pawel!
Comment 5 Marc Khouzam CLA 2010-05-04 09:20:34 EDT 
Created attachment 166950 [details]
Tiny simplification

Thanks for the fix Pawel!  I guess the extra refresh that was removed by bug 310992 was hiding this bug.  Sorry about the mis-diagnosis.

I've committed this patch to simplify the check that had the error.  I hope that is ok with you.
Comment 6 Marc Khouzam CLA 2010-05-04 09:36:50 EDT 
Created attachment 166952 [details]
Java doc fix

Looking at this code, I realize that the javadoc for IStack.getStackDepth() is not clear (I wrote it :-( ).  I think we should indicate if getStackDepth() should restrict itself to maxDepth or if it is allowed to return a number greater than stackDepth.  

After thinking about it, I think we should allow the service to return more depth that requested if it wants (it could be a cheap operation).  In fact, the DSF-GDB implementation, when using the cache you just fixed, does not always respect the maxDepth param.  But the views are prepared for that.

I suggest the following change.  Not committed.
What do you think?
Comment 7 Pawel Piech CLA 2010-05-04 13:27:08 EDT 
(In reply to comment #5)
> I've committed this patch to simplify the check that had the error.  I hope
> that is ok with you.
+1

(In reply to comment #6)
> I suggest the following change.  Not committed.
> What do you think?
Yes, this makes sense to me.
Comment 8 Marc Khouzam CLA 2010-05-04 13:58:27 EDT 
(In reply to comment #7)
> (In reply to comment #6)
> > I suggest the following change.  Not committed.
> > What do you think?
> Yes, this makes sense to me.

Thanks.  Committed to HEAD.