[PATCH] Fix for PR breakpoints/16392: info prog after failure to insert breakpoint can cause gdb crash

[Sergio Durigan Junior <sergiodj at redhat dot com>]

Hi,

This patch fixes PR breakpoints/16392.  The failure happens when one
inserts a breakpoint at an invalid address, runs the inferior (thus
receiving a warning by GDB), and then issues a "info program" command
after the warning.

What happens when the user runs the inferior is:

proceed -> insert_breakpoints -> insert_breakpoint_locations

And the error actually happens on insert_breakpoint_locations.  When it
notices that the breakpoint failed to be inserted, it calls error_stream
(in order to print the warning), which in turn calls error and aborts
the proceed call.  This leaves GDB in a bad state, because there was
much more thing to be done after the breakpoint insertion.

I had some alternatives to deal with this.  The first one was to somehow
give up proceeding the inferior at all, and maybe kill it.  I didn't
like this approach because I thought it would be a drastic solution to a
not-so-urgent problem.  Another alternative would be to just call
warning from insert_breakpoint_locations (instead of error; I actually
had a patch implementing a new "warning_stream" function), and then stop
throwing the error.  However, other functions are tailored expecting an
error to be thrown from that function (more on that later).  Therefore,
the last alternative seemed like the better one: to handle the exception
from proceed, and continue the function even if there is an error
inserting the breakpoint.

I implemented the latter, and noticed that GDB was still stopping after
it noticed that the breakpoint was invalid.  I was kind of surprised
because I was expecting the inferior to continue running, which would
force the user to do a C-c if she wanted to issue a "info prog".  Also,
when I first implemented the "warning_stream" idea, the inferior
actually continued running...  Well, time for a little more
investigation.

I noticed that keep_going also calls insert_breakpoints, but that it
actually *checks* for the exception, and doesn't keep going if there was
some error.  Therefore, the modus operandi for GDB doesn't change with
this patch (i.e., the inferior will still be stopped if the breakpoint
is invalid), but now the user can inspect the program with "info prog".

I hope I managed to cover everything here.  This patch also includes a
testcase for the bug, and does not introduce any regressions on x86_64
Fedora 17.

OK to apply?

-- 
Sergio

gdb/
2014-01-19  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR breakpoints/16392
	* infrun.c (proceed): Catch the exception from insert_breakpoints.

gdb/testsuite/
2014-01-19  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR breakpoints/16392
	* gdb.base/break-invalid-addr-info-prog.exp: New file.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 51540b3..e91e8c0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2247,7 +2247,22 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
      or if we are stepping over one but we're using displaced stepping
      to do so.  */
   if (! tp->control.trap_expected || use_displaced_stepping (gdbarch))
-    insert_breakpoints ();
+    {
+      volatile struct gdb_exception ex;
+
+      /* We are interested in catching this exception (thrown by
+	 insert_breakpoint_locations), but we will not make anything
+	 with it, because we want to continue with the proceed.  If we
+	 stop now, and we are dealing with the initialization of the
+	 inferior, then GDB will be end in a bad internal state and
+	 will not be able to handle the inferior later (see PR
+	 breakpoints/16392). This exception will be caught again later
+	 on keep_going, and then it will do the right thing there.  */
+      TRY_CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  insert_breakpoints ();
+	}
+    }
 
   if (!non_stop)
     {
diff --git a/gdb/testsuite/gdb.base/break-invalid-addr-info-prog.exp b/gdb/testsuite/gdb.base/break-invalid-addr-info-prog.exp
new file mode 100644
index 0000000..0b0bcf6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-invalid-addr-info-prog.exp
@@ -0,0 +1,33 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test if, after setting a breakpoint in an invalid address in the
+# inferior and running it (thus causing GDB to display the error), the
+# command "info program" works OK (see PR breakpoints/16392).
+
+standard_testfile normal.c
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } then {
+    return -1
+}
+
+gdb_test "break *0xdeadbeef" "Breakpoint 1 at 0xdeadbeef" \
+    "Break at invalid address"
+
+gdb_test "run" "Starting program: .*\[\r\n\]+Warning:\[\r\n\]+Cannot insert breakpoint 1\.\[\r\n\]+Cannot access memory at address 0xdeadbeef.*" \
+    "Running the inferior"
+
+gdb_test "info program" "\tUsing the running image of child process $decimal\.\[\r\n\]+Program stopped at $hex\.\[\r\n\]+It stopped with signal SIGTRAP, Trace/breakpoint trap." \
+    "info program works"