Bugzilla – Full Text Bug Listing
| Summary: | Patch to fix function breakpoints | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Oyvind Harboe <oyvind.harboe> | ||||||||
| Component: | cdt-debug | Assignee: | Nobody - feel free to take it <nobody> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | cdtdoug | ||||||||
| Version: | 3.1 | ||||||||||
| Target Milestone: | 4.0 RC0 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows 2000 | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Oyvind Harboe
Created attachment 37866 [details]
Fixes function breakpoints
The patch is not "pure" as it contains some changes to make sure that breakpoints outside CDT projects also work.
I need to move on to my chores, so I'm just storing the patch as-is for now in bugzilla.
Created attachment 45676 [details]
Function and address breakpoints work better with this patch
This patch stops CDT from trying to convert function and address breakpoints to line breakpoints(bad! bad!).
This is really annoying when debugging low level code as CDT will incorrectly translate address breakpoints to some willy-nilly source file & line number.
Oftentimes when debugging low level stuff there *is* no translation at all. The address breakpoints can even be some ephemeral code in RAM that is copied from flash to handle flash programming issues and as such the address might have nothing to do with where the (position independent code) is according to the elf file.
Can I request that this fix is marked for CDT 4.0? Thanks. Created attachment 60355 [details]
Patch against CDT 4.0
Seems to work fine, except that I get two breakpoints when double clicking on a source line.
Toma, are you still planning to look at it? If not, reassign it to me. It is in my futures list .. you can have it now if you like. (In reply to comment #6) > It is in my futures list .. you can have it now if you like. Thanks. If the fix is not simple, I'll reassign it back to you :) Oyvind, I don't think this patch is correct. It converts all line breakpoints inside one function to the breakpoint set to the function, doesn't it?
For example,
void foo() {
...
line 1;
line2;
....
}
Breakpoints at line1 and line2 will be converted to the breakpoint at foo.
Ok, moving target milestone to RC0. (In reply to comment #8) > Oyvind, I don't think this patch is correct. It converts all line breakpoints > inside one function to the breakpoint set to the function, doesn't it? > For example, > > void foo() { > ... > line 1; > line2; > .... > } > > Breakpoints at line1 and line2 will be converted to the breakpoint at foo. No. I don't know why, but it doesn't. Anyway: I lack the CDT know how to formulate a patch that is acceptable as is for this problem. I hope that patch provides information and pointers that be used to effectively make progress on the problem though!!! In the version of gdb I'm using (mingw 6.3.2), when you set a line breakpoint, i.e. break file.cpp:56, I do get the function name set along with the file and line number and address. The same is true with a function breakpoint, i.e. break myFunction. It's impossible to tell them apart. At this point, clearly converting a line breakpoint to a function breakpoint is incorrect, which is what your patch does in this case. I'll try to investigate a bit more to see if there are any other clues. (In reply to comment #11) > In the version of gdb I'm using (mingw 6.3.2), when you set a line breakpoint, > i.e. break file.cpp:56, I do get the function name set along with the file and > line number and address. The same is true with a function breakpoint, i.e. > break myFunction. It's impossible to tell them apart. Not so. You can set a breakpoint at an address, label or line. A breakpoint at a label *can* be at the address of a function, but it does not have to be. The GDB manual is not very helpful in clarifying this point. I typically use "break foo" to break at some point during assembly startup. Very important to get right for hardware debuggers(JTAG debuggers is a subset of hardware debuggers). > > At this point, clearly converting a line breakpoint to a function breakpoint is > incorrect, which is what your patch does in this case. I'll try to investigate > a bit more to see if there are any other clues. If the user asked for a line breakpoint, he should get a line breakpoint. If he asked for a label breakpoint, he should get a label breakpoint(ditto for address breakpoint). I guess I wasn't clear. Here's an example:
{number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0040130c",func="resizeWindow(int, int)",file="../lesson05.cpp",line="46",times="0
{number="4",type="breakpoint",disp="keep",enabled="y",addr="0x00401319",func="resizeWindow(int, int)",file="../lesson05.cpp",line="49",times="0"}]}
Here are two breakpoints the first one was set with 'break resizeWindow' and the second was set with 'break lesson05.cpp:49'. I can't tell which one is function and which one is line. Your solution picks function over line, which for the second breakpoint is incorrect. I agree this is a problem, but I need a general solution, or at least one that won't break anything.
(In reply to comment #13) > I guess I wasn't clear. Here's an example: > > {number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0040130c",func="resizeWindow(int, > int)",file="../lesson05.cpp",line="46",times="0 > > {number="4",type="breakpoint",disp="keep",enabled="y",addr="0x00401319",func="resizeWindow(int, > int)",file="../lesson05.cpp",line="49",times="0"}]} > > Here are two breakpoints the first one was set with 'break resizeWindow' and > the second was set with 'break lesson05.cpp:49'. I can't tell which one is > function and which one is line. Your solution picks function over line, which > for the second breakpoint is incorrect. I agree this is a problem, but I need a > general solution, or at least one that won't break anything. Obviously you can't use the return value. You must use the knowledge of which command was sent. Doesn't the object that represent the command parse the result? If so, it could flag the result as a function breakpoint somehow. (In reply to comment #14) Yes, that's probably the only way to fix it. In any case I'll fix the upper layer - currently it guesses what kind of breakpoint is set instead of relying on the information received from the backend. > Doesn't the object that
> represent the command parse the result? If so, it could flag the result as a
> function breakpoint somehow.
Currently, when a breakpoint is set/deleted/modified from the console a change event is fired and the breakpoint table is used to find out what has been changed.
Fixed. Added a new member (hint) to MIBreakpointChangedEvent. The value is calculated by parsing the CLI command. Oyvind, please verify it. Much better.
Note that "break foo" is different from "break *foo". The former will try to look up the address of the function, which goes haywire when using it for machine code labels. "break *foo" is the proper form for breakpoints at machine code routines I think.
break *warm_reset
273-interpreter-exec console "break *warm_reset"
~"Breakpoint 5 at 0x1030080: file /cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c, line 86.\n"
Breakpoint 5 at 0x1030080: file /cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c, line 86.
273^done
(gdb)
274-break-list
274^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="3",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="10",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[bkpt={number="5",type="breakpoint",disp="keep",enabled="y",addr="0x01030080",file="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",fullname="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",line="86",times="0"}]}
(gdb)
c
275 c
&"c\n"
c
~"\nBreakpoint "
Breakpoint ~"5, 0x01030080 in warm_reset () at /cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c:86\n"
5, 0x01030080 in warm_reset () at /cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c:86
~"86\t{\n"
86 {
275^done
(gdb)
276 info threads
&"info threads\n"
&"warning: RMT ERROR : failed to get remote thread list.\n"
276^done
(gdb)
277-stack-info-depth
277^done,depth="2"
(gdb)
278-stack-list-frames 0 2
278^done,stack=[frame={level="0",addr="0x01030080",func="warm_reset",file="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",fullname="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",line="86"},frame={level="1",addr="0x00000ec4",func="??",file="/cygdrive/c/cdtworkspace/arm/gccbuild/arm-elf/libstdc++-v3/include/bits/char_traits.h",line="287"}]
(gdb)
279-data-list-changed-registers
279^done,changed-registers=["0","1","2","3","10","12","15"]
(gdb)
break warm_reset
280-interpreter-exec console "break warm_reset"
~"Breakpoint 6 at 0x1089edc: file /cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c, line 86.\n"
Breakpoint 6 at 0x1089edc: file /cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c, line 86.
280^done
(gdb)
281-break-list
281^done,BreakpointTable={nr_rows="2",nr_cols="6",hdr=[{width="3",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="10",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[bkpt={number="5",type="breakpoint",disp="keep",enabled="y",addr="0x01030080",file="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",fullname="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",line="86",times="1"},bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x01089edc",func="Cyg_Counter",file="/cygdrive/c/workspace/xpo3/firmware/cyvizecos/packages/devs/flash/cyviz/xpo3/current/src/cyviz_xpo3_flash.c",fullname="/ecos-c/workspace/xpo3/firmware/romapp/install/include/cyg/infra/clist.hxx",line="86",times="0"}]}
(gdb)
The only other thing... I'm running with a modification where I let GDB check if a breakpoint is valid so I didn't test with CDT CVS HEAD.
Why should isTargetBreakpoint() ever return anything other than true???
### Eclipse Workspace Patch 1.0
#P org.eclipse.cdt.debug.core
Index: src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java
===================================================================
RCS file: /cvsroot/tools/org.eclipse.cdt/all/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java,v
retrieving revision 1.68
diff -u -r1.68 CBreakpointManager.java
--- src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java 27 Apr 2007 11:11:01 -0000 1.68
+++ src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java 27 Apr 2007 12:03:54 -0000
@@ -24,6 +24,7 @@
import java.util.Set;
import org.eclipse.cdt.core.IAddress;
import org.eclipse.cdt.core.IAddressFactory;
+import org.eclipse.cdt.core.IProjectInfo;
import org.eclipse.cdt.debug.core.CDIDebugModel;
import org.eclipse.cdt.debug.core.CDebugUtils;
import org.eclipse.cdt.debug.core.cdi.CDIException;
@@ -773,7 +774,7 @@
if ( cdiBreakpoint instanceof ICDILineBreakpoint ) {
Object sourceElement = getSourceElement( file );
String sourceHandle = file;
- IResource resource = getProject();
+ IResource resource = getProject().getResource();
if ( sourceElement instanceof IFile ) {
sourceHandle = ((IFile)sourceElement).getLocation().toOSString();
resource = (IResource)sourceElement;
@@ -820,7 +821,7 @@
IPath execFile = getExecFilePath();
String sourceHandle = execFile.toOSString();
ICFunctionBreakpoint breakpoint = CDIDebugModel.createFunctionBreakpoint( sourceHandle,
- getProject(),
+ getProject().getResource(),
cdiBreakpoint.getLocator().getFunction(),
-1,
-1,
@@ -851,7 +852,7 @@
IPath execFile = getExecFilePath();
String sourceHandle = execFile.toOSString();
ICWatchpoint watchpoint = CDIDebugModel.createWatchpoint( sourceHandle,
- getProject(),
+ getProject().getResource(),
cdiWatchpoint.isWriteType(),
cdiWatchpoint.isReadType(),
cdiWatchpoint.getWatchExpression(),
@@ -976,18 +977,12 @@
if ( breakpoint instanceof ICAddressBreakpoint )
return supportsAddressBreakpoint( (ICAddressBreakpoint)breakpoint );
if ( breakpoint instanceof ICLineBreakpoint ) {
- try {
- String handle = breakpoint.getSourceHandle();
- ISourceLocator sl = getSourceLocator();
- if ( sl instanceof ICSourceLocator )
- return ( ((ICSourceLocator)sl).findSourceElement( handle ) != null );
- else if ( sl instanceof CSourceLookupDirector ) {
- return ( ((CSourceLookupDirector)sl).contains( breakpoint ) );
- }
- }
- catch( CoreException e ) {
- return false;
- }
+ /* if this isn't a valid line breakpoint, then GDB will produce an
+ * error message, which is sufficient handling of the problem.
+ *
+ * Also second guessing GDB is fraught with problems.
+ */
+ return true;
}
else {
IProject project = resource.getProject();
@@ -1076,7 +1071,7 @@
return path;
}
- private IProject getProject() {
+ private IProjectInfo getProject() {
return getDebugTarget().getProject();
}
(In reply to comment #18) I was following the gdb docs, "break main" will be presented as a function breakpoint and "break *main" as an address breakpoint. Is this incorrect? (In reply to comment #19) I don't understand the question. Is it related to this bug? If no please raise a new defect. This problem is fixed. I'll send off a new PR if I discover something new & related. |