Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 367256

Summary: Debugger doesn't handle invalid breakpoints properly
Product: [Tools] CDT Reporter: Nobody - feel free to take it <nobody>
Component: cdt-debug-dsf-gdbAssignee: Nobody - feel free to take it <nobody>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: branko.drevensek, cdtdoug, jonah, malaperle, mike.wrighton, pawel.1.piech
Version: 8.0   
Target Milestone: 9.3.0   
Hardware: PC   
OS: Linux   
See Also: https://git.eclipse.org/r/85888
https://git.eclipse.org/r/#/c/6082/
https://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=cf51cb6b13aae738151452197ea40bc5d4927b61
Whiteboard:
Bug Depends on:    
Bug Blocks: 388260    
Attachments:
Description Flags
GDB error handler patch none

Description Nobody - feel free to take it CLA 2011-12-20 17:49:17 EST
To reproduce, start a debug session and type "break *0x0" in the GDB console. After that resuming or stepping doesn't have any effect - the stack frame in the Debug view remains the same until the breakpoint is removed.

Here is the gdb trace:

877,073 22-interpreter-exec console "break *0x0"
877,078 ~"Breakpoint 4 at 0x0\n"
877,096 22^done
877,097 (gdb) 
882,679 23-exec-continue --thread 1
882,687 23^error,msg="Warning:\nCannot insert breakpoint 4.\nError accessing memory address 0x0: Input/output error.\n"
882,698 (gdb) 

The CDI handles this a little bit better by popping up an error message.
Comment 1 Nobody - feel free to take it CLA 2012-03-16 16:06:25 EDT
*** Bug 374547 has been marked as a duplicate of this bug. ***
Comment 2 Mike Wrighton CLA 2012-04-09 14:57:27 EDT
Created attachment 213766 [details]
GDB error handler patch

The attached patch displays an error dialog in the following cases:

* A GDB run or step command fails.
* GDB reports an OOB breakpoint error. Example: error during internal step over temporary breakpoint.
* GDB reports an error as a second result record i.e. ^running followed by ^error. Example: invalid breakpoint is hit on run command, during startup.

A hook is included in the form of ICommandError which allow clients to override MI commands so that they can parse the output and return a custom, structured error message (a summary string and a list of detail strings).
Comment 3 Nobody - feel free to take it CLA 2012-05-04 09:39:54 EDT
*** Bug 378468 has been marked as a duplicate of this bug. ***
Comment 4 Nobody - feel free to take it CLA 2012-05-22 18:17:42 EDT
I've modified Mike's patch and pushed it to Gerrit - https://git.eclipse.org/r/6082. 
Marc, can you please review it?
The simple way to reproduce the main cases is to set a breakpoint in the disassembly view at 0x0. The breakpoint will be added to the Breakpoints view and will be picked when the session is started, so we can test the case when the breakpoint is inserted at launch. 
After the error occurs, disable the breakpoint and press "resume". When the session is suspended, enable the breakpoint and try to resume the session again. Various run control operations can be tested by repeating this sequence.
There is a special case, to reproduce it 
- disable the invalid breakpoint before starting the session 
- uncheck "Stop at main" option 
- set a "normal" breakpoint somewhere in the code
- start the session 
- when the "normal" breakpoint hit enable the invalid breakpoint and step.
Comment 5 Nobody - feel free to take it CLA 2012-05-22 18:26:52 EDT
One more thing. The error messages reported by GDB in this case are very confusing. We added a a couple of new API methods that would allow us to modify these messages in our product.
Comment 6 Marc Khouzam CLA 2012-05-24 11:18:40 EDT
(In reply to comment #4)

Thanks for the details on how to reproduce.

I don't understand why the below is a special case? 

> There is a special case, to reproduce it 
> - disable the invalid breakpoint before starting the session 
> - uncheck "Stop at main" option 
> - set a "normal" breakpoint somewhere in the code
> - start the session 
> - when the "normal" breakpoint hit enable the invalid breakpoint and step.


(In reply to comment #5)
> One more thing. The error messages reported by GDB in this case are very
> confusing. We added a a couple of new API methods that would allow us to modify
> these messages in our product.

I agree that the errors from GDB are not very helpful.  Why not add your better error messages to CDT?  Are they product-specific?  Can you adapt them to CDT, or at least sumbit one version for CDT, and still override with your own?
Comment 7 Nobody - feel free to take it CLA 2012-05-24 12:27:47 EDT
(In reply to comment #6)
> (In reply to comment #4)
> 
> Thanks for the details on how to reproduce.
> 
> I don't understand why the below is a special case? 
> 
> > There is a special case, to reproduce it 
> > - disable the invalid breakpoint before starting the session 
> > - uncheck "Stop at main" option 
> > - set a "normal" breakpoint somewhere in the code
> > - start the session 
> > - when the "normal" breakpoint hit enable the invalid breakpoint and step.
>

I mentioned this case because it is handled by a different code than other cases, so it has to be tested separately. Who would think that setting a breakpoint at main is different than setting a breakpoint somewhere else in the code. I don't know why this happens and let Mike comment on it.

> 
> (In reply to comment #5)
> > One more thing. The error messages reported by GDB in this case are very
> > confusing. We added a a couple of new API methods that would allow us to modify
> > these messages in our product.
> 
> I agree that the errors from GDB are not very helpful.  Why not add your better
> error messages to CDT?  Are they product-specific?  Can you adapt them to CDT,
> or at least sumbit one version for CDT, and still override with your own?

I don't think there is something specific to our product in our version and am sure we can submit it to CDT. Again Mike is the right person to comment.
Comment 8 Mike Wrighton CLA 2012-05-24 15:18:14 EDT
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #4)
> > 
> > Thanks for the details on how to reproduce.
> > 
> > I don't understand why the below is a special case? 
> > 
> > > There is a special case, to reproduce it 
> > > - disable the invalid breakpoint before starting the session 
> > > - uncheck "Stop at main" option 
> > > - set a "normal" breakpoint somewhere in the code
> > > - start the session 
> > > - when the "normal" breakpoint hit enable the invalid breakpoint and step.
> >
> 
> I mentioned this case because it is handled by a different code than other
> cases, so it has to be tested separately. Who would think that setting a
> breakpoint at main is different than setting a breakpoint somewhere else in the
> code. I don't know why this happens and let Mike comment on it.

That's correct, it's just because the breakpoint on main is set as a temporary breakpoint (-t). It seems that when you stop at a temporary breakpoint, then insert an invalid breakpoint, the next/step/continue command fails correctly. If you stop at a regular breakpoint and do the same, you get 2 result records - the second result record is an error and is caught by the call handleAsyncDebugError.

> 
> > 
> > (In reply to comment #5)
> > > One more thing. The error messages reported by GDB in this case are very
> > > confusing. We added a a couple of new API methods that would allow us to modify
> > > these messages in our product.
> > 
> > I agree that the errors from GDB are not very helpful.  Why not add your better
> > error messages to CDT?  Are they product-specific?  Can you adapt them to CDT,
> > or at least sumbit one version for CDT, and still override with your own?
> 
> I don't think there is something specific to our product in our version and am
> sure we can submit it to CDT. Again Mike is the right person to comment.

In our product we override certain commands like MIExecContinue, and parse the GDB error string in the getResult() method, returning an extended MIInfo object which has some structured error information (a title, some details and a list of sub errors). Our status handler then displays this error information, if available. We decided not to include this in the patch but I suppose we could review it, or implement a simpler mechanism for CDT?
Comment 9 Branko Drevensek CLA 2014-03-14 10:26:29 EDT
How can I help to get this into Luna?
Comment 10 Marc Khouzam CLA 2014-03-14 11:30:02 EDT
(In reply to Branko Drevensek from comment #9)
> How can I help to get this into Luna?

I believe there were comments on gerrit:
https://git.eclipse.org/r/6082
Can you confirm they have been addressed?
Comment 11 Marc Khouzam CLA 2014-03-14 11:30:27 EDT
Comment on attachment 213766 [details]
GDB error handler patch

Obsoleting patch since it was posted on gerrit.
Comment 12 Nobody - feel free to take it CLA 2014-07-01 00:19:58 EDT
*** Bug 438594 has been marked as a duplicate of this bug. ***
Comment 13 Marc-André Laperle CLA 2014-07-01 00:35:20 EDT
This can also happen easily with watch points, see bug 438594. It's been almost two years now. Would it be a good idea to look into resuming Mike's work?
Comment 14 Nobody - feel free to take it CLA 2014-07-01 13:35:28 EDT
(In reply to Marc-Andre Laperle from comment #13)
> This can also happen easily with watch points, see bug 438594. It's been
> almost two years now. Would it be a good idea to look into resuming Mike's
> work?

Sure. I had spent some time looking at the patch and trying to modify it but don't have time for it now.
Comment 15 Eclipse Genie CLA 2016-11-28 12:32:46 EST
New Gerrit change created: https://git.eclipse.org/r/85888
Comment 16 Nobody - feel free to take it CLA 2016-11-28 12:47:27 EST
I pushed a patch to Gerrit (https://git.eclipse.org/r/85888) that handles the following case:

36-exec-continue --thread 1
36^running
*running,thread-id="all"
(gdb) 
&"Warning:\n"
&"Cannot insert breakpoint 2.\n"
&"Cannot access memory at address 0x0\n"
&"\n"
36^error,msg="Command aborted."
(gdb)

The steps to reproduce the case are the same but the behavior of GDB has changed (I'm have tried with versions 7.11 and 7.12). Now "^error" appears after "^running" and is associated with the latest "exec-*". The downside of this change is that the error message doesn't contain the cause of failure.

Marc or Marc-Andre, do you have time to review the patch?