[ping] In MI mode -var-create --language not working.

Message ID AC542571535E904D8E8ADAE745D60B1944430663@IRSMSX104.ger.corp.intel.com
State New, archived
Headers

Commit Message

Walfred Tedeschi June 18, 2015, 12:53 p.m. UTC
  Pedro,

From the MI side Vladimir reported to be ok with the change.

May I push?


Thanks and regards,
-Fred

-----Original Message-----
From: Tedeschi, Walfred
Sent: Thursday, April 23, 2015 11:37 AM
To: palves@redhat.com; vladimir@codesourcery.com
Cc: gdb-patches@sourceware.org; Tedeschi, Walfred
Subject: [PATCH v2] In MI mode -var-create --language not working.

Pedro,

I have looked for potential users of the same fix, could not find though.
Though I was suspicious about the a routine prepare_re_set_context on the breakpoint file.  However there was no clean-up routine there what made me think that the code was already taking care of language on the right way.

All other changes should have been addressed.

Thanks a lot for your review and sorry for the delay.

Regards,
-Fred


Trying to use --language to create a variable showed that --language was not always working.  In some cases GDB understands that the language is automatic and cannot parse the language set while executing the command.
In order to fix this just before executing the command language mode should be set to manual.  In this way GDB can parse the expression using the language given in the command.
To do so mode has temporarily to be changed to manual expressing that any evaluation done using the language parameter has priority over the automatic one.
Tests are also included doing evaluations when the default language is c/c++ and when default language is Fortran.

2014-07-25  Walfred Tedeschi  <walfred.tedeschi@intel.com>

	* utils.c (saved_language_and_mode): New.
	(do_restore_current_language): Add language and mode
	to be restored.
	(make_cleanup_restore_current_language): Pass the mode
	and language to do_restore_current_language.

gdb/mi

	* mi-main.c (mi_cmd_execute): When --language is present set
	language mode to manual.

gdb/testsuite:

	* gdb.mi/mi-language-c.cc: New file.
	* gdb.mi/mi-language-c.exp: New file.
	* gdb.mi/mi-language-fortran.cc: New file.
	* gdb.mi/mi-language-fortran.exp: New file.

---
 gdb/mi/mi-main.c                             |  1 +
 gdb/testsuite/gdb.mi/mi-language-c.cc        | 36 +++++++++++++++++
 gdb/testsuite/gdb.mi/mi-language-c.exp       | 60 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-language-fortran.exp | 54 +++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-language-fortran.f90 | 38 ++++++++++++++++++
 gdb/utils.c                                  | 24 ++++++++---
 6 files changed, 207 insertions(+), 6 deletions(-)  create mode 100644 gdb/testsuite/gdb.mi/mi-language-c.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-language-c.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-language-fortran.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-language-fortran.f90


Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052


Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
On 11/04/2014 06:57 PM, Tedeschi, Walfred wrote:
> Hello Vladimir,
>
> This is not really a ping, first one  I have sent to the wrong maintainer. (sorry)
>
> Any feedback on that:
> https://sourceware.org/ml/gdb-patches/2014-09/msg00868.html

Hi Fred,

I've looked at the patch above, and have no objections to MI parts of it.

Thanks,

--
Vladimir Prus
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software/
  

Comments

Pedro Alves June 24, 2015, 1:12 p.m. UTC | #1
Hi Walfred,

On 06/18/2015 01:53 PM, Tedeschi, Walfred wrote:

> I have looked for potential users of the same fix, could not find though.
> Though I was suspicious about the a routine prepare_re_set_context on the breakpoint file.  However there was no clean-up routine there what made me think that the code was already taking care of language on the right way.

That actually looks like a bug.

grepping for make_cleanup_restore_current_language finds these:

 mi/mi-main.c:2253:      make_cleanup_restore_current_language ();
 parse.c:1214:  inner_chain = make_cleanup_restore_current_language ();
 parse.c:1282:      old_chain = make_cleanup_restore_current_language ();

The parse.c ones may postdate your original patch (not sure), but they
look like could/should use the same treatment.  So I still thing we
should have a set_language_and_mode (or some such) function or maybe
even pass the desired language to make_cleanup_restore_current_language
(maybe rename it) and make that set the language and mode, which then
makes it impossible to forget to force manual mode if you're going
to override it.

> Trying to use --language to create a variable showed that --language was
> not always working.  In some cases GDB understands that the language is automatic
> and cannot parse the language set while executing the command.
> In order to fix this just before executing the command language mode
> should be set to manual.  In this way GDB can parse the expression using
> the language given in the command.

Would you mind clarifying the commit log?  I had to apply
the patch locally to understand why/where/how this could happen.
I suggest:

~~~~~
Trying to use --language to create a variable object does not always
work.  If the language mode is set to automatic, and if there's a
selected frame (that is, the program has been started), GDB's parsing
routines switch to the frame's block's language, with the end result
being the explicit --language is ignored.  See parse_exp_in_context_1's
use of language_mode_auto.

In order to fix this, if --language was specified, force the language
mode to manual before executing the MI command.
~~~


> +   Copyright 2014 Free Software Foundation, Inc.

2014-2015

> +# Copyright 2014 Free Software Foundation, Inc.

Ditto.

> +gdb_exit
> \ No newline at end of file

Please add a newline.


> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -444,24 +444,36 @@ make_cleanup_free_so (struct so_list *so)
>  
>  /* Helper for make_cleanup_restore_current_language.  */
>  
> +struct saved_language_and_mode
> +{
> +  enum language lang;
> +  enum language_mode mode;
> +};
> +
>  static void
>  do_restore_current_language (void *p)
>  {
> -  enum language saved_lang = (uintptr_t) p;
> +  struct saved_language_and_mode *lang_and_mode = p;  enum language 
> + saved_lang = lang_and_mode->lang;
>  

Something really odd here.  Misplaced line break?

> +  language_mode = lang_and_mode->mode;
>    set_language (saved_lang);
>  }
>  
> -/* Remember the current value of CURRENT_LANGUAGE and make it restored when
> -   the cleanup is run.  */
> +/* Remember the current value of CURRENT_LANGUAGE and
> +   LANGUAGE_MODE restoring them when the cleanup is run.  */
>  
>  struct cleanup *
>  make_cleanup_restore_current_language (void)  {
> -  enum language saved_lang = current_language->la_language;
> +  struct saved_language_and_mode *lang_and_mode
> +    = XNEW (struct saved_language_and_mode);
> +
> +  lang_and_mode->lang = current_language->la_language;  
> + lang_and_mode->mode = language_mode;
>  

Indentation looks odd here.  Should be two spaces in the
mode line too.

> +mi_create_breakpoint "-t $srcfile:$bp_lineno" "add bp"\
> +  "del" "struct_test" ".*$srcfile" $bp_lineno $hex \
> +  "MI-language-fortran insert breakpoint at line $bp_lineno (only bp)"

Please avoid line numbers in test messages, so that the test
messages remain stable when someone tweaks the tests' source code.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 59717ca..0198981 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2236,6 +2236,7 @@  mi_cmd_execute (struct mi_parse *parse)
   if (parse->language != language_unknown)
     {
       make_cleanup_restore_current_language ();
+      language_mode = language_mode_manual;
       set_language (parse->language);
     }
 
diff --git a/gdb/testsuite/gdb.mi/mi-language-c.cc b/gdb/testsuite/gdb.mi/mi-language-c.cc
new file mode 100644
index 0000000..0c8bd42
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-language-c.cc
@@ -0,0 +1,36 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+struct A
+{
+  virtual ~A ();
+  int a1;
+};
+
+A::~A ()
+{
+  a1 = 800;
+}
+
+int
+main (void)
+{
+  A alpha;
+  alpha.a1 = 100;
+
+  return alpha.a1; /* only bp.  */
+}
diff --git a/gdb/testsuite/gdb.mi/mi-language-c.exp b/gdb/testsuite/gdb.mi/mi-language-c.exp
new file mode 100644
index 0000000..00f5210
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-language-c.exp
@@ -0,0 +1,60 @@ 
+# 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/>.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile .cc
+
+if [get_compiler_info "c++"] {
+    return -1
+}
+
+if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable {debug 
+c++}] != ""} {
+  untested $testfile.exp
+  return -1
+}
+
+mi_gdb_load ${binfile}
+
+if ![mi_run_to_main] {
+    untested "could not run to main"
+    return -1
+}
+
+set lineno [gdb_get_line_number "only bp."]
+
+mi_create_breakpoint "$srcfile:$lineno" "add mi-language1 bp" keep {main\(\)} \
+                     "$srcfile" $lineno $hex "create break at main"
+
+mi_execute_to "exec-continue" "breakpoint-hit" \
+              "main" "" ".*" ".*" {"" "disp=\"keep\""} \
+              "continue to main bp"
+
+mi_gdb_test "-var-create --language fortran alpha_1 * (alpha%a1)" \
+            "\\^done,name=\"alpha_1\",numchild=\"0\",value=\"100\",type=\"int\",thread-id=\"\[0-9\]+\",has_more=\"0\"" \
+            "-var-create from c using fortran language"
+
+mi_gdb_test "-var-create alpha_2 * (alpha.a1)" \
+            "\\^done,name=\"alpha_2\",numchild=\"0\",value=\"100\",type=\"int\",thread-id=\"\[0-9\]+\",has_more=\"0\"" \
+            "-var-create from c using default language"
+gdb_exit
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.mi/mi-language-fortran.exp b/gdb/testsuite/gdb.mi/mi-language-fortran.exp
new file mode 100644
index 0000000..1f38a7b
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-language-fortran.exp
@@ -0,0 +1,54 @@ 
+# Copyright 2015 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile .f90
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+     {debug f90}] != "" } {
+     untested mi-language-fortran.exp
+     return -1
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load ${binfile}
+
+set bp_lineno [gdb_get_line_number "only bp"]
+
+mi_create_breakpoint "-t $srcfile:$bp_lineno" "add bp"\
+  "del" "struct_test" ".*$srcfile" $bp_lineno $hex \
+  "MI-language-fortran insert breakpoint at line $bp_lineno (only bp)"
+
+mi_run_cmd
+
+mi_expect_stop "breakpoint-hit" "struct_test" "" ".*$srcfile" \
+               "$bp_lineno" { "" "disp=\"del\"" } "run to bp"
+
+mi_gdb_test "-var-create --language c alpha_1 * (p.c)" \
+            "\\^done,name=\"alpha_1\",numchild=\"0\",value=\"1\",type=\"integer\\(kind=4\\)\",thread-id=\"\[0-9\]+\",has_more=\"0\"" \
+            "-var-create from fortran using fortran language"
+
+mi_gdb_test "-var-create alpha_2 * (p%c)" \
+            "\\^done,name=\"alpha_2\",numchild=\"0\",value=\"1\",type=\"integer\\(kind=4\\)\",thread-id=\"\[0-9\]+\",has_more=\"0\"" \
+            "-var-create from fortran using default language"
+gdb_exit
diff --git a/gdb/testsuite/gdb.mi/mi-language-fortran.f90 b/gdb/testsuite/gdb.mi/mi-language-fortran.f90
new file mode 100644
index 0000000..f24d7d9
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-language-fortran.f90
@@ -0,0 +1,38 @@ 
+! Copyright 2006-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/>.
+
+
+program struct_test
+
+  type bar
+    integer :: c
+    real :: d
+  end type
+  type foo
+    real :: a
+    type(bar) :: x
+    character*7 :: b
+  end type foo
+  type(foo) :: q
+  type(bar) :: p
+
+  p = bar(1, 2.375)
+  q%a = 3.125
+  q%b = "abcdefg"
+  q%x%c = 1
+  q%x%d = 2.375
+  print *,p,q                     ! only bp
+
+end program struct_test
diff --git a/gdb/utils.c b/gdb/utils.c
index 4da9501..cffa399 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -444,24 +444,36 @@  make_cleanup_free_so (struct so_list *so)
 
 /* Helper for make_cleanup_restore_current_language.  */
 
+struct saved_language_and_mode
+{
+  enum language lang;
+  enum language_mode mode;
+};
+
 static void
 do_restore_current_language (void *p)
 {
-  enum language saved_lang = (uintptr_t) p;
+  struct saved_language_and_mode *lang_and_mode = p;  enum language 
+ saved_lang = lang_and_mode->lang;
 
+  language_mode = lang_and_mode->mode;
   set_language (saved_lang);
 }
 
-/* Remember the current value of CURRENT_LANGUAGE and make it restored when
-   the cleanup is run.  */
+/* Remember the current value of CURRENT_LANGUAGE and
+   LANGUAGE_MODE restoring them when the cleanup is run.  */
 
 struct cleanup *
 make_cleanup_restore_current_language (void)  {
-  enum language saved_lang = current_language->la_language;
+  struct saved_language_and_mode *lang_and_mode
+    = XNEW (struct saved_language_and_mode);
+
+  lang_and_mode->lang = current_language->la_language;  
+ lang_and_mode->mode = language_mode;
 
-  return make_cleanup (do_restore_current_language,
-		       (void *) (uintptr_t) saved_lang);
+  return make_cleanup_dtor (do_restore_current_language,
+		       (void *) lang_and_mode, xfree);
 }
 
 /* Helper function for make_cleanup_clear_parser_state.  */
--
2.1.0