FYI, pushed: [RFA v5] (Ada) Fix frame argument printing when using auto language mode

Message ID 20190508174752.GA17058@adacore.com
State New, archived
Headers

Commit Message

Joel Brobecker May 8, 2019, 5:47 p.m. UTC
  Hello everyone,

We just realized that the patch below never got pushed despite
the conditional approval; so I just rebased it, made the required
adjustments, retested it, and then pushed it to master.

Attached is the version that got pushed.

> > gdb/ChangeLog:
> > 
> >     * ada-valprint.c (ada_val_print_gnat_array): Remove language
> >     parameter and use Ada language definition instead.
> >     (ada_val_print_ptr): Remove unused language parameter.
> >     (ada_val_print_num): Remove language parameter and use Ada language
> >     definition instead.
> >     (ada_val_print_enum, ada_val_print_flt): Remove unused language
> >     parameter.
> >     (ada_val_print_struct_union, ada_val_print_ref): Remove language
> >     parameter and use Ada language definition instead.
> >     (ada_val_print_1): Update all ada_val_print_xxx calls.
> 
> You forgot to say that you removed the "language" parameter.
> 
> >     (ada_val_print): Update ada_val_print_1 call.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> >     * gdb.ada/frame_arg_lang.exp: New testcase.
> >     * gdb.ada/frame_arg_lang/bla.adb: New file.
> >     * gdb.ada/frame_arg_lang/pck.ads: New file.
> >     * gdb.ada/frame_arg_lang/pck.adb: New file.
> >     * gdb.ada/frame_arg_lang/foo.c: New file.
> 
> This patch is OK for me, modulo the question below. Please give
> everyone who commmented on the patch already a day or two to either
> comment or request more time to comment before you push.
  

Comments

Philippe Waroquiers May 8, 2019, 8:48 p.m. UTC | #1
On Wed, 2019-05-08 at 10:47 -0700, Joel Brobecker wrote:
> Hello everyone,
> 
> We just realized that the patch below never got pushed despite
> the conditional approval; so I just rebased it, made the required
> adjustments, retested it, and then pushed it to master.
Note that there was a version v6:
https://sourceware.org/ml/gdb-patches/2018-03/msg00005.html
but the subject above speaks about v5.

Maybe worth double checking that the final version was pushed ...

Philippe
  

Patch

From 2228ef7700e81bfedbffe352f3efa3716224cdbe Mon Sep 17 00:00:00 2001
From: Xavier Roirand <roirand@adacore.com>
Date: Wed, 8 May 2019 12:55:44 -0400
Subject: [PATCH] When debugging a mixed Ada/C program using this scenario:

- set print frame-arguements all
- an Ada function named pck.call_me calls a C function named break_me
- you put a breakpoint in break_me and the program reaches this
  breakpoint.

Now display the backtrace:

  (gdb) bt
  #0  break_me () at [...]
  #1  0x000000000040243e in pck.call_me (
      s={P_ARRAY = 0x7fffffffe21c, P_BOUNDS = 0x41e6e8}) at [...]

whereas we should expect:

  (gdb) bt
  #0  break_me () at [...]
  #1  0x000000000040243e in pck.call_me (s="test") at [...]

The problem is that GDB prints the S parameter in the pck.call_me Ada
function using the current language, so the C one, because the program
is stopped in a C function, whereas it should use the pck.call_me frame
one. This behavior is ok when user manually changes the language but it's
not the right one when language is auto.

This patch fixes this problem so now when using auto language, all Ada
frame arguments are printed using Ada like syntax when the frame is part
of Ada code, even if the program is stopped in a frame using a different
language.

If the user explicitly sets a language (using "set language ...") then
no change here, all the Ada frame arguments are printed using this
language.

gdb/ChangeLog:

    * ada-valprint.c (ada_val_print_gnat_array): Remove language
    parameter and use Ada language definition instead.
    (ada_val_print_ptr): Remove unused language parameter.
    (ada_val_print_num): Remove language parameter and use Ada language
    definition instead.
    (ada_val_print_enum, ada_val_print_flt): Remove unused language
    parameter.
    (ada_val_print_struct_union, ada_val_print_ref): Remove language
    parameter and use Ada language definition instead.
    (ada_val_print_1): Update all ada_val_print_xxx calls.
    Remove language parameter.
    (ada_val_print): Update ada_val_print_1 call.

gdb/testsuite/ChangeLog:

    * gdb.ada/frame_arg_lang.exp: New testcase.
    * gdb.ada/frame_arg_lang/bla.adb: New file.
    * gdb.ada/frame_arg_lang/pck.ads: New file.
    * gdb.ada/frame_arg_lang/pck.adb: New file.
    * gdb.ada/frame_arg_lang/foo.c: New file.

Tested on x86_64-linux, no regressions.
---
 gdb/ChangeLog                                | 15 ++++
 gdb/ada-valprint.c                           | 56 +++++++--------
 gdb/testsuite/ChangeLog                      |  8 +++
 gdb/testsuite/gdb.ada/frame_arg_lang.exp     | 73 ++++++++++++++++++++
 gdb/testsuite/gdb.ada/frame_arg_lang/bla.adb | 22 ++++++
 gdb/testsuite/gdb.ada/frame_arg_lang/foo.c   | 22 ++++++
 gdb/testsuite/gdb.ada/frame_arg_lang/pck.adb | 24 +++++++
 gdb/testsuite/gdb.ada/frame_arg_lang/pck.ads | 18 +++++
 8 files changed, 207 insertions(+), 31 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/frame_arg_lang.exp
 create mode 100644 gdb/testsuite/gdb.ada/frame_arg_lang/bla.adb
 create mode 100644 gdb/testsuite/gdb.ada/frame_arg_lang/foo.c
 create mode 100644 gdb/testsuite/gdb.ada/frame_arg_lang/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/frame_arg_lang/pck.ads

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0f69508ed6b..81f72a91e28 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@ 
+2019-05-08  Joel Brobecker  <brobecker@adacore.com>
+
+	* ada-valprint.c (ada_val_print_gnat_array): Remove language
+	parameter and use Ada language definition instead.
+	(ada_val_print_ptr): Remove unused language parameter.
+	(ada_val_print_num): Remove language parameter and use Ada language
+	definition instead.
+	(ada_val_print_enum, ada_val_print_flt): Remove unused language
+	parameter.
+	(ada_val_print_struct_union, ada_val_print_ref): Remove language
+	parameter and use Ada language definition instead.
+	(ada_val_print_1): Update all ada_val_print_xxx calls.
+	Remove language parameter.
+	(ada_val_print): Update ada_val_print_1 call.
+
 2019-05-08  Tom Tromey  <tromey@adacore.com>
 
 	* remote.c (remote_hw_watchpoint_limit)
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 5d7823d7204..0654049d77b 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -765,8 +765,7 @@  ada_val_print_gnat_array (struct type *type, const gdb_byte *valaddr,
 			  int offset, CORE_ADDR address,
 			  struct ui_file *stream, int recurse,
 			  struct value *original_value,
-			  const struct value_print_options *options,
-			  const struct language_defn *language)
+			  const struct value_print_options *options)
 {
   struct value *mark = value_mark ();
   struct value *val;
@@ -788,7 +787,8 @@  ada_val_print_gnat_array (struct type *type, const gdb_byte *valaddr,
   else
     val_print (value_type (val),
 	       value_embedded_offset (val), value_address (val),
-	       stream, recurse, val, options, language);
+	       stream, recurse, val, options,
+	       language_def (language_ada));
   value_free_to_mark (mark);
 }
 
@@ -800,8 +800,7 @@  ada_val_print_ptr (struct type *type, const gdb_byte *valaddr,
 		   int offset, int offset_aligned, CORE_ADDR address,
 		   struct ui_file *stream, int recurse,
 		   struct value *original_value,
-		   const struct value_print_options *options,
-		   const struct language_defn *language)
+		   const struct value_print_options *options)
 {
   val_print (type, offset, address, stream, recurse,
 	     original_value, options, language_def (language_c));
@@ -827,8 +826,7 @@  ada_val_print_num (struct type *type, const gdb_byte *valaddr,
 		   int offset, int offset_aligned, CORE_ADDR address,
 		   struct ui_file *stream, int recurse,
 		   struct value *original_value,
-		   const struct value_print_options *options,
-		   const struct language_defn *language)
+		   const struct value_print_options *options)
 {
   if (ada_is_fixed_point_type (type))
     {
@@ -859,12 +857,13 @@  ada_val_print_num (struct type *type, const gdb_byte *valaddr,
 
 	  val_print (target_type,
 		     value_embedded_offset (v), 0, stream,
-		     recurse + 1, v, options, language);
+		     recurse + 1, v, options,
+		     language_def (language_ada));
 	}
       else
 	val_print (TYPE_TARGET_TYPE (type), offset,
 		   address, stream, recurse, original_value,
-		   options, language);
+		   options, language_def (language_ada));
       return;
     }
   else
@@ -923,8 +922,7 @@  ada_val_print_enum (struct type *type, const gdb_byte *valaddr,
 		    int offset, int offset_aligned, CORE_ADDR address,
 		    struct ui_file *stream, int recurse,
 		    struct value *original_value,
-		    const struct value_print_options *options,
-		    const struct language_defn *language)
+		    const struct value_print_options *options)
 {
   int i;
   unsigned int len;
@@ -967,8 +965,7 @@  ada_val_print_flt (struct type *type, const gdb_byte *valaddr,
 		   int offset, int offset_aligned, CORE_ADDR address,
 		   struct ui_file *stream, int recurse,
 		   struct value *original_value,
-		   const struct value_print_options *options,
-		   const struct language_defn *language)
+		   const struct value_print_options *options)
 {
   if (options->format)
     {
@@ -988,8 +985,7 @@  ada_val_print_struct_union
   (struct type *type, const gdb_byte *valaddr, int offset,
    int offset_aligned, CORE_ADDR address, struct ui_file *stream,
    int recurse, struct value *original_value,
-   const struct value_print_options *options,
-   const struct language_defn *language)
+   const struct value_print_options *options)
 {
   if (ada_is_bogus_array_descriptor (type))
     {
@@ -1001,7 +997,8 @@  ada_val_print_struct_union
 
   if (print_field_values (type, valaddr, offset_aligned,
 			  stream, recurse, original_value, options,
-			  0, type, offset_aligned, language) != 0
+			  0, type, offset_aligned,
+			  language_def (language_ada)) != 0
       && options->prettyformat)
     {
       fprintf_filtered (stream, "\n");
@@ -1052,8 +1049,7 @@  ada_val_print_ref (struct type *type, const gdb_byte *valaddr,
 		   int offset, int offset_aligned, CORE_ADDR address,
 		   struct ui_file *stream, int recurse,
 		   struct value *original_value,
-		   const struct value_print_options *options,
-		   const struct language_defn *language)
+		   const struct value_print_options *options)
 {
   /* For references, the debugger is expected to print the value as
      an address if DEREF_REF is null.  But printing an address in place
@@ -1077,7 +1073,7 @@  ada_val_print_ref (struct type *type, const gdb_byte *valaddr,
 	deref_val = ada_tag_value_at_base_address (deref_val);
 
       common_val_print (deref_val, stream, recurse + 1, options,
-			language);
+			language_def (language_ada));
       return;
     }
 
@@ -1106,7 +1102,7 @@  ada_val_print_ref (struct type *type, const gdb_byte *valaddr,
   val_print (value_type (deref_val),
 	     value_embedded_offset (deref_val),
 	     value_address (deref_val), stream, recurse + 1,
-	     deref_val, options, language);
+	     deref_val, options, language_def (language_ada));
 }
 
 /* See the comment on ada_val_print.  This function differs in that it
@@ -1117,8 +1113,7 @@  ada_val_print_1 (struct type *type,
 		 int offset, CORE_ADDR address,
 		 struct ui_file *stream, int recurse,
 		 struct value *original_value,
-		 const struct value_print_options *options,
-		 const struct language_defn *language)
+		 const struct value_print_options *options)
 {
   int offset_aligned;
   const gdb_byte *valaddr = value_contents_for_printing (original_value);
@@ -1131,7 +1126,7 @@  ada_val_print_1 (struct type *type,
     {
       ada_val_print_gnat_array (type, valaddr, offset, address,
 				stream, recurse, original_value,
-				options, language);
+				options);
       return;
     }
 
@@ -1150,33 +1145,33 @@  ada_val_print_1 (struct type *type,
     case TYPE_CODE_PTR:
       ada_val_print_ptr (type, valaddr, offset, offset_aligned,
 			 address, stream, recurse, original_value,
-			 options, language);
+			 options);
       break;
 
     case TYPE_CODE_INT:
     case TYPE_CODE_RANGE:
       ada_val_print_num (type, valaddr, offset, offset_aligned,
 			 address, stream, recurse, original_value,
-			 options, language);
+			 options);
       break;
 
     case TYPE_CODE_ENUM:
       ada_val_print_enum (type, valaddr, offset, offset_aligned,
 			  address, stream, recurse, original_value,
-			  options, language);
+			  options);
       break;
 
     case TYPE_CODE_FLT:
       ada_val_print_flt (type, valaddr, offset, offset_aligned,
 			 address, stream, recurse, original_value,
-			 options, language);
+			 options);
       break;
 
     case TYPE_CODE_UNION:
     case TYPE_CODE_STRUCT:
       ada_val_print_struct_union (type, valaddr, offset, offset_aligned,
 				  address, stream, recurse,
-				  original_value, options, language);
+				  original_value, options);
       break;
 
     case TYPE_CODE_ARRAY:
@@ -1188,7 +1183,7 @@  ada_val_print_1 (struct type *type,
     case TYPE_CODE_REF:
       ada_val_print_ref (type, valaddr, offset, offset_aligned,
 			 address, stream, recurse, original_value,
-			 options, language);
+			 options);
       break;
     }
 }
@@ -1206,8 +1201,7 @@  ada_val_print (struct type *type,
   try
     {
       ada_val_print_1 (type, embedded_offset, address,
-		       stream, recurse, val, options,
-		       current_language);
+		       stream, recurse, val, options);
     }
   catch (const gdb_exception_error &except)
     {
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 817576c4fbd..a556eb36c7a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2019-05-08  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.ada/frame_arg_lang.exp: New testcase.
+	* gdb.ada/frame_arg_lang/bla.adb: New file.
+	* gdb.ada/frame_arg_lang/pck.ads: New file.
+	* gdb.ada/frame_arg_lang/pck.adb: New file.
+	* gdb.ada/frame_arg_lang/foo.c: New file.
+
 2019-05-08  Tom Tromey  <tromey@adacore.com>
 
 	* gdb.python/py-value.exp (test_value_in_inferior): Add Ada test.
diff --git a/gdb/testsuite/gdb.ada/frame_arg_lang.exp b/gdb/testsuite/gdb.ada/frame_arg_lang.exp
new file mode 100644
index 00000000000..e24ca7d4a2b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/frame_arg_lang.exp
@@ -0,0 +1,73 @@ 
+# Copyright 2018-2019 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 "ada.exp"
+
+standard_ada_testfile bla
+set cfile "foo"
+set csrcfile ${srcdir}/${subdir}/${testdir}/${cfile}.c
+set cobject [standard_output_file ${cfile}.o]
+
+gdb_compile "${csrcfile}" "${cobject}" object [list debug]
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional_flags=-largs additional_flags=${cobject} additional_flags=-margs]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.c]
+runto "foo.c:$bp_location"
+
+gdb_test_no_output "set print frame-arguments all"
+
+# Here is the scenario:
+#  - Once stopped in a C function, with language_mode set to auto, print
+#    backtrace, we should see the Ada frame arguments printed using Ada
+#    syntax.
+#  - Set language to C, then check that printing backtrace shows the Ada
+#    frame arguments using C syntax.
+#  - Set language back to auto, check language mode value, then print
+#    backtrace, we should see Ada frame arguments printed using Ada C
+#    syntax.
+
+gdb_test "show lang" \
+         "The current source language is \"auto; currently c\"." \
+         "show language when set to 'auto; c'"
+
+gdb_test "bt" \
+         "#1  $hex in pck\\.call_me \\(s=\"test\"\\).*" \
+         "backtrace with auto: c"
+
+gdb_test_no_output "set language c" \
+                   "Set current source language to \"manual; currently c\"."
+
+gdb_test "show lang" \
+         "The current source language is \"c\"." \
+         "show language when set to 'c'"
+
+gdb_test "bt" \
+         "#1  $hex in pck\\.call_me \\(s={P_ARRAY = $hex, P_BOUNDS = $hex}\\).*" \
+         "backtrace with language forced to 'c'"
+
+gdb_test_no_output "set language auto" \
+                   "Set current source language to \"auto; currently c\"."
+
+gdb_test "show lang" \
+         "The current source language is \"auto; currently c\"." \
+         "show language when set back to 'auto; c'"
+
+gdb_test "bt" \
+         "#1  $hex in pck\\.call_me \\(s=\"test\"\\).*" \
+         "backtrace with language back to 'auto; c'"
diff --git a/gdb/testsuite/gdb.ada/frame_arg_lang/bla.adb b/gdb/testsuite/gdb.ada/frame_arg_lang/bla.adb
new file mode 100644
index 00000000000..5e058554943
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/frame_arg_lang/bla.adb
@@ -0,0 +1,22 @@ 
+--  Copyright 2018-2019 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;
+
+procedure Bla is
+   S : String := "test";
+begin
+   Call_Me (S);
+end Bla;
diff --git a/gdb/testsuite/gdb.ada/frame_arg_lang/foo.c b/gdb/testsuite/gdb.ada/frame_arg_lang/foo.c
new file mode 100644
index 00000000000..e0150314658
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/frame_arg_lang/foo.c
@@ -0,0 +1,22 @@ 
+/* Copyright 2018-2019 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/>.  */
+
+int
+break_me (void)
+{
+  return 0; /* STOP */
+}
diff --git a/gdb/testsuite/gdb.ada/frame_arg_lang/pck.adb b/gdb/testsuite/gdb.ada/frame_arg_lang/pck.adb
new file mode 100644
index 00000000000..65a3169f6c3
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/frame_arg_lang/pck.adb
@@ -0,0 +1,24 @@ 
+--  Copyright 2018-2019 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 C_Break_Me;
+   pragma Import (C, C_Break_Me, "break_me");
+
+   procedure Call_Me (S: in out String) is
+   begin
+      C_Break_Me;
+   end Call_Me;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/frame_arg_lang/pck.ads b/gdb/testsuite/gdb.ada/frame_arg_lang/pck.ads
new file mode 100644
index 00000000000..faff9595e24
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/frame_arg_lang/pck.ads
@@ -0,0 +1,18 @@ 
+--  Copyright 2018-2019 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 Call_Me (S: in out String);
+end Pck;
-- 
2.17.1