public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/fortran: Don't access non-existent type fields
@ 2020-07-16  9:47 Andrew Burgess
  2020-10-19  9:55 ` [PATCHv2] " Andrew Burgess
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Burgess @ 2020-07-16  9:47 UTC (permalink / raw)
  To: gdb-patches

When attempting to call a Fortran function for which there is no debug
information we currently trigger undefined behaviour in GDB by
accessing non-existent type fields.

The reason is that in order to prepare the arguments for a call to a
Fortran function we need to know about the type of each argument.  If
the function being called has no debug information then obviously GDB
doesn't know about the argument types and we should either give the
user an error or pick a suitable default.  What we currently do is
just assume the fields exist and access them anyway, which is clearly
wrong.

The reason GDB needs to know the argument type is to tell if the
argument is artificial or not, artificial arguments will be passed by
value while non-artificial arguments will be passed by reference.

When considering what was the "correct" default to assume I struggled
to decide, both assumptions seem to have advantages.  For example, if
we assume all arguments are non-artificial and pass them by reference
then passing the "normal" arguments to a function works how the user
expects, for example:

  (gdb) call function_without_debug (1, 2, 3)

will pass the three integers 1, 2, 3.  This is just how we would
expect this to appear in the Fortran source code, and is how we would
call the same function if it did have debug information.

The problem is that if we need to pass an artificial argument by value
then we just can't do it under this scheme.  So, if we instead assume
all arguments are artificial, to call the same function the user would
now have to write:

  (gdb) call function_without_debug (&1, &2, &3)

which is different to how the function is called when it does have
debug information.

My proposal in this commit is to add a new setting that lets the user
pick between these two choices. So we now have:

  set fortran arguments-are-artificial on|off
  show fortran arguments-are-artificial

The default is off, so GDB assumes arguments are not artificial, and
passes by reference, this is the first case highlighted above.

When this setting is turned on we get the second behaviour and
non-artificial arguments need to have their address taken.

gdb/ChangeLog:

	PR fortran/26155
	* NEWS: Mention new setting.
	* eval.c (evaluate_subexp_standard): Check number of fields a type
	has before accessing the fields.
	* f-lang.c: Add 'gdbcmnd.h' include.
	(fortran_arguments_are_artificial): New static global.
	(show_fortran_arguments_are_artificial): New function.
	(set_fortran_list): New static global.
	(show_fortran_list): New static global.
	(_initialize_f_language): Register new commands.
	(fortran_argument_convert): Update comment, make static.
	(fortran_prepare_argument): New function.
	* f-lang.h (fortran_argument_convert): Delete declaration.
	(fortran_prepare_argument): Declare.

gdb/testsuite/ChangeLog:

	PR fortran/26155
	* gdb.fortran/call-no-debug-func.f90: New file.
	* gdb.fortran/call-no-debug-prog.f90: New file.
	* gdb.fortran/call-no-debug.exp: New file.

gdb/doc/ChangeLog:

	PR fortran/26155
	* gdb.texinfo (Fortran): Document new setting.
---
 gdb/ChangeLog                                 |  17 +++
 gdb/NEWS                                      |  10 ++
 gdb/doc/ChangeLog                             |   5 +
 gdb/doc/gdb.texinfo                           |  54 +++++++++
 gdb/eval.c                                    |  21 +---
 gdb/f-lang.c                                  | 106 ++++++++++++++++-
 gdb/f-lang.h                                  |  31 +++--
 gdb/testsuite/ChangeLog                       |   7 ++
 .../gdb.fortran/call-no-debug-func.f90        |  29 +++++
 .../gdb.fortran/call-no-debug-prog.f90        |  35 ++++++
 gdb/testsuite/gdb.fortran/call-no-debug.exp   | 108 ++++++++++++++++++
 11 files changed, 390 insertions(+), 33 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/call-no-debug-func.f90
 create mode 100644 gdb/testsuite/gdb.fortran/call-no-debug-prog.f90
 create mode 100644 gdb/testsuite/gdb.fortran/call-no-debug.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index ded544d6400..e5952fbfdbe 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -75,6 +75,16 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
   executable file; if 'warn', just display a warning; if 'off', don't
   attempt to detect a mismatch.
 
+set fortran arguments-are-artificial on|off
+show fortran arguments-are-artificial
+  When calling Fortran functions without debug information GDB needs
+  to know if arguments should be treated as artificial or not.  By
+  default this setting is off, and all arguments are treated as
+  non-artificial, and passed by reference.
+  When this setting is on, arguments are treated as artificial and
+  passed by value, to pass a non-artificial argument take its address
+  with '&' and pass that instead.
+
 tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a002084d5b9..e732cc2bc96 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -16848,6 +16848,7 @@
 currently supports only the features of Fortran 77 language.
 
 @cindex trailing underscore, in Fortran symbols
+@anchor{fortran_trailing_underscores}
 Some Fortran compilers (@sc{gnu} Fortran 77 and Fortran 95 compilers
 among them) append an underscore to the names of variables and
 functions.  When you debug programs compiled by those compilers, you
@@ -16915,6 +16916,59 @@
 block whose name is @var{common-name}.  With no argument, the names of
 all @code{COMMON} blocks visible at the current program location are
 printed.
+
+@cindex artificial arguments, Fortran
+@kindex set fortran arguments-are-artificial
+@kindex show fortran arguments-are-artificial
+@item set fortran arguments-are-artificial @r{[}on|off@r{]}
+@item show fortran arguments-are-artificial
+Fortran subroutines and functions have the concept of artificial
+arguments.  For example when passing an assumed-shape array the array
+argument is pass-by-reference and identifies the contents of the
+array, while a second artificial parameter is pass-by-value and
+identifies the length of the array.
+
+When calling Fortran program functions (@pxref{Calling,,Calling
+Program Functions}) that don't have debug information @value{GDBN} has
+no way to know which parameters are artificial and should be
+pass-by-value, and which are non-artificial, and should be
+pass-by-reference.
+
+When this setting is @samp{off}, which is the default, and you call a
+program function that does not have debug information, @value{GDBN}
+will assume all arguments are not artificial.  Every argument will be
+passed by reference.  This means that if the argument is not already
+in the programs memory @value{GDBN} will allocate space, copy the
+argument into the programs memory, and then pass a pointer to the
+argument to the program function.
+
+When this setting is @samp{on}, and you call a program function that
+does not have debug information, @value{GDBN} will assume all arguments
+are artificial and pass them by value.  If you need to pass a mix of
+non-artificial and artificial arguments, then pass the address of the
+argument using the @code{&} operator.  The following Fortran function
+takes an integer and string argument.  The string, being
+assumed-shape, will require a third artifical argument containing the
+strings length:
+
+@smallexample
+integer function a_func (num, str)
+  integer :: num
+  character(len=*) :: str
+  ! ....
+end function a_func
+@end smallexample
+
+If the above function was compiled without debug information, then to
+call it within @value{GDBN}, you might do this:
+
+@smallexample
+(@value{GDBN}) set fortran arguments-are-artificial on
+(@value{GDBN}) print (integer) a_func_ (&99, &'abc', 3)
+@end smallexample
+
+For an explanation of why @code{a_func_} was used see the description
+of @ref{fortran_trailing_underscores,,trailing underscores}.
 @end table
 
 @node Pascal
diff --git a/gdb/eval.c b/gdb/eval.c
index c62c35f3183..d30603a3587 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1994,21 +1994,12 @@ evaluate_subexp_standard (struct type *expect_type,
 	  tem = 1;
 	  for (; tem <= nargs; tem++)
 	    {
-	      argvec[tem] = evaluate_subexp_with_coercion (exp, pos, noside);
-	      /* Arguments in Fortran are passed by address.  Coerce the
-		 arguments here rather than in value_arg_coerce as otherwise
-		 the call to malloc to place the non-lvalue parameters in
-		 target memory is hit by this Fortran specific logic.  This
-		 results in malloc being called with a pointer to an integer
-		 followed by an attempt to malloc the arguments to malloc in
-		 target memory.  Infinite recursion ensues.  */
-	      if (code == TYPE_CODE_PTR || code == TYPE_CODE_FUNC)
-		{
-		  bool is_artificial
-		    = TYPE_FIELD_ARTIFICIAL (value_type (arg1), tem - 1);
-		  argvec[tem] = fortran_argument_convert (argvec[tem],
-							  is_artificial);
-		}
+	      bool is_internal_func = (code == TYPE_CODE_INTERNAL_FUNCTION);
+	      argvec[tem] = fortran_prepare_argument (exp, pos,
+						      (tem - 1),
+						      is_internal_func,
+						      value_type (arg1),
+						      noside);
 	    }
 	  argvec[tem] = 0;	/* signal end of arglist */
 	  if (noside == EVAL_SKIP)
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 58b41d11d11..252fe928afd 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -36,6 +36,7 @@
 #include "c-lang.h"
 #include "target-float.h"
 #include "gdbarch.h"
+#include "gdbcmd.h"
 
 #include <math.h>
 
@@ -802,16 +803,78 @@ builtin_f_type (struct gdbarch *gdbarch)
   return (const struct builtin_f_type *) gdbarch_data (gdbarch, f_type_data);
 }
 
+/* Whether GDB should assume arguments to functions without debug are
+   artificial or not.  */
+
+static bool fortran_arguments_are_artificial = false;
+
+/* Implement 'show fortran arguments-are-artificial'.  */
+
+static void
+show_fortran_arguments_are_artificial (struct ui_file *file, int from_tty,
+				       struct cmd_list_element *c,
+				       const char *value)
+{
+  fprintf_filtered (file, _("Assuming arguments to Fortran functions "
+			    "without debug are artificial is %s.\n"),
+		    value);
+}
+
+/* Command-list for the "set/show fortran" prefix command.  */
+
+static struct cmd_list_element *set_fortran_list;
+static struct cmd_list_element *show_fortran_list;
+
 void _initialize_f_language ();
 void
 _initialize_f_language ()
 {
   f_type_data = gdbarch_data_register_post_init (build_fortran_types);
+
+  add_basic_prefix_cmd ("fortran", no_class,
+                        _("Prefix command for changing Fortran-specific settings."),
+                        &set_fortran_list, "set fortran ", 0, &setlist);
+
+  add_show_prefix_cmd ("fortran", no_class,
+                       _("Generic command for showing Fortran-specific settings."),
+                       &show_fortran_list, "show fortran ", 0, &showlist);
+
+  add_setshow_boolean_cmd ("arguments-are-artificial", class_vars,
+                           &fortran_arguments_are_artificial, _("\
+Sets whether arguments to functions without debug information are artificial."), _("\
+Show whether arguments to functions without debug information are artificial."), _("\
+When calling a function in the inferior that does not have debug\n\
+information GDB needs to decide if the arguments are artificial or not.\n\
+\n\
+Artificial arguments are passed by value while non-artificial arguments\n\
+are passed by reference.\n\
+When this setting is on GDB will assume all arguments are artificial and\n\
+pass them by value.  If you need to pass a non-artificial argument then\n\
+pass the address of the argument.\n\
+\n\
+This setting only effects calling functions without debug information.  For\n\
+functions with debug information GDB knows which arguments are artificial,\n\
+and which are not."),
+                           NULL,
+                           show_fortran_arguments_are_artificial,
+                           &set_fortran_list, &show_fortran_list);
 }
 
-/* See f-lang.h.  */
+/* Ensures that function argument VALUE is in the appropriate form to
+   pass to a Fortran function.  Returns a possibly new value that should
+   be used instead of VALUE.
+
+   When IS_ARTIFICIAL is true this indicates an artificial argument,
+   e.g. hidden string lengths which the GNU Fortran argument passing
+   convention specifies as being passed by value.
 
-struct value *
+   When IS_ARTIFICIAL is false, the argument is passed by pointer.  If the
+   value is already in target memory then return a value that is a pointer
+   to VALUE.  If VALUE is not in memory (e.g. an integer literal), allocate
+   space in the target, copy VALUE in, and return a pointer to the in
+   memory copy.  */
+
+static struct value *
 fortran_argument_convert (struct value *value, bool is_artificial)
 {
   if (!is_artificial)
@@ -845,3 +908,42 @@ fortran_preserve_arg_pointer (struct value *arg, struct type *type)
     return value_type (arg);
   return type;
 }
+
+/* See f-lang.h.  */
+
+value *
+fortran_prepare_argument (struct expression *exp, int *pos,
+			  int arg_num, bool is_internal_call_p,
+			  struct type *func_type, enum noside noside)
+{
+  if (is_internal_call_p)
+    return evaluate_subexp_with_coercion (exp, pos, noside);
+
+  bool is_artificial;
+  if (arg_num >= func_type->num_fields ())
+    {
+      /* We are unable to know if this argument is artificial or not.  The
+	 behaviour now depends on 'set fortran arguments-are-artificial'.  */
+      if (fortran_arguments_are_artificial)
+	{
+	  /* If the expression the user is trying to pass here starts by
+	     taking the address of a value then they are trying to pass a
+	     non-artificial argument, strip away the address-of operator,
+	     and allow FORTRAN_ARGUMENT_CONVERT to fix things up.  */
+	  if (exp->elts[*pos].opcode == UNOP_ADDR)
+	    {
+	      ++(*pos);
+	      is_artificial = false;
+	    }
+	  else
+	    is_artificial = true;
+	}
+      else
+	is_artificial = false;
+    }
+  else
+    is_artificial = TYPE_FIELD_ARTIFICIAL (func_type, arg_num);
+
+  struct value *arg_val = evaluate_subexp_with_coercion (exp, pos, noside);
+  return fortran_argument_convert (arg_val, is_artificial);
+}
diff --git a/gdb/f-lang.h b/gdb/f-lang.h
index 4710b14aa62..6a12527594b 100644
--- a/gdb/f-lang.h
+++ b/gdb/f-lang.h
@@ -89,22 +89,21 @@ struct builtin_f_type
 /* Return the Fortran type table for the specified architecture.  */
 extern const struct builtin_f_type *builtin_f_type (struct gdbarch *gdbarch);
 
-/* Ensures that function argument VALUE is in the appropriate form to
-   pass to a Fortran function.  Returns a possibly new value that should
-   be used instead of VALUE.
-
-   When IS_ARTIFICIAL is true this indicates an artificial argument,
-   e.g. hidden string lengths which the GNU Fortran argument passing
-   convention specifies as being passed by value.
-
-   When IS_ARTIFICIAL is false, the argument is passed by pointer.  If the
-   value is already in target memory then return a value that is a pointer
-   to VALUE.  If VALUE is not in memory (e.g. an integer literal), allocate
-   space in the target, copy VALUE in, and return a pointer to the in
-   memory copy.  */
-
-extern struct value *fortran_argument_convert (struct value *value,
-					       bool is_artificial);
+/* Prepare (and return) an argument value ready for an inferior function
+   call to a Fortran function.  EXP and POS are the expressions describing
+   the argument to prepare.  ARG_NUM is the argument number being
+   prepared, with 0 being the first argument and so on.  FUNC_TYPE is the
+   type of the function being called.
+
+   IS_INTERNAL_CALL_P is true if this is a call to a function of type
+   TYPE_CODE_INTERNAL_FUNCTION, otherwise this parameter is false.
+
+   NOSIDE has its usual meaning for expression parsing (see eval.c).  */
+
+extern value *fortran_prepare_argument (struct expression *exp, int *pos,
+					int arg_num, bool is_internal_call_p,
+					struct type *func_type,
+					enum noside noside);
 
 /* Ensures that function argument TYPE is appropriate to inform the debugger
    that ARG should be passed as a pointer.  Returns the potentially updated
diff --git a/gdb/testsuite/gdb.fortran/call-no-debug-func.f90 b/gdb/testsuite/gdb.fortran/call-no-debug-func.f90
new file mode 100644
index 00000000000..c443b21103b
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/call-no-debug-func.f90
@@ -0,0 +1,29 @@
+! Copyright 2020 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/>.
+
+! Return ARG plus 1.
+integer function some_func (arg)
+  integer :: arg
+
+  some_func = (arg + 1)
+end function some_func
+
+! Print STR.
+integer function string_func (str)
+  character(len=*) :: str
+
+  print *, str
+  string_func = 0
+end function string_func
diff --git a/gdb/testsuite/gdb.fortran/call-no-debug-prog.f90 b/gdb/testsuite/gdb.fortran/call-no-debug-prog.f90
new file mode 100644
index 00000000000..988ed10a1d8
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/call-no-debug-prog.f90
@@ -0,0 +1,35 @@
+! Copyright 2020 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 main
+  implicit none
+
+  interface
+     integer function some_func (arg)
+       integer :: arg
+     end function some_func
+
+     integer function string_func (str)
+       character(len=*) :: str
+     end function string_func
+  end interface
+
+  integer :: val
+
+  val = some_func (1)
+  print *, val
+  val = string_func ('hello')
+  print *, val
+end program main
diff --git a/gdb/testsuite/gdb.fortran/call-no-debug.exp b/gdb/testsuite/gdb.fortran/call-no-debug.exp
new file mode 100644
index 00000000000..9d88c3cb7f9
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/call-no-debug.exp
@@ -0,0 +1,108 @@
+# Copyright 2020 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 calling Fortran functions that are compiled without debug
+# information.
+
+if {[skip_fortran_tests]} { return -1 }
+
+standard_testfile call-no-debug-prog.f90 call-no-debug-func.f90
+load_lib fortran.exp
+
+if {[prepare_for_testing_full "failed to prepare" \
+	 [list ${binfile} [list debug f90] \
+	      $srcfile [list debug f90] \
+	      $srcfile2 [list nodebug f90]]]} {
+    return -1
+}
+
+if ![fortran_runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+# Find a possibly mangled version of NAME, a function we want to call
+# that has no debug information available.  We hope that the mangled
+# version of NAME contains the pattern NAME, and so we use 'info
+# functions' to find a possible suitable symbol.
+#
+# If no suitable function is found then return the empty string.
+proc find_mangled_name { name } {
+    global hex gdb_prompt
+
+    set into_non_debug_symbols false
+    set symbol_name "*unknown*"
+    gdb_test_multiple "info function $name" "" {
+	-re ".*Non-debugging symbols:\r\n" {
+	    set into_non_debug_symbols true
+	    exp_continue
+	}
+	-re "$hex.*\[ \t\]+(\[^\r\n\]+)\r\n" {
+	    set symbol_name $expect_out(1,string)
+	    exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    # Done.
+	}
+    }
+
+    # If we couldn't find a suitable symbol name return the empty
+    # string.
+    if { $symbol_name == "*unknown*" } {
+	return ""
+    }
+
+    return $symbol_name
+}
+
+# Call the function SOME_FUNC, that takes a single integer and returns
+# an integer.  We test this in the default mode, where GDB assumes
+# arguments passed by the user are not artificial, and so passes all
+# arguments by reference.
+#
+# Then we switch GDB to assume that all arguments are artificial, we
+# then have to pass the address of the integer argument.
+set symbol_name [find_mangled_name "some_func"]
+if { $symbol_name == "" } {
+    untested "couldn't find suitable name for 'some_func'"
+} else {
+    gdb_test "ptype ${symbol_name}" "type = <unknown return type> \\(\\)"
+    gdb_test "print ${symbol_name} (1)" \
+	"'${symbol_name}' has unknown return type; cast the call to its declared return type"
+    gdb_test "print (integer) ${symbol_name} (1)" " = 2"
+    gdb_test_no_output "set fortran arguments-are-artificial on"
+    gdb_test "print (integer) ${symbol_name} (&3)" " = 4"
+}
+
+# Call the function STRING_FUNC which takes an assumed shape character
+# array (i.e. a string), and returns an integer.
+#
+# At least for gfortran, passing the string will pass both the data
+# pointer and an artificial argument, the length of the string.
+#
+# To handle passing the artificial we must switch GDB into the mode
+# where it assumes arguments are artificial, and then we pass the
+# address of the array data, and the length as pass-by-value.
+set symbol_name [find_mangled_name "string_func"]
+if { $symbol_name == "" } {
+    untested "couldn't find suitable name for 'string_func'"
+} else {
+    gdb_test "ptype ${symbol_name}" "type = <unknown return type> \\(\\)"
+    gdb_test_no_output "set fortran arguments-are-artificial on" \
+	"ensure fortran artificial arguments is on"
+    gdb_test "print ${symbol_name} (&'abcdefg', 3)" \
+	"'${symbol_name}' has unknown return type; cast the call to its declared return type"
+    gdb_test "call (integer) ${symbol_name} (&'abcdefg', 3)" " abc\r\n\\\$\\d+ = 0"
+}
-- 
2.25.4


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-24 20:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16  9:47 [PATCH] gdb/fortran: Don't access non-existent type fields Andrew Burgess
2020-10-19  9:55 ` [PATCHv2] " Andrew Burgess
2020-10-20 19:32   ` Tom Tromey
2021-02-11 15:48     ` Andrew Burgess
2021-02-24 17:47       ` Andrew Burgess
2021-02-24 20:47         ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).