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

* [PATCHv2] gdb/fortran: Don't access non-existent type fields
  2020-07-16  9:47 [PATCH] gdb/fortran: Don't access non-existent type fields Andrew Burgess
@ 2020-10-19  9:55 ` Andrew Burgess
  2020-10-20 19:32   ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Burgess @ 2020-10-19  9:55 UTC (permalink / raw)
  To: gdb-patches

Updated for latest GDB master.  Things have moved around a bit, but no
significant code changes.

No changes at all in docs since v1.

---

commit 57f4e3d54b8999a9d203d454a9aa4f48952a7b69
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Mon Jul 13 15:53:14 2020 +0100

    gdb/fortran: Don't access non-existent type fields
    
    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.
            * f-lang.c: Add 'gdbcmnd.h' include.
            (evaluate_subexp_f): Call fortran_prepare_argument.
            (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.
    
    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.

diff --git a/gdb/NEWS b/gdb/NEWS
index 1789cf31356..b09a2d90532 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -117,6 +117,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 2636b6f9903..02759d16bf7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -16852,6 +16852,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
@@ -16919,6 +16920,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/f-lang.c b/gdb/f-lang.c
index e13097baee4..c6631adc37d 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -36,11 +36,17 @@
 #include "c-lang.h"
 #include "target-float.h"
 #include "gdbarch.h"
+#include "gdbcmd.h"
 
 #include <math.h>
 
 /* Local functions */
 
+static value *fortran_prepare_argument (struct expression *exp, int *pos,
+					int arg_num, bool is_internal_call_p,
+					struct type *func_type,
+					enum noside noside);
+
 /* Return the encoding that should be used for the character type
    TYPE.  */
 
@@ -467,22 +473,11 @@ evaluate_subexp_f (struct type *expect_type, struct expression *exp,
 	    int 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)
@@ -1050,16 +1045,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.
 
-struct 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.  */
+
+static struct value *
 fortran_argument_convert (struct value *value, bool is_artificial)
 {
   if (!is_artificial)
@@ -1093,3 +1150,58 @@ fortran_preserve_arg_pointer (struct value *arg, struct type *type)
     return value_type (arg);
   return type;
 }
+
+/* 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).
+
+   Arguments in Fortran are normally passed by address, we 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.  */
+
+static 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..146071634fb 100644
--- a/gdb/f-lang.h
+++ b/gdb/f-lang.h
@@ -89,23 +89,6 @@ 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);
-
 /* Ensures that function argument TYPE is appropriate to inform the debugger
    that ARG should be passed as a pointer.  Returns the potentially updated
    argument type.
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"
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] gdb/fortran: Don't access non-existent type fields
  2020-10-19  9:55 ` [PATCHv2] " Andrew Burgess
@ 2020-10-20 19:32   ` Tom Tromey
  2021-02-11 15:48     ` Andrew Burgess
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2020-10-20 19:32 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Updated for latest GDB master.  Things have moved around a bit, but no
Andrew> significant code changes.

Andrew> No changes at all in docs since v1.

This contents of the patch seem fine to me.
I just wanted to note something about the direction.

Andrew>     When considering what was the "correct" default to assume I struggled
Andrew>     to decide, both assumptions seem to have advantages.

In C what is done now is to make the default case an error, and then
require the user to cast the function to the correct type.  This way
return types are handled correctly as well.

I'm sure you recall but this used to be different, and gdb just assumed
that functions returned int, which was an ongoing source of confusion.
I wonder whether this setting will also cause confusion.

I don't know Fortran, so I don't know if the C approach is available.
I suppose not; in which case this seems reasonable enough.

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] gdb/fortran: Don't access non-existent type fields
  2020-10-20 19:32   ` Tom Tromey
@ 2021-02-11 15:48     ` Andrew Burgess
  2021-02-24 17:47       ` Andrew Burgess
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Burgess @ 2021-02-11 15:48 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

* Tom Tromey <tom@tromey.com> [2020-10-20 13:32:55 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> Updated for latest GDB master.  Things have moved around a bit, but no
> Andrew> significant code changes.
> 
> Andrew> No changes at all in docs since v1.
> 
> This contents of the patch seem fine to me.
> I just wanted to note something about the direction.
> 
> Andrew>     When considering what was the "correct" default to assume I struggled
> Andrew>     to decide, both assumptions seem to have advantages.
> 
> In C what is done now is to make the default case an error, and then
> require the user to cast the function to the correct type.  This way
> return types are handled correctly as well.

I think you can mostly get away with casting to the correct return
type, right?  This is certainly the case in Fortran, even in this
patch, so:

  (gdb) some_func_ ()
  'some_func_' has unknown return type; cast the call to its declared return type

But I think that if we could cast to the actual type of the function
this would solve the problem fully.

Unfortunately the Fortran parser doesn't support parsing function
signatures right now.

> I'm sure you recall but this used to be different, and gdb just assumed
> that functions returned int, which was an ongoing source of confusion.
> I wonder whether this setting will also cause confusion.

Having thought about this I agree with you.  I think a solution where
we cast to the actual function type will be less confusion than having
flags to toggle.

Like I said, the Fortran parser doesn't currently support parsing
function type signatures.

But I think we can provide a first cut of this patch that solves the
initial bug report without having either the switch I originally
proposed, or an updated parser.

Then we can come back later and extend the parser to offer an even
better experience.

So, the patch below focuses on just the initial problem of accessing
outside the argument type list.  This stops GDB crashing.

I do also provide a slightly tweaked default behaviour for how GDB
handles arguments of unknown type, see below for full details.

All feedback welcome,

Thanks,
Andrew

--

commit e0aa5c0a333bbef706ef5c8085977d4a730208a9
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Fri Nov 13 10:39:23 2020 +0000

    gdb/fortran: don't access non-existent type fields
    
    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.
    
    I an ideal solution for this problem would be to allow the user to
    cast the function to be the correct type, we already do this to some
    degree with the return value, for example:
    
      (gdb) print some_func_ ()
      'some_func_' has unknown return type; cast the call to its declared return type
      (gdb) print (integer) some_func_ ()
      $1 = 1
    
    But if we could extend this to allow casting to the full function type
    we could figure out from the signature what the real parameters, any
    maybe figure out what artificial parameters we should expect.  Maybe
    something like this:
    
      (gdb) print ((integer () (integer, double)) some_other_func_ (1, 2.3)
    
    Alas, right now the Fortran expression parser doesn't seem to support
    parsing function signatures, and we certainly don't have support for
    figuring out real vs artificial arguments from a signature.
    
    Still, I think we can prevent GDB from accessing undefined memory and
    provide a reasonable default behaviour.
    
    In this commit I:
    
      - Only ask if the argument is artificial if the type of the argument
      is actually known.
    
      - Unknown arguments are assumed to be artificial and passed by
      value (non-artificial arguments are pass by reference).
    
      - If an artificial argument is prefixed with '&' by the user then we
      treat the argument as pass-by-reference.
    
    With these three changes we avoid undefined behaviour in GDB, and
    allow the user, in most cases, to get a reasonably natural default
    behaviour.
    
    gdb/ChangeLog:
    
            PR fortran/26155
            * f-lang.c (fortran_argument_convert): Delete declaration.
            (fortran_prepare_argument): New function.
            (evaluate_subexp_f): Move logic to new function
            fortran_prepare_argument.
    
    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.

diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 57dd2ed7e31..80cdf88a726 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -68,8 +68,10 @@ show_fortran_array_slicing_debug (struct ui_file *file, int from_tty,
 
 /* Local functions */
 
-static struct value *fortran_argument_convert (struct value *value,
-					       bool is_artificial);
+static value *fortran_prepare_argument (struct expression *exp, int *pos,
+					int arg_num, bool is_internal_call_p,
+					struct type *func_type,
+					enum noside noside);
 
 /* Return the encoding that should be used for the character type
    TYPE.  */
@@ -1065,22 +1067,11 @@ evaluate_subexp_f (struct type *expect_type, struct expression *exp,
 	    int 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)
@@ -1541,6 +1532,59 @@ fortran_argument_convert (struct value *value, bool is_artificial)
     return value;
 }
 
+/* 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).
+
+   Arguments in Fortran are normally passed by address, we 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.  */
+
+static 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 = ((arg_num >= func_type->num_fields ())
+			? true
+			: TYPE_FIELD_ARTIFICIAL (func_type, arg_num));
+
+  /* If this is an artificial argument, then either, this is an argument
+     beyond the end of the known arguments, or possibly, there are no known
+     arguments (maybe missing debug info).
+
+     For these artificial arguments, if the user has prefixed it with '&'
+     (for address-of), then lets always allow this to succeed, even if the
+     argument is not actually in inferior memory.  This will allow the user
+     to pass arguments to a Fortran function even when there's no debug
+     information.
+
+     As we already pass the address of non-artificial arguments, all we
+     need to do if skip the UNOP_ADDR operator in the expression and mark
+     the argument as non-artificial.  */
+  if (is_artificial && exp->elts[*pos].opcode == UNOP_ADDR)
+    {
+      (*pos)++;
+      is_artificial = false;
+    }
+
+  struct value *arg_val = evaluate_subexp_with_coercion (exp, pos, noside);
+  return fortran_argument_convert (arg_val, is_artificial);
+}
+
 /* See f-lang.h.  */
 
 struct type *
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..4ede0b44fd4
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/call-no-debug.exp
@@ -0,0 +1,102 @@
+# 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.  As the function has no debug information then we have
+# to pass the integer argument as '&1' so that GDB will send the
+# address of an integer '1' (as Fortran arguments are pass by
+# reference).
+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"
+}
+
+# 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.
+#
+# The compiled program is expecting the address of the string, so we
+# prefix that argument with '&', but the artificial length parameter
+# is pass by value, so there's no need for '&' in that case.
+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 "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"
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] gdb/fortran: Don't access non-existent type fields
  2021-02-11 15:48     ` Andrew Burgess
@ 2021-02-24 17:47       ` Andrew Burgess
  2021-02-24 20:47         ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Burgess @ 2021-02-24 17:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Ping!

I'm planning to merge the second version of this patch in the near
future.  It's much smaller in scope than the first, and just focuses
on preventing undefined behaviour, rather than adding any new
features.

Let me know if there's any feedback.

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2021-02-11 15:48:58 +0000]:

> * Tom Tromey <tom@tromey.com> [2020-10-20 13:32:55 -0600]:
> 
> > >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> > 
> > Andrew> Updated for latest GDB master.  Things have moved around a bit, but no
> > Andrew> significant code changes.
> > 
> > Andrew> No changes at all in docs since v1.
> > 
> > This contents of the patch seem fine to me.
> > I just wanted to note something about the direction.
> > 
> > Andrew>     When considering what was the "correct" default to assume I struggled
> > Andrew>     to decide, both assumptions seem to have advantages.
> > 
> > In C what is done now is to make the default case an error, and then
> > require the user to cast the function to the correct type.  This way
> > return types are handled correctly as well.
> 
> I think you can mostly get away with casting to the correct return
> type, right?  This is certainly the case in Fortran, even in this
> patch, so:
> 
>   (gdb) some_func_ ()
>   'some_func_' has unknown return type; cast the call to its declared return type
> 
> But I think that if we could cast to the actual type of the function
> this would solve the problem fully.
> 
> Unfortunately the Fortran parser doesn't support parsing function
> signatures right now.
> 
> > I'm sure you recall but this used to be different, and gdb just assumed
> > that functions returned int, which was an ongoing source of confusion.
> > I wonder whether this setting will also cause confusion.
> 
> Having thought about this I agree with you.  I think a solution where
> we cast to the actual function type will be less confusion than having
> flags to toggle.
> 
> Like I said, the Fortran parser doesn't currently support parsing
> function type signatures.
> 
> But I think we can provide a first cut of this patch that solves the
> initial bug report without having either the switch I originally
> proposed, or an updated parser.
> 
> Then we can come back later and extend the parser to offer an even
> better experience.
> 
> So, the patch below focuses on just the initial problem of accessing
> outside the argument type list.  This stops GDB crashing.
> 
> I do also provide a slightly tweaked default behaviour for how GDB
> handles arguments of unknown type, see below for full details.
> 
> All feedback welcome,
> 
> Thanks,
> Andrew
> 
> --
> 
> commit e0aa5c0a333bbef706ef5c8085977d4a730208a9
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> Date:   Fri Nov 13 10:39:23 2020 +0000
> 
>     gdb/fortran: don't access non-existent type fields
>     
>     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.
>     
>     I an ideal solution for this problem would be to allow the user to
>     cast the function to be the correct type, we already do this to some
>     degree with the return value, for example:
>     
>       (gdb) print some_func_ ()
>       'some_func_' has unknown return type; cast the call to its declared return type
>       (gdb) print (integer) some_func_ ()
>       $1 = 1
>     
>     But if we could extend this to allow casting to the full function type
>     we could figure out from the signature what the real parameters, any
>     maybe figure out what artificial parameters we should expect.  Maybe
>     something like this:
>     
>       (gdb) print ((integer () (integer, double)) some_other_func_ (1, 2.3)
>     
>     Alas, right now the Fortran expression parser doesn't seem to support
>     parsing function signatures, and we certainly don't have support for
>     figuring out real vs artificial arguments from a signature.
>     
>     Still, I think we can prevent GDB from accessing undefined memory and
>     provide a reasonable default behaviour.
>     
>     In this commit I:
>     
>       - Only ask if the argument is artificial if the type of the argument
>       is actually known.
>     
>       - Unknown arguments are assumed to be artificial and passed by
>       value (non-artificial arguments are pass by reference).
>     
>       - If an artificial argument is prefixed with '&' by the user then we
>       treat the argument as pass-by-reference.
>     
>     With these three changes we avoid undefined behaviour in GDB, and
>     allow the user, in most cases, to get a reasonably natural default
>     behaviour.
>     
>     gdb/ChangeLog:
>     
>             PR fortran/26155
>             * f-lang.c (fortran_argument_convert): Delete declaration.
>             (fortran_prepare_argument): New function.
>             (evaluate_subexp_f): Move logic to new function
>             fortran_prepare_argument.
>     
>     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.
> 
> diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> index 57dd2ed7e31..80cdf88a726 100644
> --- a/gdb/f-lang.c
> +++ b/gdb/f-lang.c
> @@ -68,8 +68,10 @@ show_fortran_array_slicing_debug (struct ui_file *file, int from_tty,
>  
>  /* Local functions */
>  
> -static struct value *fortran_argument_convert (struct value *value,
> -					       bool is_artificial);
> +static value *fortran_prepare_argument (struct expression *exp, int *pos,
> +					int arg_num, bool is_internal_call_p,
> +					struct type *func_type,
> +					enum noside noside);
>  
>  /* Return the encoding that should be used for the character type
>     TYPE.  */
> @@ -1065,22 +1067,11 @@ evaluate_subexp_f (struct type *expect_type, struct expression *exp,
>  	    int 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)
> @@ -1541,6 +1532,59 @@ fortran_argument_convert (struct value *value, bool is_artificial)
>      return value;
>  }
>  
> +/* 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).
> +
> +   Arguments in Fortran are normally passed by address, we 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.  */
> +
> +static 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 = ((arg_num >= func_type->num_fields ())
> +			? true
> +			: TYPE_FIELD_ARTIFICIAL (func_type, arg_num));
> +
> +  /* If this is an artificial argument, then either, this is an argument
> +     beyond the end of the known arguments, or possibly, there are no known
> +     arguments (maybe missing debug info).
> +
> +     For these artificial arguments, if the user has prefixed it with '&'
> +     (for address-of), then lets always allow this to succeed, even if the
> +     argument is not actually in inferior memory.  This will allow the user
> +     to pass arguments to a Fortran function even when there's no debug
> +     information.
> +
> +     As we already pass the address of non-artificial arguments, all we
> +     need to do if skip the UNOP_ADDR operator in the expression and mark
> +     the argument as non-artificial.  */
> +  if (is_artificial && exp->elts[*pos].opcode == UNOP_ADDR)
> +    {
> +      (*pos)++;
> +      is_artificial = false;
> +    }
> +
> +  struct value *arg_val = evaluate_subexp_with_coercion (exp, pos, noside);
> +  return fortran_argument_convert (arg_val, is_artificial);
> +}
> +
>  /* See f-lang.h.  */
>  
>  struct type *
> 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..4ede0b44fd4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/call-no-debug.exp
> @@ -0,0 +1,102 @@
> +# 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.  As the function has no debug information then we have
> +# to pass the integer argument as '&1' so that GDB will send the
> +# address of an integer '1' (as Fortran arguments are pass by
> +# reference).
> +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"
> +}
> +
> +# 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.
> +#
> +# The compiled program is expecting the address of the string, so we
> +# prefix that argument with '&', but the artificial length parameter
> +# is pass by value, so there's no need for '&' in that case.
> +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 "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"
> +}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] gdb/fortran: Don't access non-existent type fields
  2021-02-24 17:47       ` Andrew Burgess
@ 2021-02-24 20:47         ` Tom Tromey
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2021-02-24 20:47 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Ping!
Andrew> I'm planning to merge the second version of this patch in the near
Andrew> future.  It's much smaller in scope than the first, and just focuses
Andrew> on preventing undefined behaviour, rather than adding any new
Andrew> features.

Andrew> Let me know if there's any feedback.

It seems alright to me.  Thank you.

Tom

^ 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).