Bug 240556 - [expressions] Wrong handling of Thread or Memory/Process context
Summary: [expressions] Wrong handling of Thread or Memory/Process context
Status: ASSIGNED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf-gdb (show other bugs)
Version: 0 DD 1.1   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact: Jonah Graham CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-11 22:53 EDT by Marc Khouzam CLA
Modified: 2020-09-04 15:17 EDT (History)
2 users (show)

See Also:


Attachments
Patch to use top frame when thread selected (3.67 KB, patch)
2008-07-16 10:27 EDT, Marc Khouzam CLA
no flags Details | Diff
Suggest approach for Process context (1.82 KB, patch)
2008-07-17 09:23 EDT, Marc Khouzam CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Khouzam CLA 2008-07-11 22:53:45 EDT
The Expression service accepts an ExpressionDMC who's parent is a FrameDMC, an IMIExecutionDMC or an IMemoryDMC.  When a FrameDMC is used things work properly.

However, if a IMIExecutionDMC is used, which is a Thread, the expression service will make requests to GDB without paying attention to the current frame.  What that means is that the expression value that will be shown will be the one as per the current frame of GDB.  So, if I select a frame with a valid expression in it, then another frame with the same expression in it, but having a different value, and then select the Thread context, the expression value that will be shown will be the one of the last frame that was selected.

The same applies to the Memory (process) context.

In the case of CDI, when selecting a thread directly, the expression does not show a value anymore.  We should at least do the same.  However, an even better solution would be to evaluate the expression based on the thread name space, which would give global variables.
Comment 1 Marc Khouzam CLA 2008-07-11 23:02:23 EDT
(In reply to comment #0)
> However, an even better
> solution would be to evaluate the expression based on the thread name space,
> which would give global variables.

Also, this would make sense for a process context since global variables apply to a process.  I don't think there is a way to define variable specific to a thread, so if selecting a Thread context or a Process context, the same variables should be shown.

It would be nice to have this in the Variables view as well.

Comment 2 Pawel Piech CLA 2008-07-14 17:24:33 EDT
From my experience, if a user wants to see variables of a thread they usually are expecting to see variables in context of the top stack frame of that thread.  This could most simply be achieved in ExpressionService.createExpression() by calling MIStack.createFrameDMContext() directly to create an instance of a stack frame.

If the user selects a process in DV, I agree it would be best to evaluate in the global context only, but GDB doesn't have that capability...
Comment 3 Marc Khouzam CLA 2008-07-16 10:18:37 EDT
(In reply to comment #2)
> From my experience, if a user wants to see variables of a thread they usually
> are expecting to see variables in context of the top stack frame of that
> thread.  This could most simply be achieved in
> ExpressionService.createExpression() by calling MIStack.createFrameDMContext()
> directly to create an instance of a stack frame.

I liked that idea and coded a patch for it.  However during testing, I noticed that the variables view does not behave like this.  If I select a thread (or a prcess) in the debug view, the variables view goes blank.  This would not match the expressions view which would show the value of expressions as per the top frame.

I think it would be better to be consistent.  We could make the expression view just show the expression but not its value (effectively going 'blank'), or we could have the variables view use the top stack frame when a thread is selected.

If the user does expect the top-frame value when selecting a thread, then I vote to have the variables view show those.  But to be honest, either solution is fine with me, as long as we are consistent.

What do you say?

> If the user selects a process in DV, I agree it would be best to evaluate in
> the global context only, but GDB doesn't have that capability...

What we could do is use the C++ operator :: which allows to evaluate a variable in the global scope.  I believe the CDT does this.  However, this does not work in C and may prove complicated for complex expressions... It probably deserves a bug of its own.  Until can we make the expression view go 'blank' like the variables view in this case?

Comment 4 Marc Khouzam CLA 2008-07-16 10:27:45 EDT
Created attachment 107618 [details]
Patch to use top frame when thread selected

I'm posting the patch I wrote so I don't lose it.  It has JUnit tests also.  Let's see where the discussion goes to know if we'll use it.
Comment 5 Pawel Piech CLA 2008-07-16 13:23:41 EDT
I agree(In reply to comment #3)
> If the user does expect the top-frame value when selecting a thread, then I
> vote to have the variables view show those.  But to be honest, either solution
> is fine with me, as long as we are consistent.
> 
> What do you say?
I agree that we should fix the variables view for this.  Please open a new bug for that purpose or rename/retarget this one after you commit your change.
Comment 6 Marc Khouzam CLA 2008-07-16 15:45:15 EDT
(In reply to comment #5)
> I agree(In reply to comment #3)
> > If the user does expect the top-frame value when selecting a thread, then I
> > vote to have the variables view show those.  But to be honest, either solution
> > is fine with me, as long as we are consistent.
> > 
> > What do you say?
> I agree that we should fix the variables view for this.  Please open a new bug
> for that purpose or rename/retarget this one after you commit your change.
> 

I opened Bug 241147 for the variables view.

I committed the patch for the expression service to act as if the top frame is selected when a thread is selected.

Another patch is coming for the selecting a process context.
Comment 7 Marc Khouzam CLA 2008-07-17 09:23:43 EDT
Created attachment 107730 [details]
Suggest approach for Process context

I suggest that when a process is selected, the ExpressionService gives: "Invalid context for evaluating expressions".
The Expressions View should go blank as in CDI but with this patch it would at least show the error message.  I like this behavior better than the random behavior of today.

I cannot check this in because it breaks all the memory tests which I think cheat a bit and use the control context all the time.  I will fix those first and then check things in, if I get no objection to this approach.
Comment 8 Pawel Piech CLA 2008-07-17 13:06:17 EDT
(In reply to comment #7)
> I suggest that when a process is selected, the ExpressionService gives:
> "Invalid context for evaluating expressions".
> The Expressions View should go blank as in CDI but with this patch it would at
> least show the error message.  I like this behavior better than the random
> behavior of today.
Logically, user should be able to evaluate expressions against the global symbol data of the process, so this seems like a limitation of GDB itself.  If the view going blank behavior is important enough, we could extend the variables VM node in the GDB plugins to accomplish this behavior.

> ..., if I get no objection to this approach.
No objections here.