[RFA] Need post-processing of parameters for function calls in Ada

[RFA] Need post-processing of parameters for function calls in Ada

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

Hello,

Currently, calling an Ada function that takes an array does not work.
Let's take an example that uses a String:

    (gdb) call call_me ("bonjour")
    Invalid cast.

The problem in our case is that there are many different possible
representations for an array, and the representation that is passed
during the function call does not match what the function expects.
So we try a conversion and kaboum!

Because the conversion is so Ada specific (conversion from one form
of array to another), the conversion needs to be performed by the
Ada language.  The attached patch adds a call to ada_convert_actuals()
inside call_function_by_hand when doing the call in Ada.  We need
to do the conversion there rather than ahead of calling call_function_
by_hand because some conversions need to allocate some memory in
the inferior, and we do it in the stack area that this function sets up.

2008-01-08  Joel Brobecker  <brobecker@adacore.com>

        * infcall.c: #include "ada-lang.h".
        (call_function_by_hand): Add post processing of function arguments
        for Ada function calls.
        * Makefile.in (infcall.o): Update dependencies.

I was able to write a testcase that reproduces the problem:

2008-01-08  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/arrayparam: New test program.
        * gdb.ada/arrayparam.exp: New testcase.

Tested on x86-linux, no regression.
OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: arrayparam.diff --]
[-- Type: text/plain, Size: 1317 bytes --]

Index: infcall.c
===================================================================
--- infcall.c	(revision 98)
+++ infcall.c	(revision 99)
@@ -34,6 +34,7 @@
 #include "gdb_string.h"
 #include "infcall.h"
 #include "dummy-frame.h"
+#include "ada-lang.h"
 
 /* NOTE: cagney/2003-04-16: What's the future of this code?
 
@@ -556,6 +557,9 @@ call_function_by_hand (struct value *fun
   if (nargs < TYPE_NFIELDS (ftype))
     error (_("too few arguments in function call"));
 
+  if (current_language->la_language == language_ada)
+    ada_convert_actuals (function, nargs, args, &sp);
+
   {
     int i;
     for (i = nargs - 1; i >= 0; i--)
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 98)
+++ Makefile.in	(revision 99)
@@ -2271,7 +2271,7 @@ ia64-tdep.o: ia64-tdep.c $(defs_h) $(inf
 infcall.o: infcall.c $(defs_h) $(breakpoint_h) $(target_h) $(regcache_h) \
 	$(inferior_h) $(gdb_assert_h) $(block_h) $(gdbcore_h) $(language_h) \
 	$(objfiles_h) $(gdbcmd_h) $(command_h) $(gdb_string_h) $(infcall_h) \
-	$(dummy_frame_h)
+	$(dummy_frame_h) $(ada_lang_h)
 inf-child.o: inf-child.c $(defs_h) $(regcache_h) $(memattr_h) $(symtab_h) \
 	$(target_h) $(inferior_h) $(gdb_string_h)
 infcmd.o: infcmd.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \

[-- Attachment #3: arrayparam-tc.diff --]
[-- Type: text/plain, Size: 5467 bytes --]

Index: gdb.ada/arrayparam.exp
===================================================================
--- gdb.ada/arrayparam.exp	(revision 0)
+++ gdb.ada/arrayparam.exp	(revision 100)
@@ -0,0 +1,61 @@
+# Copyright 2008 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "arrayparam"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+# Verify that a call to a function that takes an array as a parameter
+# works without problem.
+
+gdb_test "print call_me (\"bonjour\")" \
+         "void" \
+         "print call_me (\"bonjour\")"
+
+# Verify that the array was passed properly by checking the global
+# variables that Call_Me sets as side-effects.
+
+gdb_test "print first" \
+         "98 'b'" \
+         "print first after function call"
+
+gdb_test "print last" \
+         "114 'r'" \
+         "print lasta after function call"
+
+gdb_test "print length" \
+         "7" \
+         "print length after function call"
+
Index: gdb.ada/arrayparam/pck.adb
===================================================================
--- gdb.ada/arrayparam/pck.adb	(revision 0)
+++ gdb.ada/arrayparam/pck.adb	(revision 100)
@@ -0,0 +1,28 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+
+   procedure Call_Me (Str : String) is
+   begin
+      Length := Str'Length;
+      if Length > 0 then
+         First := Str (Str'First);
+         Last := Str (Str'Last);
+      end if;
+   end Call_Me;
+
+end Pck;
+
Index: gdb.ada/arrayparam/pck.ads
===================================================================
--- gdb.ada/arrayparam/pck.ads	(revision 0)
+++ gdb.ada/arrayparam/pck.ads	(revision 100)
@@ -0,0 +1,25 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Pck is
+
+   First : Character := ASCII.NUL;
+   Last : Character := ASCII.NUL;
+   Length : Integer := 0;
+
+   procedure Call_Me (Str : String);
+
+end Pck;
+
Index: gdb.ada/arrayparam/foo.adb
===================================================================
--- gdb.ada/arrayparam/foo.adb	(revision 0)
+++ gdb.ada/arrayparam/foo.adb	(revision 100)
@@ -0,0 +1,26 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   My_String : constant String := "Hello World";
+begin
+   First := ASCII.NUL;
+   Last := ASCII.NUL;
+   Length := -1;
+   Call_Me (My_String);  -- STOP
+end Foo;
+

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Daniel Jacobowitz]

On Tue, Jan 08, 2008 at 06:36:22AM -0800, Joel Brobecker wrote:
> Because the conversion is so Ada specific (conversion from one form
> of array to another), the conversion needs to be performed by the
> Ada language.  The attached patch adds a call to ada_convert_actuals()
> inside call_function_by_hand when doing the call in Ada.  We need
> to do the conversion there rather than ahead of calling call_function_
> by_hand because some conversions need to allocate some memory in
> the inferior, and we do it in the stack area that this function sets up.

Is this an argument coercion?  The right place may be
value_arg_coerce.  If the only problem is that you need the sp, we
could supply it to that function and move the coercion after the stack
frame setup.  I don't think this will be a problem even if we need a
function call during argument coercion.

This might allow a useful option, to avoid calling malloc when passing
strings to a C function.  We can not do that in general, since the
function might save the pointer, but there are some cases (like strcmp).

Really this sort of thing ought to go through the language vector, but
I'm happy leaving that until a second language needs it.

> 
> 2008-01-08  Joel Brobecker  <brobecker@adacore.com>
> 
>         * infcall.c: #include "ada-lang.h".
>         (call_function_by_hand): Add post processing of function arguments
>         for Ada function calls.
>         * Makefile.in (infcall.o): Update dependencies.
> 
> I was able to write a testcase that reproduces the problem:
> 
> 2008-01-08  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdb.ada/arrayparam: New test program.
>         * gdb.ada/arrayparam.exp: New testcase.
> 
> Tested on x86-linux, no regression.
> OK to commit?
> 
> Thanks,
> -- 
> Joel

> Index: infcall.c
> ===================================================================
> --- infcall.c	(revision 98)
> +++ infcall.c	(revision 99)
> @@ -34,6 +34,7 @@
>  #include "gdb_string.h"
>  #include "infcall.h"
>  #include "dummy-frame.h"
> +#include "ada-lang.h"
>  
>  /* NOTE: cagney/2003-04-16: What's the future of this code?
>  
> @@ -556,6 +557,9 @@ call_function_by_hand (struct value *fun
>    if (nargs < TYPE_NFIELDS (ftype))
>      error (_("too few arguments in function call"));
>  
> +  if (current_language->la_language == language_ada)
> +    ada_convert_actuals (function, nargs, args, &sp);
> +
>    {
>      int i;
>      for (i = nargs - 1; i >= 0; i--)
> Index: Makefile.in
> ===================================================================
> --- Makefile.in	(revision 98)
> +++ Makefile.in	(revision 99)
> @@ -2271,7 +2271,7 @@ ia64-tdep.o: ia64-tdep.c $(defs_h) $(inf
>  infcall.o: infcall.c $(defs_h) $(breakpoint_h) $(target_h) $(regcache_h) \
>  	$(inferior_h) $(gdb_assert_h) $(block_h) $(gdbcore_h) $(language_h) \
>  	$(objfiles_h) $(gdbcmd_h) $(command_h) $(gdb_string_h) $(infcall_h) \
> -	$(dummy_frame_h)
> +	$(dummy_frame_h) $(ada_lang_h)
>  inf-child.o: inf-child.c $(defs_h) $(regcache_h) $(memattr_h) $(symtab_h) \
>  	$(target_h) $(inferior_h) $(gdb_string_h)
>  infcmd.o: infcmd.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \

> Index: gdb.ada/arrayparam.exp
> ===================================================================
> --- gdb.ada/arrayparam.exp	(revision 0)
> +++ gdb.ada/arrayparam.exp	(revision 100)
> @@ -0,0 +1,61 @@
> +# Copyright 2008 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +if $tracelevel then {
> +    strace $tracelevel
> +}
> +
> +load_lib "ada.exp"
> +
> +set testdir "arrayparam"
> +set testfile "${testdir}/foo"
> +set srcfile ${srcdir}/${subdir}/${testfile}.adb
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +file mkdir ${objdir}/${subdir}/${testdir}
> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
> +  return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
> +
> +set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
> +runto "foo.adb:$bp_location"
> +
> +# Verify that a call to a function that takes an array as a parameter
> +# works without problem.
> +
> +gdb_test "print call_me (\"bonjour\")" \
> +         "void" \
> +         "print call_me (\"bonjour\")"
> +
> +# Verify that the array was passed properly by checking the global
> +# variables that Call_Me sets as side-effects.
> +
> +gdb_test "print first" \
> +         "98 'b'" \
> +         "print first after function call"
> +
> +gdb_test "print last" \
> +         "114 'r'" \
> +         "print lasta after function call"
> +
> +gdb_test "print length" \
> +         "7" \
> +         "print length after function call"
> +
> Index: gdb.ada/arrayparam/pck.adb
> ===================================================================
> --- gdb.ada/arrayparam/pck.adb	(revision 0)
> +++ gdb.ada/arrayparam/pck.adb	(revision 100)
> @@ -0,0 +1,28 @@
> +--  Copyright 2008 Free Software Foundation, Inc.
> +--
> +--  This program is free software; you can redistribute it and/or modify
> +--  it under the terms of the GNU General Public License as published by
> +--  the Free Software Foundation; either version 3 of the License, or
> +--  (at your option) any later version.
> +--
> +--  This program is distributed in the hope that it will be useful,
> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +--  GNU General Public License for more details.
> +--
> +--  You should have received a copy of the GNU General Public License
> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +package body Pck is
> +
> +   procedure Call_Me (Str : String) is
> +   begin
> +      Length := Str'Length;
> +      if Length > 0 then
> +         First := Str (Str'First);
> +         Last := Str (Str'Last);
> +      end if;
> +   end Call_Me;
> +
> +end Pck;
> +
> Index: gdb.ada/arrayparam/pck.ads
> ===================================================================
> --- gdb.ada/arrayparam/pck.ads	(revision 0)
> +++ gdb.ada/arrayparam/pck.ads	(revision 100)
> @@ -0,0 +1,25 @@
> +--  Copyright 2008 Free Software Foundation, Inc.
> +--
> +--  This program is free software; you can redistribute it and/or modify
> +--  it under the terms of the GNU General Public License as published by
> +--  the Free Software Foundation; either version 3 of the License, or
> +--  (at your option) any later version.
> +--
> +--  This program is distributed in the hope that it will be useful,
> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +--  GNU General Public License for more details.
> +--
> +--  You should have received a copy of the GNU General Public License
> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +package Pck is
> +
> +   First : Character := ASCII.NUL;
> +   Last : Character := ASCII.NUL;
> +   Length : Integer := 0;
> +
> +   procedure Call_Me (Str : String);
> +
> +end Pck;
> +
> Index: gdb.ada/arrayparam/foo.adb
> ===================================================================
> --- gdb.ada/arrayparam/foo.adb	(revision 0)
> +++ gdb.ada/arrayparam/foo.adb	(revision 100)
> @@ -0,0 +1,26 @@
> +--  Copyright 2008 Free Software Foundation, Inc.
> +--
> +--  This program is free software; you can redistribute it and/or modify
> +--  it under the terms of the GNU General Public License as published by
> +--  the Free Software Foundation; either version 3 of the License, or
> +--  (at your option) any later version.
> +--
> +--  This program is distributed in the hope that it will be useful,
> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +--  GNU General Public License for more details.
> +--
> +--  You should have received a copy of the GNU General Public License
> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +with Pck; use Pck;
> +
> +procedure Foo is
> +   My_String : constant String := "Hello World";
> +begin
> +   First := ASCII.NUL;
> +   Last := ASCII.NUL;
> +   Length := -1;
> +   Call_Me (My_String);  -- STOP
> +end Foo;
> +


-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Joel Brobecker]

Thanks muchly for the quick feedback!

> Is this an argument coercion?  The right place may be
> value_arg_coerce.

Indeed. It looks like the two functions are doing the same thing,
but for different languages.

> If the only problem is that you need the sp, we
> could supply it to that function and move the coercion after the stack
> frame setup.  I don't think this will be a problem even if we need a
> function call during argument coercion.

(funny that you're talking about function call during argument coercion,
that will be my next patch which I just finished ;-).

I don't think we'll need to move the argument coercion phase...

> This might allow a useful option, to avoid calling malloc when passing
> strings to a C function.  We can not do that in general, since the
> function might save the pointer, but there are some cases (like strcmp).

... unless that is needed to allow calling malloc. But this can be
discussed independently, if you prefer.

> Really this sort of thing ought to go through the language vector, but
> I'm happy leaving that until a second language needs it.

I agree. I propose the following:

  - Add a new la_value_arg_coerce method in the language vector.
    The prototype would be identical to the current value_arg_coerce
    but with the added SP.

  - rename value_arg_coerce c_value_arg_coerce. Possibly move it to
    c-lang.
  
  - Make all languages use c_value_arg_coerce, except for Ada, which
    would use our own coerce routine.

That should take care of everything (except maybe allowing malloc
calls).

My only concern is that the Ada coerce might be relying on the C coerce
routine to handle the simple cases. So I might need to call the C coerce
routine at the end of the Ada coerce routine. I need to look deeper
into this.

What do you think?

Thanks,
-- 
Joel

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Daniel Jacobowitz]

On Tue, Jan 08, 2008 at 07:29:00AM -0800, Joel Brobecker wrote:
> I don't think we'll need to move the argument coercion phase...
> 
> > This might allow a useful option, to avoid calling malloc when passing
> > strings to a C function.  We can not do that in general, since the
> > function might save the pointer, but there are some cases (like strcmp).
> 
> ... unless that is needed to allow calling malloc. But this can be
> discussed independently, if you prefer.

You're right.  I misread the code; sp is already properly set up by
this point.

> I agree. I propose the following:
> 
>   - Add a new la_value_arg_coerce method in the language vector.
>     The prototype would be identical to the current value_arg_coerce
>     but with the added SP.
> 
>   - rename value_arg_coerce c_value_arg_coerce. Possibly move it to
>     c-lang.
>   
>   - Make all languages use c_value_arg_coerce, except for Ada, which
>     would use our own coerce routine.
> 
> That should take care of everything (except maybe allowing malloc
> calls).
> 
> My only concern is that the Ada coerce might be relying on the C coerce
> routine to handle the simple cases. So I might need to call the C coerce
> routine at the end of the Ada coerce routine. I need to look deeper
> into this.

Does the existing routine do anything inappropriate for Ada?  I
suspect it is appropriate for all languages, and we can call a
language-specific routine in addition to it.  But if it does anything
you'd rather it didn't, then we should let Ada avoid it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Joel Brobecker]

> > My only concern is that the Ada coerce might be relying on the C coerce
> > routine to handle the simple cases. So I might need to call the C coerce
> > routine at the end of the Ada coerce routine. I need to look deeper
> > into this.
> 
> Does the existing routine do anything inappropriate for Ada?  I
> suspect it is appropriate for all languages, and we can call a
> language-specific routine in addition to it.  But if it does anything
> you'd rather it didn't, then we should let Ada avoid it.

I don't think it does. In fact, we are actually still calling it
after we're done with the ada_convert_actuals:

  if (current_language->la_language == language_ada)
    ada_convert_actuals (function, nargs, args, &sp);

  {
    int i;
    for (i = nargs - 1; i >= 0; i--)
      {
        [...]

        args[i] = value_arg_coerce (args[i], param_type, prototyped);

        [...]
      }

I am just wondering whether this is actually useful at all or not.
It depends whether value_arg_coerce covers some cases that the Ada
routine doesn't and yet needs. That's on my end to check. Worst that
would happen is that I would call the C routine from the Ada one.

Should I proceed with my proposal?

-- 
Joel

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Daniel Jacobowitz]

On Tue, Jan 08, 2008 at 07:49:41AM -0800, Joel Brobecker wrote:
> Should I proceed with my proposal?

I would prefer to put the call inside the existing value_arg_coerce,
at least to start with.

Otherwise, yes please!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Joel Brobecker]

> > Should I proceed with my proposal?
> 
> I would prefer to put the call inside the existing value_arg_coerce,
> at least to start with.

Do you mean: dropping the new language field, and just add a call
to the Ada coerce routine inside value_arg_coerce (or in other words,
more the change I first proposed inside value_arg_coerce)? That's
a much smaller change, and I'm OK with that!

-- 
Joel

Re: [RFA] Need post-processing of parameters for function calls in Ada

[Daniel Jacobowitz]

On Tue, Jan 08, 2008 at 08:13:37AM -0800, Joel Brobecker wrote:
> > > Should I proceed with my proposal?
> > 
> > I would prefer to put the call inside the existing value_arg_coerce,
> > at least to start with.
> 
> Do you mean: dropping the new language field, and just add a call
> to the Ada coerce routine inside value_arg_coerce (or in other words,
> more the change I first proposed inside value_arg_coerce)? That's
> a much smaller change, and I'm OK with that!

Either that, or calling the language hook from value_arg_coerce.  I'm
happy to just call the Ada function directly.

-- 
Daniel Jacobowitz
CodeSourcery

[RFA] Fix parameter coercing for Ada function calls (take 2)

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

Hi Daniel,

Does this look like what you had in mind? It's a little more work
than my first patch, but indeed a little more elegant.

2008-01-08  Joel Brobecker  <brobecker@adacore.com>

        * ada-lang.c (ada_convert_actual): Renames convert_actual.
        Make non-static.
        (ada_convert_actuals): Delete.
        * ada-lang.c (ada_convert_actual): Add declaration.
        (ada_convert_actuals): Remove declaration.
        * infcall.c: #include "ada-lang.h".
        (value_arg_coerce): Add new parameter sp.  Add handling of Ada
        function call parameters.
        * Makefile.in (infcall.o): Update dependencies.

I am also reposting the testcase for everyone's convenience
(I feel sufficiently confident about the testcase that's very simple
to auto-review myself, but a second pair of eyes is always appreciated).

2008-01-08  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/arrayparam: New test program.
        * gdb.ada/arrayparam.exp: New testcase.

All tested on x86-linux, no regression.

OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: arrayparam.diff --]
[-- Type: text/plain, Size: 4687 bytes --]

Index: ada-lang.c
===================================================================
--- ada-lang.c	(revision 107)
+++ ada-lang.c	(revision 108)
@@ -3942,9 +3942,9 @@ ensure_lval (struct value *val, CORE_ADD
    allocating any necessary descriptors (fat pointers), or copies of
    values not residing in memory, updating it as needed.  */
 
-static struct value *
-convert_actual (struct value *actual, struct type *formal_type0,
-                CORE_ADDR *sp)
+struct value *
+ada_convert_actual (struct value *actual, struct type *formal_type0,
+                    CORE_ADDR *sp)
 {
   struct type *actual_type = ada_check_typedef (value_type (actual));
   struct type *formal_type = ada_check_typedef (formal_type0);
@@ -4036,30 +4036,6 @@ make_array_descriptor (struct type *type
   else
     return descriptor;
 }
-
-
-/* Assuming a dummy frame has been established on the target, perform any
-   conversions needed for calling function FUNC on the NARGS actual
-   parameters in ARGS, other than standard C conversions.  Does
-   nothing if FUNC does not have Ada-style prototype data, or if NARGS
-   does not match the number of arguments expected.  Use *SP as a
-   stack pointer for additional data that must be pushed, updating its
-   value as needed.  */
-
-void
-ada_convert_actuals (struct value *func, int nargs, struct value *args[],
-                     CORE_ADDR *sp)
-{
-  int i;
-
-  if (TYPE_NFIELDS (value_type (func)) == 0
-      || nargs != TYPE_NFIELDS (value_type (func)))
-    return;
-
-  for (i = 0; i < nargs; i += 1)
-    args[i] =
-      convert_actual (args[i], TYPE_FIELD_TYPE (value_type (func), i), sp);
-}
 \f
 /* Dummy definitions for an experimental caching module that is not
  * used in the public sources. */
Index: ada-lang.h
===================================================================
--- ada-lang.h	(revision 107)
+++ ada-lang.h	(revision 108)
@@ -278,8 +278,9 @@ extern void ada_printchar (int, struct u
 extern void ada_printstr (struct ui_file *, const gdb_byte *,
 			  unsigned int, int, int);
 
-extern void ada_convert_actuals (struct value *, int, struct value **,
-                                 CORE_ADDR *);
+struct value *ada_convert_actual (struct value *actual,
+                                  struct type *formal_type0,
+                                  CORE_ADDR *sp);
 
 extern struct value *ada_value_subscript (struct value *, int,
                                           struct value **);
Index: infcall.c
===================================================================
--- infcall.c	(revision 107)
+++ infcall.c	(revision 108)
@@ -34,6 +34,7 @@
 #include "gdb_string.h"
 #include "infcall.h"
 #include "dummy-frame.h"
+#include "ada-lang.h"
 
 /* NOTE: cagney/2003-04-16: What's the future of this code?
 
@@ -91,19 +92,23 @@ Unwinding of stack if a signal is receiv
 
 
 /* Perform the standard coercions that are specified
-   for arguments to be passed to C functions.
+   for arguments to be passed to C or Ada functions.
 
    If PARAM_TYPE is non-NULL, it is the expected parameter type.
    IS_PROTOTYPED is non-zero if the function declaration is prototyped.  */
 
 static struct value *
 value_arg_coerce (struct value *arg, struct type *param_type,
-		  int is_prototyped)
+		  int is_prototyped, CORE_ADDR *pc)
 {
   struct type *arg_type = check_typedef (value_type (arg));
   struct type *type
     = param_type ? check_typedef (param_type) : arg_type;
 
+  /* Perform any Ada-specific coercion first.  */
+  if (current_language->la_language == language_ada)
+    arg = ada_convert_actual (arg, type, pc);
+
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_REF:
@@ -577,7 +582,7 @@ call_function_by_hand (struct value *fun
 	else
 	  param_type = NULL;
 
-	args[i] = value_arg_coerce (args[i], param_type, prototyped);
+	args[i] = value_arg_coerce (args[i], param_type, prototyped, &sp);
 
 	if (param_type != NULL && language_pass_by_reference (param_type))
 	  args[i] = value_addr (args[i]);
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 107)
+++ Makefile.in	(revision 108)
@@ -2271,7 +2271,7 @@ ia64-tdep.o: ia64-tdep.c $(defs_h) $(inf
 infcall.o: infcall.c $(defs_h) $(breakpoint_h) $(target_h) $(regcache_h) \
 	$(inferior_h) $(gdb_assert_h) $(block_h) $(gdbcore_h) $(language_h) \
 	$(objfiles_h) $(gdbcmd_h) $(command_h) $(gdb_string_h) $(infcall_h) \
-	$(dummy_frame_h)
+	$(dummy_frame_h) $(ada_lang_h)
 inf-child.o: inf-child.c $(defs_h) $(regcache_h) $(memattr_h) $(symtab_h) \
 	$(target_h) $(inferior_h) $(gdb_string_h)
 infcmd.o: infcmd.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \

[-- Attachment #3: arrayparam-tc.diff --]
[-- Type: text/plain, Size: 5467 bytes --]

Index: gdb.ada/arrayparam.exp
===================================================================
--- gdb.ada/arrayparam.exp	(revision 0)
+++ gdb.ada/arrayparam.exp	(revision 100)
@@ -0,0 +1,61 @@
+# Copyright 2008 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "arrayparam"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+# Verify that a call to a function that takes an array as a parameter
+# works without problem.
+
+gdb_test "print call_me (\"bonjour\")" \
+         "void" \
+         "print call_me (\"bonjour\")"
+
+# Verify that the array was passed properly by checking the global
+# variables that Call_Me sets as side-effects.
+
+gdb_test "print first" \
+         "98 'b'" \
+         "print first after function call"
+
+gdb_test "print last" \
+         "114 'r'" \
+         "print lasta after function call"
+
+gdb_test "print length" \
+         "7" \
+         "print length after function call"
+
Index: gdb.ada/arrayparam/pck.adb
===================================================================
--- gdb.ada/arrayparam/pck.adb	(revision 0)
+++ gdb.ada/arrayparam/pck.adb	(revision 100)
@@ -0,0 +1,28 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+
+   procedure Call_Me (Str : String) is
+   begin
+      Length := Str'Length;
+      if Length > 0 then
+         First := Str (Str'First);
+         Last := Str (Str'Last);
+      end if;
+   end Call_Me;
+
+end Pck;
+
Index: gdb.ada/arrayparam/pck.ads
===================================================================
--- gdb.ada/arrayparam/pck.ads	(revision 0)
+++ gdb.ada/arrayparam/pck.ads	(revision 100)
@@ -0,0 +1,25 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Pck is
+
+   First : Character := ASCII.NUL;
+   Last : Character := ASCII.NUL;
+   Length : Integer := 0;
+
+   procedure Call_Me (Str : String);
+
+end Pck;
+
Index: gdb.ada/arrayparam/foo.adb
===================================================================
--- gdb.ada/arrayparam/foo.adb	(revision 0)
+++ gdb.ada/arrayparam/foo.adb	(revision 100)
@@ -0,0 +1,26 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   My_String : constant String := "Hello World";
+begin
+   First := ASCII.NUL;
+   Last := ASCII.NUL;
+   Length := -1;
+   Call_Me (My_String);  -- STOP
+end Foo;
+

Re: [RFA] Fix parameter coercing for Ada function calls (take 2)

[Daniel Jacobowitz]

On Tue, Jan 08, 2008 at 10:52:19AM -0800, Joel Brobecker wrote:
> Hi Daniel,
> 
> Does this look like what you had in mind? It's a little more work
> than my first patch, but indeed a little more elegant.

Yes, exactly.

>     If PARAM_TYPE is non-NULL, it is the expected parameter type.
>     IS_PROTOTYPED is non-zero if the function declaration is prototyped.  */

Please add the new parameter to the comment.

>  static struct value *
>  value_arg_coerce (struct value *arg, struct type *param_type,
> -		  int is_prototyped)
> +		  int is_prototyped, CORE_ADDR *pc)

... and call it sp :-)


-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Fix parameter coercing for Ada function calls (take 2)

[Joel Brobecker]

> > Does this look like what you had in mind? It's a little more work
> > than my first patch, but indeed a little more elegant.
> 
> Yes, exactly.

Cool! Attached is what I ended checking in.

> >  static struct value *
> >  value_arg_coerce (struct value *arg, struct type *param_type,
> > -		  int is_prototyped)
> > +		  int is_prototyped, CORE_ADDR *pc)
> 
> ... and call it sp :-)

Eyes rolling backwards... Thanks for spotting this! Time to go to bed...

Thanks again,
-- 
Joel