[PATCH] Fix for call feature having nine parameters or more in AIX

[PATCH] Fix for call feature having nine parameters or more in AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 7308 bytes --]

Respected Ulrich and GDB community members,

Hi,

Please find attached the patch. See:- 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch

This is a patch which is a fix to a problem that has occurred due to non-zero extension of a word in 64 bit mode.

Consider the code named as Example 1 pasted below this email.

The GDB output for the same without applying this patch with the code compiled in 64 bit mode is as follows:-
./gdb ~/gdb_tests/nine_parameter_func_64
GNU gdb (GDB) 14.0.50.20230811-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
    http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/nine_parameter_func_64...
(gdb) b main
Breakpoint 1 at 0x10000934: file /home/aditya/gdb_tests/nine_parameter_func.c, line 27.
(gdb) r
Starting program: /home/aditya/gdb_tests/nine_parameter_func_64

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = -268435248 , 10th para = -268374976
j = -268435248
$1 = -1059136197
(gdb)

From the output kindly check what is going on with 9th parameter and beyond. It is some garbage value.

The reason is that in the 64-bit mode, ideally, I thought from the compiler we should have got the zero extended and 8 word aligned number but we do not. We ourselves must do it. This is like the issue we solved last year.{ https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=7aae1a86b30185224554d450e233ca967359b75d}

So, debugging further I realized that the parameters of function in AIX are stored in registers 3 to 10. More about this fact can be read in this document {https://www.ibm.com/docs/en/aix/7.2?topic=overview-register-usage-conventions}. If the function has more than 8 parameters then the 9th one onwards,  we store the function parameters in the stack. This can be seen in the rs6000-aix-tdep.c file in the dummy_call function from line 700 and beyond. Over here we have this line below.

write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);

This the root cause of this issue. Since the arg->contents ().data () is not zero extended we are going to get something corrupt. This works in 32 bit programs whose output I have pasted below Example 1 as 32 bit output without patch.

In 64-bit mode we will need to add 6-word space which will be 6 * 8 = 48, plus ii * wordsize = ii * 8 to the SP as well.. So, this I have done in the below lines along with zero extending the output,

+store_unsigned_integer (word, tdep->wordsize, byte_order,
+                                                                    unpack_long (type, arg->contents ().data ()));
+                    write_memory (sp + 48 + (ii * 8), word, PPC_MAX_REGISTER_SIZE);

Where ii is the parameter number i.e. 9th or 10 th or 11 th and so on.

After this in AIX, the output of 64 bit code of Example 1 in GDB is as follows in 64 bit output with patch pasted below this email.

So, the issue is fixed in this case.

Since you all are experts, and I am a beginner and still learning a lot of things in GDB there are chances I might have not thought of more scenarios to this problem.

Kindly let me know if my analysis and patch is not correct. Kindly give me feedback for the same. If it the patch looks all right kindly push the same.

Have a nice day ahead.

Thanks and regards,
Aditya.


64 bit output with patch:-
./gdb ~/gdb_tests/nine_parameter_func_64
GNU gdb (GDB) 14.0.50.20230811-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
   http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/nine_parameter_func_64...
(gdb) b main
Breakpoint 1 at 0x10000934: file /home/aditya/gdb_tests/nine_parameter_func.c, line 27.
(gdb) r
Starting program: /home/aditya/gdb_tests/nine_parameter_func_64

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
Invalid character ';' in expression.
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 20)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1042

Example 1:-

cat ~/gdb_tests/nine_parameter_func.c
#include <stdio.h>

int b ()
{
  const float register f1 = 1.0;
  const float register f2 = 2.0;
  float register f3 = 2.0;
  int register i1 = 900;
  return printf("%f %f\n", f1 + i1, f2 + f3);
}

int a (int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l)
{
  const float register f3 = 12.0;
  const int register i1 = 800;
  printf("%f \n", f3 + i1);
  static int var = 123;
  b++;
  c++;
  d = e + f + g + h + i + j + k + l;
  printf ("9th para = %d , 10th para = %d\n", j, k);
  printf ("j = %d \n", j);
  return (int)(d);
}
int main ()
{
  const float register f3 = 19.0;
  const int register i1 = 700;
  printf("%f \n", f3 + i1);
  b ();
  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
  return 0;
}

32 bit output without and with patch:-
Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb)  call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb)


[-- Attachment #2: 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch --]
[-- Type: application/octet-stream, Size: 1481 bytes --]

From 9e51f460b43be22cb940c2ebb09b4f04b4b3728d Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 25 Aug 2023 03:20:07 -0500
Subject: [PATCH] Fix for call feature having 9th function parameter and beyond
 corrupt values.

In AIX the first eight function parameters are stored from R3 to R10.
If there are more than eight parameters in a function then we store the 9th parameter onwards in the stack.
While doing so, in 64 bit mode the words were not zero extended and was coming like 32 bit mode.
This patch is a fix to the same.
---
 gdb/rs6000-aix-tdep.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index 829f55981ca..2e38377f679 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -707,7 +707,16 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	      ++f_argno;
 	    }
 
-	  write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
+	  if (tdep->wordsize == 8)
+	    {
+	      gdb_byte word[PPC_MAX_REGISTER_SIZE];
+	      memset (word, 0, PPC_MAX_REGISTER_SIZE);
+	      store_unsigned_integer (word, tdep->wordsize, byte_order,
+				      unpack_long (type, arg->contents ().data ()));
+	      write_memory (sp + 48 + (ii * 8), word, PPC_MAX_REGISTER_SIZE);
+	    }
+	  else
+	    write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
 	  ii += ((len + 3) & -4) / 4;
 	}
     }
-- 
2.38.3

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>So, debugging further I realized that the parameters of function in AIX
>are stored in registers 3 to 10. More about this fact can be read in this
>document {https://www.ibm.com/docs/en/aix/7.2?topic=overview-register-usage-conventions}.
>If the function has more than 8 parameters then the 9th one onwards,  we store
>the function parameters in the stack. This can be seen in the rs6000-aix-tdep.c
>file in the dummy_call function from line 700 and beyond. Over here we have
>this line below.
>
>write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
> 
>This the root cause of this issue.

I agree you've identified a problem, but I think your patch isn't quite complete.

For example, immediately after the code you changed follows:
          ii += ((len + 3) & -4) / 4;
The intent is to always uses full stack slots even for arguments of odd sizes.

But I understand in the 64-bit ABI the stack slot size is 8 bytes, so this
should round up to the next multiple of 8.

Similarly, you need to make sure that the first loop that computes the *size*
of the stack that will be used for arguments performs the same calculations
as the code that actually fills in the arguments - or else you can overwrite
unrelated areas:

      if (argbytes)
        {
          space += ((len - argbytes + 3) & -4);
          jj = argno + 1;
        }
      else
        jj = argno;


      for (; jj < nargs; ++jj)
        {
          struct value *val = args[jj];
          space += ((val->type ()->length ()) + 3) & -4;
        }

All of this should round up to wordsize instead of 4, I guess.

Bye,
Ulrich

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 6510 bytes --]

Hi Ulrich and GDB community members,

Thank you for the feedback. Please find attached the patch. See:- 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch

> I agree you've identified a problem, but I think your patch isn't quite complete.

> For example, immediately after the code you changed follows:
>          ii += ((len + 3) & -4) / 4;
> The intent is to always uses full stack slots even for arguments of odd sizes.

> But I understand in the 64-bit ABI the stack slot size is 8 bytes, so this
> should round up to the next multiple of 8.

> Similarly, you need to make sure that the first loop that computes the *size*
> of the stack that will be used for arguments performs the same calculations
> as the code that actually fills in the arguments - or else you can overwrite
> unrelated areas:
>          space += ((len - argbytes + 3) & -4);
>          space += ((val->type ()->length ()) + 3) & -4;
> All of this should round up to wordsize instead of 4, I guess.
> This will now round up to word size after the changes in this patch

I get why you are saying this. So the slack slots need to be of 8 bytes. In this case they are adding 3 cz assume a character guy comes then it becomes 1+3 = 4, which then & -4 will give you 4. Even if a bigger guy than character datatype comes it will be 4 at least. So it will be rounded to 4 all the time.

Similarly when for our 8 byte friend we will need the number 7 instead of 3.

So 4 byte = 3, 8 byte = 7, this is a pattern of 2 power log base 2 (wordsize) -1. Example if the wordsize is 4 then we get 2*pow ( log 2 (4)) – 1 = 2 pow (2) – 1 = 4 – 1 =3.. Same substitute we can do for 8 word size and we will get seven. So in my patch you will see this change wherever possible.

-                 space += ((len - argbytes + 3) & -4);
+                space += ((len - argbytes + (int) pow (2, (int)log2 (wordsize)) -1) & -wordsize);

So now space becomes 4, 8, 12 in 32 bit mode and 8, 16, 24 in 64 bit mode for 9th, 10th and 11th parameter of a function. Same goes with ii as well. It will come 8, 9, 10 irrespective of any word size.

Hope this works for all of us..

Please see the 32 bit and 64 bit output pasted. They work fine.

Thank you once again for guiding me. Let me know if it works. If this okay please push this patch. If not let me know what else I can learn or need to change.

Also, I am curious to know why didn’t I get a SEG fault or garbage the last time and why was it working without these space changes. I was trying to understand but somehow am not fully clear about this. Let me know if you did..

Have a nice day ahead.

Thanks and regards,
Aditya.

32 bit output with patch:-

Reading symbols from /home/aditya/gdb_tests/nine_parameter_func...
(gdb) b main
Breakpoint 1 at 0x1000078c: file /home/aditya/gdb_tests/nine_parameter_func.c, line 27.
(gdb) r
Starting program: /home/aditya/gdb_tests/nine_parameter_func

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 25)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1047
(gdb)

64 bit output with patch:-

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) lsit
Undefined command: "lsit".  Try "help".
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 25)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1047
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 30)
812.000000
9th para = 9 , 10th para = 983
j = 9
$3 = 1052
(gdb)

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Friday, 25 August 2023 at 4:49 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Fix for call feature having nine parameters or more in AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>So, debugging further I realized that the parameters of function in AIX
>are stored in registers 3 to 10. More about this fact can be read in this
>document {https://www.ibm.com/docs/en/aix/7.2?topic=overview-register-usage-conventions}.
>If the function has more than 8 parameters then the 9th one onwards,  we store
>the function parameters in the stack. This can be seen in the rs6000-aix-tdep.c
>file in the dummy_call function from line 700 and beyond. Over here we have
>this line below.
>
>write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
>
>This the root cause of this issue.

I agree you've identified a problem, but I think your patch isn't quite complete.

For example, immediately after the code you changed follows:
          ii += ((len + 3) & -4) / 4;
The intent is to always uses full stack slots even for arguments of odd sizes.

But I understand in the 64-bit ABI the stack slot size is 8 bytes, so this
should round up to the next multiple of 8.

Similarly, you need to make sure that the first loop that computes the *size*
of the stack that will be used for arguments performs the same calculations
as the code that actually fills in the arguments - or else you can overwrite
unrelated areas:

      if (argbytes)
        {
          space += ((len - argbytes + 3) & -4);
          jj = argno + 1;
        }
      else
        jj = argno;


      for (; jj < nargs; ++jj)
        {
          struct value *val = args[jj];
          space += ((val->type ()->length ()) + 3) & -4;
        }

All of this should round up to wordsize instead of 4, I guess.

Bye,
Ulrich

[-- Attachment #2: 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch --]
[-- Type: application/octet-stream, Size: 3427 bytes --]

From 77187ed24062c95bab11c044240cd0c78b8736d1 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 25 Aug 2023 08:22:00 -0500
Subject: [PATCH] Fix for call feature having 9th function parameter and beyond
   corrupt values.

In AIX the first eight function parameters are stored from R3 to R10.
If there are more than eight parameters in a function then we store the 9th parameter onwards in the stack.
While doing so, in 64 bit mode the words were not zero extended and was coming like 32 bit mode.
This patch is a fix to the same.
---
 gdb/rs6000-aix-tdep.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index 829f55981ca..ebcb9850dc7 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -39,6 +39,7 @@
 #include "gdbsupport/xml-utils.h"
 #include "trad-frame.h"
 #include "frame-unwind.h"
+#include <math.h>
 
 /* If the kernel has to deliver a signal, it pushes a sigcontext
    structure on the stack and then calls the signal handler, passing
@@ -649,7 +650,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
       if (argbytes)
 	{
-	  space += ((len - argbytes + 3) & -4);
+	  space += ((len - argbytes + (int) pow (2, (int)log2 (wordsize)) -1) & -wordsize);
 	  jj = argno + 1;
 	}
       else
@@ -658,7 +659,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       for (; jj < nargs; ++jj)
 	{
 	  struct value *val = args[jj];
-	  space += ((val->type ()->length ()) + 3) & -4;
+	  space += (((val->type ()->length ()) + (int) pow (2, (int)log2 (wordsize)) -1) & -wordsize);
 	}
 
       /* Add location required for the rest of the parameters.  */
@@ -679,11 +680,20 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
       if (argbytes)
 	{
-	  write_memory (sp + 24 + (ii * 4),
-			arg->contents ().data () + argbytes,
-			len - argbytes);
+	  if (wordsize == 8)
+	    {
+	      gdb_byte word[PPC_MAX_REGISTER_SIZE];
+	      memset (word, 0, PPC_MAX_REGISTER_SIZE);
+	      store_unsigned_integer (word, tdep->wordsize, byte_order,
+				      unpack_long (type, arg->contents ().data ()));
+	      write_memory (sp + 48 + (ii * wordsize), word, PPC_MAX_REGISTER_SIZE);
+	    }
+	  else
+	    write_memory (sp + 24 + (ii * wordsize),
+			  arg->contents ().data () + argbytes,
+			  len - argbytes);
 	  ++argno;
-	  ii += ((len - argbytes + 3) & -4) / 4;
+	  ii += ((len - argbytes + (int) pow (2, (int)log2 (wordsize)) - 1) & -wordsize) / wordsize;
 	}
 
       /* Push the rest of the arguments into stack.  */
@@ -707,8 +717,17 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	      ++f_argno;
 	    }
 
-	  write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
-	  ii += ((len + 3) & -4) / 4;
+	  if (wordsize == 8)
+	    {
+	      gdb_byte word[PPC_MAX_REGISTER_SIZE];
+	      memset (word, 0, PPC_MAX_REGISTER_SIZE);
+	      store_unsigned_integer (word, tdep->wordsize, byte_order,
+				      unpack_long (type, arg->contents ().data ()));
+	      write_memory (sp + 48 + (ii * wordsize), word, PPC_MAX_REGISTER_SIZE);
+	    }
+	  else
+	    write_memory (sp + 24 + (ii * wordsize), arg->contents ().data (), len);
+	  ii += ((len + (int) pow (2, (int)log2 (wordsize)) -1) & -wordsize) / wordsize;
 	}
     }
 
-- 
2.38.3

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>-                 space += ((len - argbytes + 3) & -4);
>+                space += ((len - argbytes + (int) pow (2, (int)log2 (wordsize)) -1) & -wordsize);

This seems unnecessarily complex, as wordsize is always a power of 2.
It could simply be: 
+                 space += ((len - argbytes + wordsize - 1) & -wordsize);

>If not let me know what else I can learn or need to change.

I think this is still not quite correct.  Depending on the type,
some parameters that are not integers (e.g. structs) should not
be extended, as that doesn't make sense for this type.  When
loading into registers, you see the existing code:

          memset (word, 0, reg_size);
          if (type->code () == TYPE_CODE_INT
             || type->code () == TYPE_CODE_ENUM
             || type->code () == TYPE_CODE_BOOL
             || type->code () == TYPE_CODE_CHAR)
            /* Sign or zero extend the "int" into a "word".  */
            store_unsigned_integer (word, reg_size, byte_order,
                                    unpack_long (type, arg->contents ().data ()));
          else
            memcpy (word, arg->contents ().data (), len);

It would probably be best to duplicate that logic for the
stack argument case.

Also, the "argbytes" case seems wrong - this is the case where
an argument fits "half" in registers and "half" on the stack.
In those cases, it does not make any sense to "extend" just
the second half on its own ...

Bye,
Ulrich

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 5308 bytes --]

Hi Ulrich and GDB community members,

Thank you for the feedback. Please find attached the patch. See:- 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch

>This seems unnecessarily complex, as wordsize is always a power of 2.
>It could simply be:
>+                 space += ((len - argbytes + wordsize - 1) & -wordsize);

>>If not let me know what else I can learn or need to change.

This is done.

>I think this is still not quite correct.  Depending on the type,
>some parameters that are not integers (e.g. structs) should not
>be extended, as that doesn't make sense for this type.  When
>loading into registers, you see the existing code:

>          memset (word, 0, reg_size);
>          if (type->code () == TYPE_CODE_INT
>             || type->code () == TYPE_CODE_ENUM
>             || type->code () == TYPE_CODE_BOOL
>             || type->code () == TYPE_CODE_CHAR)
>            /* Sign or zero extend the "int" into a "word".  */

>It would probably be best to duplicate that logic for the
>stack argument case.

This is done.

>Also, the "argbytes" case seems wrong - this is the case where
>an argument fits "half" in registers and "half" on the stack.
>In those cases, it does not make any sense to "extend" just
>the second half on its own ...

Yes I realised this. Thanks. I removed the zero extension. Just kept those changes that will keep it synced with the rest of the code and needed as per the word size.

Hope this looks good now. Kindly let me know. If not kindly push these changes.

Outputs are pasted below..

Have a nice day ahead.

Thanks and regards,
Aditya.

32 bit output with patch:-

Reading symbols from /home/aditya/gdb_tests/nine_parameter_func...
(gdb) b main
Breakpoint 1 at 0x1000078c: file /home/aditya/gdb_tests/nine_parameter_func.c, line 27.
(gdb) r
Starting program: /home/aditya/gdb_tests/nine_parameter_func

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 25)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1047
(gdb)

64 bit output with patch:-

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) lsit
Undefined command: "lsit".  Try "help".
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 25)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1047
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 30)
812.000000
9th para = 9 , 10th para = 983
j = 9
$3 = 1052
(gdb)



From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Friday, 25 August 2023 at 7:43 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Fix for call feature having nine parameters or more in AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>-                 space += ((len - argbytes + 3) & -4);
>+                space += ((len - argbytes + (int) pow (2, (int)log2 (wordsize)) -1) & -wordsize);

This seems unnecessarily complex, as wordsize is always a power of 2.
It could simply be:
+                 space += ((len - argbytes + wordsize - 1) & -wordsize);

>If not let me know what else I can learn or need to change.

I think this is still not quite correct.  Depending on the type,
some parameters that are not integers (e.g. structs) should not
be extended, as that doesn't make sense for this type.  When
loading into registers, you see the existing code:

          memset (word, 0, reg_size);
          if (type->code () == TYPE_CODE_INT
             || type->code () == TYPE_CODE_ENUM
             || type->code () == TYPE_CODE_BOOL
             || type->code () == TYPE_CODE_CHAR)
            /* Sign or zero extend the "int" into a "word".  */
            store_unsigned_integer (word, reg_size, byte_order,
                                    unpack_long (type, arg->contents ().data ()));
          else
            memcpy (word, arg->contents ().data (), len);

It would probably be best to duplicate that logic for the
stack argument case.

Also, the "argbytes" case seems wrong - this is the case where
an argument fits "half" in registers and "half" on the stack.
In those cases, it does not make any sense to "extend" just
the second half on its own ...

Bye,
Ulrich

[-- Attachment #2: 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch --]
[-- Type: application/octet-stream, Size: 2826 bytes --]

From f7d50b75cdb783e4942b333adfb836c68cecf2bb Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 25 Aug 2023 10:24:58 -0500
Subject: [PATCH] Fix for call feature having 9th function parameter and beyond
    corrupt values.

In AIX the first eight function parameters are stored from R3 to R10.
If there are more than eight parameters in a function then we store the 9th parameter onwards in the stack.
While doing so, in 64 bit mode the words were not zero extended and was coming like 32 bit mode.
This patch is a fix to the same.
---
 gdb/rs6000-aix-tdep.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index 829f55981ca..a3e9e94841c 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -649,7 +649,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
       if (argbytes)
 	{
-	  space += ((len - argbytes + 3) & -4);
+	  space += ((len - argbytes + wordsize -1) & -wordsize);
 	  jj = argno + 1;
 	}
       else
@@ -658,7 +658,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       for (; jj < nargs; ++jj)
 	{
 	  struct value *val = args[jj];
-	  space += ((val->type ()->length ()) + 3) & -4;
+	  space += ((val->type ()->length () + wordsize -1) & -wordsize);
 	}
 
       /* Add location required for the rest of the parameters.  */
@@ -679,11 +679,11 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
       if (argbytes)
 	{
-	  write_memory (sp + 24 + (ii * 4),
+	  write_memory (sp + 24 + (ii * wordsize),
 			arg->contents ().data () + argbytes,
 			len - argbytes);
 	  ++argno;
-	  ii += ((len - argbytes + 3) & -4) / 4;
+	  ii += ((len - argbytes + wordsize - 1) & -wordsize) / wordsize;
 	}
 
       /* Push the rest of the arguments into stack.  */
@@ -707,8 +707,21 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	      ++f_argno;
 	    }
 
-	  write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
-	  ii += ((len + 3) & -4) / 4;
+	  if (wordsize == 8 && 
+	     (type->code () == TYPE_CODE_INT
+	      || type->code () == TYPE_CODE_ENUM
+	      || type->code () == TYPE_CODE_BOOL
+	      || type->code () == TYPE_CODE_CHAR ))
+	    {
+	      gdb_byte word[PPC_MAX_REGISTER_SIZE];
+	      memset (word, 0, PPC_MAX_REGISTER_SIZE);
+	      store_unsigned_integer (word, tdep->wordsize, byte_order,
+				      unpack_long (type, arg->contents ().data ()));
+	      write_memory (sp + 6 * wordsize + (ii * wordsize), word, PPC_MAX_REGISTER_SIZE);
+	    }
+	  else
+	    write_memory (sp + 6 * wordsize + (ii * wordsize), arg->contents ().data (), len);
+	  ii += ((len + wordsize -1) & -wordsize) / wordsize;
 	}
     }
 
-- 
2.38.3

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>Hope this looks good now. Kindly let me know. If not kindly push these changes.

Just a few comments remaining:

>+         space += ((len - argbytes + wordsize -1) & -wordsize);
Please use "- 1" instead of "-1" everywhere here.

>+         write_memory (sp + 24 + (ii * wordsize),
I think this needs to be 6 * wordsize instead of 24 here too.

>+         if (wordsize == 8 &&  
>+            (type->code () == TYPE_CODE_INT
>+             || type->code () == TYPE_CODE_ENUM
>+             || type->code () == TYPE_CODE_BOOL
>+             || type->code () == TYPE_CODE_CHAR ))
>+           {
I'm not sure the "wordsize == 8" check is correct here.  It's of course
a question of how the 32-bit AIX ABI is defined, but for parameters in
registers, we do the extension on 32-bit too.  So it seems likely to me
that we need to do it for parameters in memory as well.

Bye,
Ulrich

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 3969 bytes --]

Hi Ulrich and GDB community members,

Thank you for the feedback. Please find attached the patch. See:- 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch

>>+         write_memory (sp + 24 + (ii * wordsize),
>I think this needs to be 6 * wordsize instead of 24 here too.

This is done. Thanks.

>>+         if (wordsize == 8 &&
>+            (type->code () == TYPE_CODE_INT

>I'm not sure the "wordsize == 8" check is correct here.  It's of course
>a question of how the 32-bit AIX ABI is defined, but for parameters in
>registers, we do the extension on 32-bit too.  So it seems likely to me
>that we need to do it for parameters in memory as well.

This also done.

Hope all is good now. Kindly push the patch if there are not more changes. Let me know if any more.

Have a nice day ahead.

By the way, pasted the outputs below.

Thanks and regards,
Aditya.

32 bit output with patch:-

Reading symbols from /home/aditya/gdb_tests/nine_parameter_func...
(gdb) b main
Breakpoint 1 at 0x1000078c: file /home/aditya/gdb_tests/nine_parameter_func.c, line 27.
(gdb) r
Starting program: /home/aditya/gdb_tests/nine_parameter_func

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call  a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 25)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1047
(gdb)

64 bit output with patch:-

Breakpoint 1, main () at /home/aditya/gdb_tests/nine_parameter_func.c:27
27        const float register f3 = 19.0;
(gdb) lsit
Undefined command: "lsit".  Try "help".
(gdb) list
22        printf ("j = %d \n", j);
23        return (int)(d);
24      }
25      int main ()
26      {
27        const float register f3 = 19.0;
28        const int register i1 = 700;
29        printf("%f \n", f3 + i1);
30        b ();
31        a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19);
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 19)
812.000000
9th para = 9 , 10th para = 983
j = 9
$1 = 1041
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 25)
812.000000
9th para = 9 , 10th para = 983
j = 9
$2 = 1047
(gdb) call a (1, 2, 3, 4, 5, 6, 7, 8, 9, 983, 30)
812.000000
9th para = 9 , 10th para = 983
j = 9
$3 = 1052
(gdb)

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Friday, 25 August 2023 at 9:27 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Fix for call feature having nine parameters or more in AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>Hope this looks good now. Kindly let me know. If not kindly push these changes.

Just a few comments remaining:

>+         space += ((len - argbytes + wordsize -1) & -wordsize);
Please use "- 1" instead of "-1" everywhere here.

>+         write_memory (sp + 24 + (ii * wordsize),
I think this needs to be 6 * wordsize instead of 24 here too.

>+         if (wordsize == 8 &&
>+            (type->code () == TYPE_CODE_INT
>+             || type->code () == TYPE_CODE_ENUM
>+             || type->code () == TYPE_CODE_BOOL
>+             || type->code () == TYPE_CODE_CHAR ))
>+           {
I'm not sure the "wordsize == 8" check is correct here.  It's of course
a question of how the 32-bit AIX ABI is defined, but for parameters in
registers, we do the extension on 32-bit too.  So it seems likely to me
that we need to do it for parameters in memory as well.

Bye,
Ulrich

[-- Attachment #2: 0001-Fix-for-call-feature-having-9th-function-parameter-a.patch --]
[-- Type: application/octet-stream, Size: 2806 bytes --]

From e590c888d0d4cb9309abf14c12f5617a6a5de2b2 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Fri, 25 Aug 2023 11:30:02 -0500
Subject: [PATCH] Fix for call feature having 9th function parameter and beyond
     corrupt values.

In AIX the first eight function parameters are stored from R3 to R10.
If there are more than eight parameters in a function then we store the 9th parameter onwards in the stack.
While doing so, in 64 bit mode the words were not zero extended and was coming like 32 bit mode.
This patch is a fix to the same.
---
 gdb/rs6000-aix-tdep.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index 829f55981ca..8c3a23788b3 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -649,7 +649,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
       if (argbytes)
 	{
-	  space += ((len - argbytes + 3) & -4);
+	  space += ((len - argbytes + wordsize -1) & -wordsize);
 	  jj = argno + 1;
 	}
       else
@@ -658,7 +658,7 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       for (; jj < nargs; ++jj)
 	{
 	  struct value *val = args[jj];
-	  space += ((val->type ()->length ()) + 3) & -4;
+	  space += ((val->type ()->length () + wordsize -1) & -wordsize);
 	}
 
       /* Add location required for the rest of the parameters.  */
@@ -679,11 +679,11 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
       if (argbytes)
 	{
-	  write_memory (sp + 24 + (ii * 4),
+	  write_memory (sp + 6 * wordsize + (ii * wordsize),
 			arg->contents ().data () + argbytes,
 			len - argbytes);
 	  ++argno;
-	  ii += ((len - argbytes + 3) & -4) / 4;
+	  ii += ((len - argbytes + wordsize - 1) & -wordsize) / wordsize;
 	}
 
       /* Push the rest of the arguments into stack.  */
@@ -707,8 +707,20 @@ rs6000_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	      ++f_argno;
 	    }
 
-	  write_memory (sp + 24 + (ii * 4), arg->contents ().data (), len);
-	  ii += ((len + 3) & -4) / 4;
+	  if (type->code () == TYPE_CODE_INT
+	     || type->code () == TYPE_CODE_ENUM
+	     || type->code () == TYPE_CODE_BOOL
+	     || type->code () == TYPE_CODE_CHAR )
+	    {
+	      gdb_byte word[PPC_MAX_REGISTER_SIZE];
+	      memset (word, 0, PPC_MAX_REGISTER_SIZE);
+	      store_unsigned_integer (word, tdep->wordsize, byte_order,
+				      unpack_long (type, arg->contents ().data ()));
+	      write_memory (sp + 6 * wordsize + (ii * wordsize), word, PPC_MAX_REGISTER_SIZE);
+	    }
+	  else
+	    write_memory (sp + 6 * wordsize + (ii * wordsize), arg->contents ().data (), len);
+	  ii += ((len + wordsize -1) & -wordsize) / wordsize;
 	}
     }
 
-- 
2.38.3

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>Hope all is good now. Kindly push the patch if there are not more changes. Let me know if any more.

This is OK.  I've checked the patch in now.

Thanks,
Ulrich

Re: [PATCH] Fix for call feature having nine parameters or more in AIX

[Aditya Kamath1]

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

Thank you :)

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Friday, 25 August 2023 at 10:19 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Fix for call feature having nine parameters or more in AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>Hope all is good now. Kindly push the patch if there are not more changes. Let me know if any more.

This is OK.  I've checked the patch in now.

Thanks,
Ulrich