From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4500 invoked by alias); 9 May 2010 17:58:29 -0000 Received: (qmail 4490 invoked by uid 22791); 9 May 2010 17:58:27 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 09 May 2010 17:58:20 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D62F32BABBB for ; Sun, 9 May 2010 13:58:18 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id MR3RSvRoDGKd for ; Sun, 9 May 2010 13:58:18 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 609A12BABB6 for ; Sun, 9 May 2010 13:58:18 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 4D84DF58F9; Sun, 9 May 2010 10:58:16 -0700 (PDT) From: Joel Brobecker To: gdb-patches@sourceware.org Subject: [RFC/commit] parse breakpoint condition according to breakpoint location Date: Sun, 09 May 2010 17:58:00 -0000 Message-Id: <1273427894-4461-1-git-send-email-brobecker@adacore.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-05/txt/msg00221.txt.bz2 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 * 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 * 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 . + +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 . + +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 . */ + +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 . + +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 . + +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 . + +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 . + +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