Bug 352748

Comment 1 Daniel 2011-07-21 09:55:03 EDT

I don't think that's only a problem for disassambly. You should keep in mind that also control mechanism are failing. For example stepping also not working, because the invalid thread is activated.

Comment 2 Marc Khouzam 2011-07-21 10:31:10 EDT

(In reply to comment #1)
> I don't think that's only a problem for disassambly. You should keep in mind
> that also control mechanism are failing. For example stepping also not working,
> because the invalid thread is activated.

Can you show the 'gdb traces' for that case?  I don't understand exactly what you mean by 'stepping is not working'.  To get the traces:
http://wiki.eclipse.org/CDT/User/FAQ#I.27ve_been_asked_for_.27gdb_traces.27.2C_where_can_I_find_them.3F

Comment 3 Daniel 2011-07-22 02:26:00 EDT

Created attachment 200145 [details]
gdb traces steping on breakpoints

I've got 3 breakpoints inside:
- 1st formal on startup
- 2nd show invalid stepping
- 3rd show stepping with workaround(suspend also last thread)

2nd:
- target becomes running, no step why?

3rd:
- last thread suspend by hand
- target stops on the next line after breakpoint

i've changed some names(functions, paths, vars).

Comment 4 Daniel 2011-07-22 05:07:24 EDT

(In reply to comment #2)
> Can you show the 'gdb traces' for that case?  I don't understand exactly what
> you mean by 'stepping is not working'.  

I've verified it now also with a simple tesp project(using pthread) and you are right, there are no such problems with stepping.

Comment 5 Marc Khouzam 2012-04-18 15:47:00 EDT

The problem is that whenever CDT issues a command using the --thread-group flag, GDB will focus on a random thread.  There is a comment in the GDB MI code saying:

       /* This behaviour means that if --thread-group option identifies
     an inferior with multiple threads, then a random one will be
     picked.  This is not a problem -- frontend should always
     provide --thread if it wishes to operate on a specific
     thread.  */

Previously, (before bug 344298) the IDisassemblyDMContext was the entire GDB (GDBControlDMContext).  In that case, we would not use the --thread-group flag and GDB would simply keep focus on the thread currently in focus, which was a stopped thread (or else the Disassembly view wouldn't display anything anyway).  This was a lucky coincidence that may not have always worked.

But to handle multi-process, we had to move IDisassemblyDMContext to the process level to show the disassembly of the right process.  That is when we started to use --thread-group, which is really what we care about: the process for which we want the disassembly; the thread should not need to be specified since we specify a starting and ending address for the code.  The fact that GDB cannot show disassembly if the currently selected thread is running seems to be a limitation in GDB.

Let's see if we can work around it...

Comment 6 Marc Khouzam 2012-04-18 16:31:30 EDT

Created attachment 214206 [details]
Adding IDisassemblyDMContext to the thread contexts

The only place that calls the IDisassembly service is DisassemblyBackendDsf, which requires a frame to be selected in the debug view.  That means that we always have access to a specific thread when we look for IDisassemblyDMContext.

Therefore, we can actually associate the IDisassemblyDMContext to a thread instead of a process (container), and force GDB to focus on the thread that the user selected.

For API compatibility but also because it make sense that a process be associated with a IDisassemblyDMContext, even if we can't make use of it with GDB, I suggest we leave the containers associated with IDisassemblyDMContext also.

This patch adds IDisassemblyDMContext to each of our three implementations of a thread context (IExecutionDMContext).  It fixes the problem as described in this bug.  The Disassembly JUnit tests pass for GDB 7.4.

Comment 7 Marc Khouzam 2012-04-18 16:41:37 EDT

I've pushed the fix to Gerrit:
https://git.eclipse.org/r/#/c/5624/

Toni, I couldn't find you on Gerrit, but could you review either on Gerrit or the last attached patch?  It is a three line change.

Comment 8 Daniel 2012-04-19 01:28:29 EDT

Thanks for the patch and your investigation. 
Where could i find a nightly build of the module to test the fix?

Comment 9 Anton Leherbauer 2012-04-19 03:36:24 EDT

(In reply to comment #7)
> Toni, I couldn't find you on Gerrit, but could you review either on Gerrit or
> the last attached patch?  It is a three line change.

Hi Marc, I have reviewed and commented on Gerrit (first-time user).

Comment 10 Marc Khouzam 2012-04-19 10:24:08 EDT

(In reply to comment #9)
> Hi Marc, I have reviewed and commented on Gerrit (first-time user).

Thanks for the quick response.

From Gerrit:
> I don't know if there are any extenders of the generic GDB/MI 
> services other than the GDB one. If they exist they might be 
> affected, but I doubt it.

Do you mean that their own implementation may have the same problem and they would need to fix it themselves, or do you mean that this change may affect their behavior?  For the former there isn't much we can do :), and for the latter, yes, it would impact them, but hopefully for the better.

Comment 11 Marc Khouzam 2012-04-19 10:25:47 EDT

(In reply to comment #8)
> Thanks for the patch and your investigation. 
> Where could i find a nightly build of the module to test the fix?

I've just pushed the fix to master and started a build.
You can get that build once it is finished here:
https://hudson.eclipse.org/hudson/job/cdt-nightly/1054/

Comment 12 Marc Khouzam 2012-04-19 10:26:47 EDT

Sigh.  Toni, I screwed up the review flag.  Can you mark as approved again?  Sorry.

Comment 13 Anton Leherbauer 2012-04-19 10:40:23 EDT

(In reply to comment #10)
> Do you mean that their own implementation may have the same problem and they
> would need to fix it themselves, or do you mean that this change may affect
> their behavior?  For the former there isn't much we can do :), and for the
> latter, yes, it would impact them, but hopefully for the better.

I was referring to the latter case.
One problem I could imagine for extenders of MIRunControl is that a thread (MIExecutionDMC) might actually not be a valid disassembly context.  But this seems rather unlikely.

Comment 14 Marc Khouzam 2012-04-19 11:18:39 EDT

(In reply to comment #13)
> (In reply to comment #10)
> > Do you mean that their own implementation may have the same problem and they
> > would need to fix it themselves, or do you mean that this change may affect
> > their behavior?  For the former there isn't much we can do :), and for the
> > latter, yes, it would impact them, but hopefully for the better.
> 
> I was referring to the latter case.
> One problem I could imagine for extenders of MIRunControl is that a thread
> (MIExecutionDMC) might actually not be a valid disassembly context.  But this
> seems rather unlikely.

I guess that is a possibility, in which case we would need to move the IDisassemblyDMContext to a GDB-specific context implementation.  However, as you say, I don't believe anyone is using GDB/MI without GDB anymore (it was only WindRiver, as far as I know).