[PATCH 2/2] 658143 Fails to support multiple breakpoints

[Dodji Seketeli <dodji seketeli org>]

Hello,

Consider this short code snippet:

$ cat -n ~/test.cc
     1    struct S
     2    {
     3        int m_0;
     4        int m_1;
     5
     6        S ()
     7        {
     8            m_0 = 0;
     9            m_1 = 0;
    10        }
    11
    12        S (int a, int b)
    13        {
    14            m_0 = a;
    15            m_1 = b;
    16        }
    17    };
    18
    19    int
    20    main()
    21    {
    22        S s0;
    23        S s1 (2, 3);
    24    }

The behaviour of GDB is that setting a breakpoint to the location "S::S"
(the constructor of S) actually sets two breakpoints; one to line 8 in
the first constructor and one to line 14 in the second constructor.

The problem is that Nemiver doesn't expect that setting one breakpoint
yields could yield multiple breakpoints to be actually set in return.
That situation violates a faulty assertion in the code:

|X|virtual void
nemiver::OnBreakpointHandler::do_handle(nemiver::CommandAndOutput&):/home/dodji/devel/git/nemiver/master/src/dbgengine/nmv-gdb-engine.cc:1452:condition
(bps.size () == 1) failed; raising exception

The patch below changes the callback slot called whenever a breakpoint
is set to make it potentially report that several breakpoint were set.
It also adjust all the parts of the debugger that rely on that callback
slot to make them act accordingly.

Tested an applied to master and gtk2-branch.

From: Dodji Seketeli <dodji seketeli org>
Date: Sun, 4 Sep 2011 10:54:02 +0200
Subject: [PATCH 2/2] 658143 Fails to support multiple breakpoints

	* src/dbgengine/nmv-i-debugger.h
	(IDebugger::breakpoints_set_signal): Renamed
	IDebugger::breakpoint_set_signal into this.  Make it take a map of
	breakpoints instead of just one.
	(IDebugger::set_breakpoint): Change the type of the callback slot
	so that it takes a map of breakpoints.
	* src/dbgengine/nmv-gdb-engine.h
	(GDBEngine::breakpoints_set_signal): Renamed
	IDebugger::breakpoint_set_signal into this.  Make it take a map of
	breakpoints instead of just one.
	(GDBEngine::set_breakpoint) Change the type of the callback slot
	so that it takes a map of breakpoints.
	* src/dbgengine/nmv-gdb-engine.cc
	(GDBEngine::Priv::breakpoints_set_signal): Renamed
	GDBEngine::Priv::breakpoint_set_signal into this.  Change the
	type, make it take a map of breakpoint instead of just one
	breakpoint.
	(OnBreakpointHandler::do_handle): Expect that the result of
	setting a breakpoint can be multiple breakpoints set.  Adjust code
	to the callback slots taking a map of breakpoints instead of just
	one breakpoint.
	(GDBEngine::breakpoints_set_signal): Renamed
	GDBEngine::breakpoint_set_signal into this.  Make it take a map of
	breakpoints instead of just one.
	(GDBEngine::set_breakpoint): Change the type of the callback slot
	so that it takes a map of breakpoints.
	* src/persp/dbgperspective/nmv-breakpoints-view.cc
	(BreakpointsView::Priv::Priv): Adjust.
	(BreakpointsView::Priv::on_debugger_breakpoints_set_signal):
	Renamed BreakpointsView::Priv::on_debugger_breakpoint_set_signal
	into this.  Adjust.
	* src/persp/dbgperspective/nmv-dbg-perspective.cc
	(DBGPerspective::on_break_before_jump)
	(DBGPerspective::on_debugger_breakpoints_set_signal)
	(DBGPerspective::init_debugger_signals): Adjust.
---
 src/dbgengine/nmv-gdb-engine.cc                  |   36 ++++++++++------------
 src/dbgengine/nmv-gdb-engine.h                   |    6 ++--
 src/dbgengine/nmv-i-debugger.h                   |    6 ++--
 src/persp/dbgperspective/nmv-breakpoints-view.cc |   18 ++++++-----
 src/persp/dbgperspective/nmv-dbg-perspective.cc  |   23 +++++++-------
 5 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/src/dbgengine/nmv-gdb-engine.cc b/src/dbgengine/nmv-gdb-engine.cc
index be56da1..29534e6 100644
--- a/src/dbgengine/nmv-gdb-engine.cc
+++ b/src/dbgengine/nmv-gdb-engine.cc
@@ -262,8 +262,8 @@ public:
                          const UString&> breakpoints_list_signal;
 
     mutable sigc::signal<void,
-                         const std::pair<int, const IDebugger::Breakpoint&>&,
-                         const UString& /*cookie*/> breakpoint_set_signal;
+                         const std::map<int, IDebugger::Breakpoint>&,
+                         const UString& /*cookie*/> breakpoints_set_signal;
 
     mutable sigc::signal<void,
                          const vector<OverloadsChoiceEntry>&,
@@ -1444,27 +1444,23 @@ struct OnBreakpointHandler: OutputHandler {
         if (has_breaks
             && (a_in.command ().name () == "set-breakpoint"
                 || a_in.command ().name () == "set-countpoint")) {
-            // If we are getting this reply b/c we did set a
-            // breakpoint, then only one breakpoint should be reported
-            // back to us from GDB.
+            // We are getting this reply b/c we did set a breakpoint;
+            // be aware that sometimes GDB can actually set multiple
+            // breakpoints as a result.
             const map<int, IDebugger::Breakpoint> &bps =
                 a_in.output ().result_record ().breakpoints ();
-            THROW_IF_FAIL (bps.size () == 1);
 
-            std::pair<int,
-                      const IDebugger::Breakpoint&> p (bps.begin ()->first,
-                                                       bps.begin ()->second);
             Command &c = a_in.command ();
             if (c.name () == "set-breakpoint"
                 && c.has_slot ()) {
-                IDebugger::BreakpointSlot slot =
-                    c.get_slot<IDebugger::BreakpointSlot> ();
+                IDebugger::BreakpointsSlot slot =
+                    c.get_slot<IDebugger::BreakpointsSlot> ();
                 LOG_DD ("Calling slot of IDebugger::set_breakpoint()");
-                slot (p);
+                slot (bps);
             }
-            LOG_DD ("Emitting IDebugger::breakpoint_set_signal()");
-            m_engine->breakpoint_set_signal ().emit
-                (p, a_in.command ().cookie ());
+            LOG_DD ("Emitting IDebugger::breakpoints_set_signal()");
+            m_engine->breakpoints_set_signal ().emit
+                (bps, a_in.command ().cookie ());
             m_engine->set_state (IDebugger::READY);
         } else if (a_in.output ().has_result_record ()
             && a_in.output ().result_record ().kind ()
@@ -3412,11 +3408,11 @@ GDBEngine::breakpoints_list_signal () const
 }
 
 sigc::signal<void,
-             const std::pair<int, const IDebugger::Breakpoint&>&,
+             const std::map<int, IDebugger::Breakpoint>&,
              const UString& /*cookie*/>&
-GDBEngine::breakpoint_set_signal () const
+GDBEngine::breakpoints_set_signal () const
 {
-    return m_priv->breakpoint_set_signal;
+    return m_priv->breakpoints_set_signal;
 }
 
 sigc::signal<void, const vector<IDebugger::OverloadsChoiceEntry>&, const UString&>&
@@ -4304,7 +4300,7 @@ GDBEngine::jump_to_position (const Loc &a_loc,
 /// \param a_cookie a string to be passed to
 /// IDebugger::breakpoints_set_signals once that signal emitted as a
 /// result of the breakpoint being set.  Note both a_slot and
-/// IDebugger::breakpoint_set_signals are 'called' upon breakpoint
+/// IDebugger::breakpoints_set_signals are 'called' upon breakpoint
 /// actual setting.  Eventually, IDebugger::breakpoints_set_signals
 /// should be dropped, so this whole cookie business would disapear.
 /// We are still in a transitional period.
@@ -4312,7 +4308,7 @@ void
 GDBEngine::set_breakpoint (const Loc &a_loc,
                            const UString &a_condition,
                            gint a_ignore_count,
-                           const BreakpointSlot &a_slot,
+                           const BreakpointsSlot &a_slot,
                            const UString &a_cookie)
 {
     LOG_FUNCTION_SCOPE_NORMAL_DD;
diff --git a/src/dbgengine/nmv-gdb-engine.h b/src/dbgengine/nmv-gdb-engine.h
index 0285470..a817da8 100644
--- a/src/dbgengine/nmv-gdb-engine.h
+++ b/src/dbgengine/nmv-gdb-engine.h
@@ -73,9 +73,9 @@ public:
                                             breakpoints_list_signal () const;
 
     sigc::signal<void,
-                const std::pair<int, const IDebugger::Breakpoint&>&,
+                const std::map<int, IDebugger::Breakpoint>&,
                 const UString& /*cookie*/>&
-        breakpoint_set_signal () const;
+        breakpoints_set_signal () const;
 
     sigc::signal<void, const vector<OverloadsChoiceEntry>&, const UString&>&
                                     got_overloads_choice_signal () const;
@@ -366,7 +366,7 @@ public:
     void set_breakpoint (const Loc &a_loc,
 			 const UString &a_condition,
 			 gint a_ignore_count,
-			 const BreakpointSlot &a_slot,
+			 const BreakpointsSlot &a_slot,
 			 const UString &a_cookie);
 
     void set_breakpoint (const UString &a_path,
diff --git a/src/dbgengine/nmv-i-debugger.h b/src/dbgengine/nmv-i-debugger.h
index 93a0901..01ef219 100644
--- a/src/dbgengine/nmv-i-debugger.h
+++ b/src/dbgengine/nmv-i-debugger.h
@@ -922,9 +922,9 @@ public:
                                          breakpoints_list_signal () const=0;
 
     virtual sigc::signal<void,
-                        const std::pair<int, const IDebugger::Breakpoint&>&,
+                        const std::map<int, IDebugger::Breakpoint>&,
                         const UString& /*cookie*/>&
-                        breakpoint_set_signal () const = 0;
+                        breakpoints_set_signal () const = 0;
 
     virtual sigc::signal<void,
                          const vector<OverloadsChoiceEntry>&,
@@ -1188,7 +1188,7 @@ public:
     virtual void set_breakpoint (const common::Loc &a_loc,
                                  const UString &a_condition,
                                  gint a_ignore_count,
-                                 const BreakpointSlot &a_slot,
+                                 const BreakpointsSlot &a_slot,
                                  const UString &a_cookie = "") = 0;
 
     virtual void set_breakpoint (const UString &a_path,
diff --git a/src/persp/dbgperspective/nmv-breakpoints-view.cc b/src/persp/dbgperspective/nmv-breakpoints-view.cc
index b47e5b2..700f1be 100644
--- a/src/persp/dbgperspective/nmv-breakpoints-view.cc
+++ b/src/persp/dbgperspective/nmv-breakpoints-view.cc
@@ -122,8 +122,8 @@ public:
         // breakpoints has changed.
         debugger->breakpoint_deleted_signal ().connect (sigc::mem_fun
                 (*this, &Priv::on_debugger_breakpoint_deleted_signal));
-        debugger->breakpoint_set_signal ().connect
-            (sigc::mem_fun (*this, &Priv::on_debugger_breakpoint_set_signal));
+        debugger->breakpoints_set_signal ().connect
+            (sigc::mem_fun (*this, &Priv::on_debugger_breakpoints_set_signal));
         debugger->breakpoints_list_signal ().connect (sigc::mem_fun
                 (*this, &Priv::on_debugger_breakpoints_list_signal));
         debugger->stopped_signal ().connect (sigc::mem_fun
@@ -545,16 +545,18 @@ public:
     }
 
     void
-    on_debugger_breakpoint_set_signal
-    (const std::pair<int, const IDebugger::Breakpoint&> &a,
+    on_debugger_breakpoints_set_signal
+    (const std::map<int, IDebugger::Breakpoint> &a,
      const UString &)
     {
         NEMIVER_TRY;
 
-        LOG_DD ("Adding breakpoint "
-                << a.second.number ());
-
-        append_breakpoint (a.second);
+        std::map<int, IDebugger::Breakpoint>::const_iterator i;
+        for (i = a.begin (); i != a.end (); ++i) {
+            LOG_DD ("Adding breakpoints "
+                    << i->second.number ());
+            append_breakpoint (i->second);
+        }
 
         NEMIVER_CATCH;
     }
diff --git a/src/persp/dbgperspective/nmv-dbg-perspective.cc b/src/persp/dbgperspective/nmv-dbg-perspective.cc
index 2a58254..31fd501 100644
--- a/src/persp/dbgperspective/nmv-dbg-perspective.cc
+++ b/src/persp/dbgperspective/nmv-dbg-perspective.cc
@@ -270,8 +270,8 @@ private:
     void on_jump_to_location_action ();
     void on_jump_to_current_location_action ();
     void on_jump_and_break_to_current_location_action ();
-    void on_break_before_jump (const std::pair<int,
-                                               const IDebugger::Breakpoint&>&,
+    void on_break_before_jump (const std::map<int,
+                                              IDebugger::Breakpoint>&,
                                const Loc &a_loc);
     void on_set_breakpoint_action ();
     void on_set_breakpoint_using_dialog_action ();
@@ -343,8 +343,8 @@ private:
     void on_debugger_command_done_signal (const UString &a_command_name,
                                           const UString &a_cookie);
 
-    void on_debugger_breakpoint_set_signal
-    (const std::pair<int, const IDebugger::Breakpoint&>&, const UString&);
+    void on_debugger_breakpoints_set_signal
+    (const std::map<int, IDebugger::Breakpoint>&, const UString&);
 
     void on_debugger_breakpoints_list_signal
                                 (const map<int, IDebugger::Breakpoint> &,
@@ -1518,8 +1518,7 @@ DBGPerspective::on_jump_and_break_to_current_location_action ()
 /// callback.
 void
 DBGPerspective::on_break_before_jump
-(const std::pair<int,
-                 const IDebugger::Breakpoint&> &,
+(const std::map<int, IDebugger::Breakpoint> &,
  const Loc &a_loc)
 {
     LOG_FUNCTION_SCOPE_NORMAL_DD;
@@ -2347,11 +2346,13 @@ DBGPerspective::on_debugger_command_done_signal (const UString &a_command,
 /// call that triggered this callback.
 ///
 void
-DBGPerspective::on_debugger_breakpoint_set_signal
-(const std::pair<int, const IDebugger::Breakpoint&> &a,
+DBGPerspective::on_debugger_breakpoints_set_signal
+(const std::map<int, IDebugger::Breakpoint> &a,
  const UString&)
 {
-    append_breakpoint (a.second);
+    std::map<int, IDebugger::Breakpoint>::const_iterator i;
+    for (i = a.begin (); i != a.end (); ++i)
+        append_breakpoint (i->second);
 }
 
 void
@@ -3735,8 +3736,8 @@ DBGPerspective::init_debugger_signals ()
     debugger ()->command_done_signal ().connect (sigc::mem_fun
             (*this, &DBGPerspective::on_debugger_command_done_signal));
 
-    debugger ()->breakpoint_set_signal ().connect (sigc::mem_fun
-            (*this, &DBGPerspective::on_debugger_breakpoint_set_signal));
+    debugger ()->breakpoints_set_signal ().connect (sigc::mem_fun
+            (*this, &DBGPerspective::on_debugger_breakpoints_set_signal));
 
     debugger ()->breakpoints_list_signal ().connect (sigc::mem_fun
             (*this, &DBGPerspective::on_debugger_breakpoints_list_signal));
-- 
1.7.6



-- 
		Dodji