[RFC/commit] parse breakpoint condition according to breakpoint location

[Joel Brobecker <brobecker at adacore dot com>]

Hello,

This is a patch that was actually approved back in 2003:

    http://www.sourceware.org/ml/gdb-patches/2003-09/msg00195.html

... but I never got to commmit it because I discovered right after
that it exposed a flaw in the current design which triggered a problem
elsewhere. An improved version of the patch was then sent:

    http://www.sourceware.org/ml/gdb-patches/2003-09/msg00225.html

... But at the same time, the discussion digressed over the use of
a global. I proposed a plan for dealing with this, but it's a huge
change (adding a language parameter to the parse_* routines), and
I don't think I got feedback on how I was planning to proceed.

Summary of the problem: 

We're debugging a multi-language application, and just stopped inside
some C code. For instance:

    (gdb) b c_function
    Breakpoint 1 at 0x4029c8: file foo.c, line 6.
    (gdb) run
    Starting program: /[...]/a

    Breakpoint 1, c_function () at foo.c:6
    6         callme ();
    (gdb) show lang
    The current source language is "auto; currently c".

The current language is C, but we want to insert a condition breakpoint
inside some Ada Code. But the problem is that the breakpoint condition
gets evaluated using the current language, and this usually triggers
an error.  For instance:

    (gdb) b mixed.adb:15 if light = green
    No symbol "green" in current context.

So the decision was, if the language is auto, to use the language
corresponding to the breakpoint location, not the current frame.
This is what this patch implements.

gdb/
2010-05-09  Joel Brobecker  <brobecker@adacore.com>

        * parse.c (parse_exp_in_context): When block is not NULL, use
        its associated language to parse the expression instead of
        the current_language.

gdb/testsuite/
2010-05-09  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/cond_lang: New testcase.

Tested on x86_64-linux. No regression.  This has also been in AdaCore's
tree since 2003, and we haven't had problems with it.

For the longer term, this approach is far from perfect, since it depends
on the current_language global, which may change over time.  For instance,
if the user switches from language auto to language c between the moment
he creates the breakpoint and the moment he runs the program, the condition
will get re-evaluated using C the next time around.  But I think that
this is already quite a good useability improvement and the only part of
the logic that could move elsewhere is the part that computes the language
to use, and that's easily moved out.   So it's not making further refinments
any harder in the future.

Hence the suggestion to include this patch as is.  I can work on cleaning
things up as a separate patch. I will send a followup mail on that. In
the meantime, any objection to this change?

Thanks,
-- 
Joel

---
 gdb/parse.c                               |   35 +++++++++++++++--
 gdb/testsuite/gdb.ada/cond_lang.exp       |   58 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/cond_lang/a.adb     |   21 ++++++++++
 gdb/testsuite/gdb.ada/cond_lang/foo.c     |   25 ++++++++++++
 gdb/testsuite/gdb.ada/cond_lang/mixed.adb |   49 ++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/cond_lang/mixed.ads |   20 ++++++++++
 gdb/testsuite/gdb.ada/cond_lang/pck.adb   |   21 ++++++++++
 gdb/testsuite/gdb.ada/cond_lang/pck.ads   |   20 ++++++++++
 8 files changed, 245 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang.exp
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang/a.adb
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang/foo.c
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang/mixed.adb
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang/mixed.ads
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/cond_lang/pck.ads

diff --git a/gdb/parse.c b/gdb/parse.c
index 7db6e92..e7e5145 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -1070,6 +1070,7 @@ parse_exp_in_context (char **stringptr, struct block *block, int comma,
 {
   volatile struct gdb_exception except;
   struct cleanup *old_chain;
+  const struct language_defn *lang = NULL;
   int subexp;
 
   lexptr = *stringptr;
@@ -1107,17 +1108,43 @@ parse_exp_in_context (char **stringptr, struct block *block, int comma,
 	expression_context_pc = BLOCK_START (expression_context_block);
     }
 
+  if (language_mode == language_mode_auto && block != NULL)
+    {
+      /* Find the language associated to the given context block.
+         Default to the current language if it can not be determined.
+
+         Note that using the language corresponding to the current frame
+         can sometimes give unexpected results.  For instance, this
+         routine is often called several times during the inferior
+         startup phase to re-parse breakpoint expressions after
+         a new shared library has been loaded.  The language associated
+         to the current frame at this moment is not relevant for
+         the breakpoint. Using it would therefore be silly, so it seems
+         better to rely on the current language rather than relying on
+         the current frame language to parse the expression. That's why
+         we do the following language detection only if the context block
+         has been specifically provided.  */
+      struct symbol *func = block_linkage_function (block);
+
+      if (func != NULL)
+        lang = language_def (SYMBOL_LANGUAGE (func));
+      if (lang == NULL || lang->la_language == language_unknown)
+        lang = current_language;
+    }
+  else
+    lang = current_language;
+
   expout_size = 10;
   expout_ptr = 0;
   expout = (struct expression *)
     xmalloc (sizeof (struct expression) + EXP_ELEM_TO_BYTES (expout_size));
-  expout->language_defn = current_language;
+  expout->language_defn = lang;
   expout->gdbarch = get_current_arch ();
 
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      if (current_language->la_parser ())
-	current_language->la_error (NULL);
+      if (lang->la_parser ())
+        lang->la_error (NULL);
     }
   if (except.reason < 0)
     {
@@ -1150,7 +1177,7 @@ parse_exp_in_context (char **stringptr, struct block *block, int comma,
   if (out_subexp)
     *out_subexp = subexp;
 
-  current_language->la_post_parser (&expout, void_context_p);
+  lang->la_post_parser (&expout, void_context_p);
 
   if (expressiondebug)
     dump_prefix_expression (expout, gdb_stdlog);
diff --git a/gdb/testsuite/gdb.ada/cond_lang.exp b/gdb/testsuite/gdb.ada/cond_lang.exp
new file mode 100644
index 0000000..dcd46da
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang.exp
@@ -0,0 +1,58 @@
+# Copyright 2010 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "cond_lang"
+set testfile "${testdir}/a"
+set cfile "${testdir}/foo"
+set adasrcfile ${srcdir}/${subdir}/${testfile}.adb
+set csrcfile ${srcdir}/${subdir}/${cfile}.c
+set cobject  ${objdir}/${subdir}/${cfile}.o
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+gdb_compile "${csrcfile}" "${cobject}" object [list debug]
+if {[gdb_compile_ada "${adasrcfile}" "${binfile}" executable [list debug]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+# Run to c_function an verify that the language automatically gets set to C.
+runto c_function
+gdb_test "show lang" \
+         "The current source language is \"auto; currently c\"\\."
+
+# Now, insert a breakpoint inside an Ada unit, using a condition written
+# in Ada. Even though the current language is "auto; currently c", we
+# expect the debugger to parse the expression using Ada, because the
+# current language mode is auto, and the breakpoint is inside Ada code.
+set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb]
+gdb_test "break mixed.adb:${bp_location} if light = green" \
+         "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\."
+
+# Now, continue until we hit the breakpoint.  If the condition is
+# evaluated correctly, the first hit will be ignored, and the debugger
+# will stop at the second hit only, when the "light" argument is equal
+# to green.
+gdb_test "continue" \
+         "Breakpoint \[0-9\]*, mixed\\.break_me \\(light=green\\) at .*"
+
+
diff --git a/gdb/testsuite/gdb.ada/cond_lang/a.adb b/gdb/testsuite/gdb.ada/cond_lang/a.adb
new file mode 100644
index 0000000..e1cda16
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang/a.adb
@@ -0,0 +1,21 @@
+--  Copyright 2010 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/>.
+
+with Mixed;
+
+procedure A is
+begin
+   Mixed.Start_Test;
+end A;
diff --git a/gdb/testsuite/gdb.ada/cond_lang/foo.c b/gdb/testsuite/gdb.ada/cond_lang/foo.c
new file mode 100644
index 0000000..fe27fc9
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang/foo.c
@@ -0,0 +1,25 @@
+/* Copyright 2010 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+extern callme (void);
+
+void
+c_function (void)
+{
+  callme ();
+}
+
diff --git a/gdb/testsuite/gdb.ada/cond_lang/mixed.adb b/gdb/testsuite/gdb.ada/cond_lang/mixed.adb
new file mode 100644
index 0000000..1d5fd31
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang/mixed.adb
@@ -0,0 +1,49 @@
+--  Copyright 2010 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/>.
+
+with Pck; use Pck;
+
+package body Mixed is
+   --  We are importing symbols from foo.o, so make sure this object file
+   --  gets linked in.
+   Pragma Linker_Options ("foo.o");
+
+   type Color is (Red, Green, Blue);
+
+   procedure C_Function;
+   pragma Import (C, C_Function, "c_function");
+
+   procedure Callme;
+   pragma Export (C, Callme, "callme");
+
+   procedure Break_Me (Light : Color) is
+   begin
+      Put_Line ("Light: " & Color'Image (Light));  --  STOP
+   end Break_Me;
+
+   procedure Callme is
+   begin
+      Break_Me (Red);
+      Break_Me (Green);
+      Break_Me (Blue);
+   end Callme;
+
+   procedure Start_Test is
+   begin
+      --  Call C_Function, which will call Callme.
+      C_Function;
+   end Start_Test;
+
+end Mixed;
diff --git a/gdb/testsuite/gdb.ada/cond_lang/mixed.ads b/gdb/testsuite/gdb.ada/cond_lang/mixed.ads
new file mode 100644
index 0000000..b3a712a
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang/mixed.ads
@@ -0,0 +1,20 @@
+--  Copyright 2010 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/>.
+
+package Mixed is
+
+   procedure Start_Test;
+
+end Mixed;
diff --git a/gdb/testsuite/gdb.ada/cond_lang/pck.adb b/gdb/testsuite/gdb.ada/cond_lang/pck.adb
new file mode 100644
index 0000000..d412fec
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang/pck.adb
@@ -0,0 +1,21 @@
+--  Copyright 2010 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/>.
+
+package body Pck is
+   procedure Put_Line (S : String) is
+   begin
+      null;
+   end Put_Line;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/cond_lang/pck.ads b/gdb/testsuite/gdb.ada/cond_lang/pck.ads
new file mode 100644
index 0000000..4fbc2a2
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/cond_lang/pck.ads
@@ -0,0 +1,20 @@
+--  Copyright 2010 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/>.
+
+package Pck is
+   procedure Put_Line (S : String);
+   --  Stub implementation of Put_Line to avoid a dependency on Text_IO.
+   --  Does actually nothing.
+end Pck;
-- 
1.6.3.3