Splitting debugger in two - specific debugger and common GUI

[MortenMacFly]

Thanks for the report, I'll look at it in a minute

Please see my note in the  (edited) post before yours...

[oBFusCATed]

Edit: What is not clear to me: Why are the BP's handled one time in m_State and another time in m_breakpoints?! There is a funny mixture of what is being used inside debuggergdb. It's not clear to me when which version is right...?!

I didn't want to mess with the plugin code too much. So I settled for some code duplication.

Here is the fix, but maybe I should extract this and the other notifications, so all plugins implement it by default

Code

Index: src/plugins/debuggergdb/debuggergdb.cpp
===================================================================
--- src/plugins/debuggergdb/debuggergdb.cpp     (revision 5809)
+++ src/plugins/debuggergdb/debuggergdb.cpp     (working copy)
@@ -2008,7 +2008,7 @@
             bpFileName.Normalize();
             edFileName.Normalize();
             if (bpFileName.GetFullPath().Matches(edFileName.GetFullPath()))
-                ed->ToggleBreakpoint(bp->line, false);
+                ed->ToggleBreakpoint(bp->line - 1, false);
         }
         // Now check and highlight the active line under debugging
         if (m_State.HasDriver())

[Jenna]

The patch works, but ...

If I add a BP with the context-menu it does not appear in the list, if I then add another BP with a left-click in the margin, both are listed.

[MortenMacFly]

I didn't want to mess with the plugin code too much. So I settled for some code duplication.

I understand why, but I think we seriously need to clean that up. In the past we had watches, breakpoints in the driver and needed to "synch" them with the BP's (in the editors). It was already confusing as the editors have a different indexing (an editor starts with line 1, a BP with line 0). Now that we have watches and BP's duplicated in the debugger, too we need to take care to "synch" 5 instances. This will lead to a lot of errors like Jens is experiencing.

If you realise that the "driver" is actually the instance of the debugger I think it's most plausible to keep the implementation in the "state" and remove it again from the debugger instance.

[MortenMacFly]

If I add a BP with the context-menu it does not appear in the list, if I then add another BP with a left-click in the margin, both are listed.

I fixed that, too and committed both fixes to the branch.

[oBFusCATed]

I understand why, but I think we seriously need to clean that up. In the past we had watches, breakpoints in the driver and needed to "synch" them with the BP's (in the editors). It was already confusing as the editors have a different indexing (an editor starts with line 1, a BP with line 0). Now that we have watches and BP's duplicated in the debugger, too we need to take care to "synch" 5 instances. This will lead to a lot of errors like Jens is experiencing.

If you realise that the "driver" is actually the instance of the debugger I think it's most plausible to keep the implementation in the "state" and remove it again from the debugger instance.

Are you sure about that the editor starts at 1, my observations are that it starts at 0.
I think, I made the debugger breakpoints to start from 1.

For me the gdb/cdb debuggers should be split in two separate plugins.
I've started the implementation of the gdb only plugin (using the MI interface).

And another thing: the duplication is needed because the outside world, should not see the implementation details (the temporary breakpoint in the current example)

p.s. this bug are caused by the late redesign of the breakpoints interface I made, so I'm sorry for the cases I've not tested
p.p.s. I can do the redesign to remove the duplication, but I'm sure I'll break more staff than that cause by the duplication

[mariocup]

Hi oBFusCATed,

I just had a look at your debugger modifications and the watch window is nice. I would prefer the same view also for the CPU register window  (with switching of the display format too) with the possibility to edit values.
For embedded controller I see the necessity to edit values of the CPU registers or memory (view) etc. Do you intend to modify also these components.

[oBFusCATed]

I could do it some day, but I've never used the CPU registers window, so I have very little motivation to do it.
I'm using the memory window more often, so there is bigger chance for improvements there.

But you never, know if I have nothing to do I can do it.
Also I think the modifications should be made by a person using the window, because he/she would know what is important and what not.

Best regards...

[oBFusCATed]

Here is a patch that fixes the parsing the GDB output, generated by "output var", when "var" is a reference.

Code

Index: src/plugins/debuggergdb/parsewatchvalue.cpp
===================================================================
--- src/plugins/debuggergdb/parsewatchvalue.cpp (revision 5826)
+++ src/plugins/debuggergdb/parsewatchvalue.cpp (working copy)
@@ -314,12 +314,20 @@
         return true;
     }
 
-    if(value[0] == wxT('{') && value[value.length() - 1] == wxT('}'))
+    // Try to find the first brace.
+    // If the watch is for a reference the brace is not at position = 0
+    wxString::size_type start = value.find(wxT('{'));
+
+    if(start != wxString::npos && value[value.length() - 1] == wxT('}'))
     {
-        int start = 1;
-        bool result = ParseGDBWatchValue(watch, value, start, value.length() - 2);
+        int t_start = start + 1;
+        bool result = ParseGDBWatchValue(watch, value, t_start, value.length() - 2);
         if(result)
+        {
+            if(start > 0)
+                watch.SetValue(value.substr(0, start));
             watch.RemoveChildren(true);
+        }
         return result;
     }
     else

[oBFusCATed]

Here is another patch, that tries to fix various breakpoint problems and little bugs

http://smrt.is-a-geek.org/codeblocks/dbg_refactor/fix_breakpoints.patch

[MortenMacFly]

http://smrt.is-a-geek.org/codeblocks/dbg_refactor/fix_breakpoints.patch

Mmmmh... I cannot apply this patch. Nearly everything is rejected. Did you use svn diff to create the patch? If not, please try that.

Just do:
svn diff > the_patch.patch
...in the SVN controlled root folder of C::B.

[eranif]

Here is another patch, that tries to fix various breakpoint problems and little bugs

http://smrt.is-a-geek.org/codeblocks/dbg_refactor/fix_breakpoints.patch

Wanted to give you a "heads-up":
The debugger wont work as expected on MacOSX since GDB/MI returns different output then Linux/Windows (the format is a bit different)
So you might want to test it on Mac as well to fix the parsing.

Eran

[MortenMacFly]

So you might want to test it on Mac as well to fix the parsing.

Good catch, Eran. Hence we have literally no active developer that owns a MAC... so it will be a hard task.

[oBFusCATed]

Mmmmh... I cannot apply this patch. Nearly everything is rejected. Did you use svn diff to create the patch? If not, please try that.

Just do:
svn diff > the_patch.patch
...in the SVN controlled root folder of C::B.

Arghhh.... the patch is created with svn diff and applies correctly on my machine (I have a gentoo ebuild (package) for svn that uses it)

Wanted to give you a "heads-up":
The debugger wont work as expected on MacOSX since GDB/MI returns different output then Linux/Windows (the format is a bit different)
So you might want to test it on Mac as well to fix the parsing.

Eran

Thanks, Eran I plan to install hackosx 10.6 when there is a proper distro for my machine, so I hope I can do some testing

[ollydbg]

Code

-------------- Build: Debugger in Code::Blocks ---------------

mingw32-g++.exe -Wall -g -pipe -mthreads -fmessage-length=0 -fexceptions -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE  -DBUILDING_PLUGIN -DWXMAKINGDLL_PROPGRID -DwxPG_USE_WXMODULE=0    -IC:\wxWidgets-2.8.10\include -IC:\wxWidgets-2.8.10\contrib\include -IC:\wxWidgets-2.8.10\lib\gcc_dll\mswu -Isdk\wxscintilla\include -Isdk\wxpropgrid\include -Iinclude\tinyxml -Iinclude -Iinclude\scripting\include -Iinclude\scripting\sqplus  -c C:\cb_debugger_branch\src\plugins\debuggergdb\cdb_driver.cpp -o .objs\plugins\debuggergdb\cdb_driver.o
mingw32-g++.exe -Wall -g -pipe -mthreads -fmessage-length=0 -fexceptions -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE  -DBUILDING_PLUGIN -DWXMAKINGDLL_PROPGRID -DwxPG_USE_WXMODULE=0    -IC:\wxWidgets-2.8.10\include -IC:\wxWidgets-2.8.10\contrib\include -IC:\wxWidgets-2.8.10\lib\gcc_dll\mswu -Isdk\wxscintilla\include -Isdk\wxpropgrid\include -Iinclude\tinyxml -Iinclude -Iinclude\scripting\include -Iinclude\scripting\sqplus  -c C:\cb_debugger_branch\src\plugins\debuggergdb\databreakpointdlg.cpp -o .objs\plugins\debuggergdb\databreakpointdlg.o
mingw32-g++.exe -Wall -g -pipe -mthreads -fmessage-length=0 -fexceptions -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE  -DBUILDING_PLUGIN -DWXMAKINGDLL_PROPGRID -DwxPG_USE_WXMODULE=0    -IC:\wxWidgets-2.8.10\include -IC:\wxWidgets-2.8.10\contrib\include -IC:\wxWidgets-2.8.10\lib\gcc_dll\mswu -Isdk\wxscintilla\include -Isdk\wxpropgrid\include -Iinclude\tinyxml -Iinclude -Iinclude\scripting\include -Iinclude\scripting\sqplus  -c C:\cb_debugger_branch\src\plugins\debuggergdb\debugger_defs.cpp -o .objs\plugins\debuggergdb\debugger_defs.o
mingw32-g++.exe -Wall -g -pipe -mthreads -fmessage-length=0 -fexceptions -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE  -DBUILDING_PLUGIN -DWXMAKINGDLL_PROPGRID -DwxPG_USE_WXMODULE=0    -IC:\wxWidgets-2.8.10\include -IC:\wxWidgets-2.8.10\contrib\include -IC:\wxWidgets-2.8.10\lib\gcc_dll\mswu -Isdk\wxscintilla\include -Isdk\wxpropgrid\include -Iinclude\tinyxml -Iinclude -Iinclude\scripting\include -Iinclude\scripting\sqplus  -c C:\cb_debugger_branch\src\plugins\debuggergdb\debuggerdriver.cpp -o .objs\plugins\debuggergdb\debuggerdriver.o
mingw32-g++.exe -Wall -g -pipe -mthreads -fmessage-length=0 -fexceptions -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE  -DBUILDING_PLUGIN -DWXMAKINGDLL_PROPGRID -DwxPG_USE_WXMODULE=0    -IC:\wxWidgets-2.8.10\include -IC:\wxWidgets-2.8.10\contrib\include -IC:\wxWidgets-2.8.10\lib\gcc_dll\mswu -Isdk\wxscintilla\include -Isdk\wxpropgrid\include -Iinclude\tinyxml -Iinclude -Iinclude\scripting\include -Iinclude\scripting\sqplus  -c C:\cb_debugger_branch\src\plugins\debuggergdb\debuggergdb.cpp -o .objs\plugins\debuggergdb\debuggergdb.o
C:\cb_debugger_branch\src\plugins\debuggergdb\debuggergdb.cpp:1748:1: error: unterminated #ifndef
C:\cb_debugger_branch\src\plugins\debuggergdb\debuggergdb.cpp: In member function 'virtual void DebuggerGDB::Break()':
C:\cb_debugger_branch\src\plugins\debuggergdb\debuggergdb.cpp:1747: warning: unused variable 'pid'
C:\cb_debugger_branch\src\plugins\debuggergdb\debuggergdb.cpp:1747: error: expected '}' at end of input
C:\cb_debugger_branch\src\plugins\debuggergdb\debuggergdb.cpp:1747: error: expected '}' at end of input
Process terminated with status 1 (12 minutes, 40 seconds)
3 errors, 9 warnings
 

build failed in the latest debugger branch under TDM mingw.