public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
@ 2017-03-29 16:21 vladimir.mezentsev
  2017-04-18 14:54 ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: vladimir.mezentsev @ 2017-03-29 16:21 UTC (permalink / raw)
  To: gdb-patches

From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

	gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
	(https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
	TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.

	Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
	No regressions.

	2017-03-27 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

	* gdb/sparc-tdep.c (sparc_structure_return_p): New function.
	* gdb/sparc-tdep.c (sparc_arg_on_registers_p): Likewise.
	* gdb/sparc-tdep.c (sparc32_store_arguments): Use the new functions.
	* gdb/sparc64-tdep.c: (sparc64_16_byte_align_p): Handle TYPE_CODE_ARRAY
	* gdb/sparc64-tdep.c: (sparc64_store_floating_fields): Likewise
	* gdb/sparc64-tdep.c: (sparc64_extract_floating_fields): Likewise
---
 gdb/sparc-tdep.c   |   68 +++++++++++++++++++++++++++++++++++++++-------------
 gdb/sparc64-tdep.c |   42 ++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 19 deletions(-)

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index d346aec..07808f3 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -297,6 +297,39 @@ sparc_structure_or_union_p (const struct type *type)
   return 0;
 }
 
+static int
+sparc_structure_return_p (const struct type *type)
+{
+  if ((TYPE_CODE (type) == TYPE_CODE_ARRAY) && (TYPE_LENGTH (type) <= 8))
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc_floating_p (t) && (TYPE_LENGTH (t) == 8))
+        return 1;
+      return 0;
+    }
+  if (sparc_floating_p (type) && (TYPE_LENGTH (type) == 16))
+    return 1;
+  return sparc_structure_or_union_p (type);
+}
+
+static int
+sparc_arg_on_registers_p (const struct type *type)
+{
+  if ((TYPE_CODE (type) == TYPE_CODE_ARRAY) && (TYPE_LENGTH (type) <= 8))
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc_floating_p (t) && (TYPE_LENGTH (t) == 8))
+        return 0;
+      return 1;
+    }
+  if (sparc_structure_or_union_p (type) || sparc_complex_floating_p (type)
+      || (sparc_floating_p (type) && (TYPE_LENGTH (type) == 16)))
+    return 0;
+  return 1;
+}
+
 /* Register information.  */
 #define SPARC32_FPU_REGISTERS                             \
   "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",         \
@@ -569,9 +602,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       struct type *type = value_type (args[i]);
       int len = TYPE_LENGTH (type);
 
-      if (sparc_structure_or_union_p (type)
-	  || (sparc_floating_p (type) && len == 16)
-	  || sparc_complex_floating_p (type))
+      if (!sparc_arg_on_registers_p (type))
 	{
 	  /* Structure, Union and Quad-Precision Arguments.  */
 	  sp -= len;
@@ -593,11 +624,8 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       else
 	{
 	  /* Integral and pointer arguments.  */
-	  gdb_assert (sparc_integral_or_pointer_p (type));
-
-	  if (len < 4)
-	    args[i] = value_cast (builtin_type (gdbarch)->builtin_int32,
-				  args[i]);
+	  gdb_assert (sparc_integral_or_pointer_p (type) ||
+                      ((TYPE_CODE (type) == TYPE_CODE_ARRAY) && (len <= 8)));
 	  num_elements += ((len + 3) / 4);
 	}
     }
@@ -619,6 +647,15 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       const bfd_byte *valbuf = value_contents (args[i]);
       struct type *type = value_type (args[i]);
       int len = TYPE_LENGTH (type);
+      gdb_byte buf[4];
+
+      if (len < 4)
+        {
+          memset (buf, 0, 4 - len);
+          memcpy (buf + 4 - len, valbuf, len);
+          valbuf = buf;
+          len = 4;
+        }
 
       gdb_assert (len == 4 || len == 8);
 
@@ -1344,10 +1381,10 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
   int len = TYPE_LENGTH (type);
   gdb_byte buf[32];
 
-  gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
+  gdb_assert (!sparc_structure_return_p (type));
 
-  if (sparc_floating_p (type) || sparc_complex_floating_p (type))
+  if (sparc_floating_p (type) || sparc_complex_floating_p (type) ||
+      (TYPE_CODE (type) == TYPE_CODE_ARRAY))
     {
       /* Floating return values.  */
       regcache_cooked_read (regcache, SPARC_F0_REGNUM, buf);
@@ -1396,11 +1433,9 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
 			    const gdb_byte *valbuf)
 {
   int len = TYPE_LENGTH (type);
-  gdb_byte buf[8];
+  gdb_byte buf[32];
 
-  gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
-  gdb_assert (len <= 8);
+  gdb_assert (!sparc_structure_return_p (type));
 
   if (sparc_floating_p (type) || sparc_complex_floating_p (type))
     {
@@ -1456,8 +1491,7 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
      guarantees that we can always find the return value, not just
      before the function returns.  */
 
-  if (sparc_structure_or_union_p (type)
-      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+  if (sparc_structure_return_p (type))
     {
       ULONGEST sp;
       CORE_ADDR addr;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index bf0da18..0b1a0ee 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -669,6 +669,12 @@ static const struct frame_base sparc64_frame_base =
 static int
 sparc64_16_byte_align_p (struct type *type)
 {
+  if ((TYPE_CODE (type) == TYPE_CODE_ARRAY)) {
+    struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+    if (sparc64_floating_p (t))
+      return 1;
+  }
   if (sparc64_floating_p (type) && TYPE_LENGTH (type) == 16)
     return 1;
 
@@ -703,7 +709,23 @@ sparc64_store_floating_fields (struct regcache *regcache, struct type *type,
 
   gdb_assert (element < 16);
 
-  if (sparc64_floating_p (type)
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      gdb_byte buf[8];
+      int regnum = SPARC_F0_REGNUM + element * 2 + bitpos / 32;
+
+      valbuf += (bitpos / 8);
+      if (len < 8)
+        {
+          memset (buf, 0, 8 - len);
+          memcpy (buf + 8 - len, valbuf, len);
+          valbuf = buf;
+          len = 8;
+        }
+      for (int n = 0; n < ((len + 3) / 4); n++)
+        regcache_cooked_write (regcache, regnum + n, valbuf + n * 4);
+    }
+  else if (sparc64_floating_p (type)
       || (sparc64_complex_floating_p (type) && len <= 16))
     {
       int regnum;
@@ -776,7 +798,23 @@ sparc64_extract_floating_fields (struct regcache *regcache, struct type *type,
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
 
-  if (sparc64_floating_p (type))
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      int len = TYPE_LENGTH (type);
+      int regnum =  SPARC_F0_REGNUM + (bitpos / 32);
+
+      valbuf += (bitpos / 8);
+      if (len < 4)
+        {
+          gdb_byte buf[4];
+          regcache_cooked_read (regcache, regnum, buf);
+          memcpy (valbuf, buf + 4 - len, len);
+        }
+      else
+        for (int i = 0; i < ((len + 3) / 4); i++)
+          regcache_cooked_read (regcache, regnum + i, valbuf + i * 4);
+    }
+  else if (sparc64_floating_p (type))
     {
       int len = TYPE_LENGTH (type);
       int regnum;
-- 
1.7.1

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-03-29 16:21 [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc vladimir.mezentsev
@ 2017-04-18 14:54 ` Pedro Alves
  2017-04-18 21:28   ` Jose E. Marchesi
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2017-04-18 14:54 UTC (permalink / raw)
  To: vladimir.mezentsev, gdb-patches

Hi Vladimir,

Sorry for the delay.  I think everyone was hoping that someone
that actually groks SPARC stuff would take a look at this.
I don't think we have any maintainer currently actively caring for
SPARC in particular.  Maybe you guys (Oracle folks) could help
review each others' patches?  I think that'd help get these
patches in quicker.

I'll comment below on formatting / convention / procedural
stuff, and assume the patch's actual substance is correct.

On 03/29/2017 05:17 PM, vladimir.mezentsev@oracle.com wrote:
> From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
> 
> 	gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
> 	(https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
> 	TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.
> 
> 	Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
> 	No regressions.

Note there should be no leading-tab indentation in the body of the
commit log.  Please make sure this is reindented on the left.

> 
> 	2017-03-27 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

Two spaces before and after your name.

> 
> 	* gdb/sparc-tdep.c (sparc_structure_return_p): New function.
> 	* gdb/sparc-tdep.c (sparc_arg_on_registers_p): Likewise.
> 	* gdb/sparc-tdep.c (sparc32_store_arguments): Use the new functions.
> 	* gdb/sparc64-tdep.c: (sparc64_16_byte_align_p): Handle TYPE_CODE_ARRAY
> 	* gdb/sparc64-tdep.c: (sparc64_store_floating_fields): Likewise
> 	* gdb/sparc64-tdep.c: (sparc64_extract_floating_fields): Likewise

Entries are relative to the closest ChangeLog file, which is gdb/ChangeLog
in this case.  File names should be not repeated, and there should be a period
after "Likewise" (though you merge those entries).  There should be no ":" between
the file name and function name.  Here's how I'd fix it:

gdb/ChangeLog:
2017-03-27  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>

	* sparc-tdep.c (sparc_structure_return_p)
	(sparc_arg_on_registers_p): New functions.
	(sparc32_store_arguments): Use them.
	* sparc64-tdep.c (sparc64_16_byte_align_p)
	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
	Handle TYPE_CODE_ARRAY.


You can find these details and more at
<https://sourceware.org/gdb/wiki/ContributionChecklist>.



> ---
>  gdb/sparc-tdep.c   |   68 +++++++++++++++++++++++++++++++++++++++-------------
>  gdb/sparc64-tdep.c |   42 ++++++++++++++++++++++++++++++-
>  2 files changed, 91 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
> index d346aec..07808f3 100644
> --- a/gdb/sparc-tdep.c
> +++ b/gdb/sparc-tdep.c
> @@ -297,6 +297,39 @@ sparc_structure_or_union_p (const struct type *type)
>    return 0;
>  }
>  
> +static int
> +sparc_structure_return_p (const struct type *type)

All functions should have an intro comment.

While at it, please let's use bool/true/false in new code.

> +{
> +  if ((TYPE_CODE (type) == TYPE_CODE_ARRAY) && (TYPE_LENGTH (type) <= 8))

GDB/GNU's coding standard avoids redundant parenthesis:

  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8)

etc. throughout the patch.

> +    {
> +      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
> +
> +      if (sparc_floating_p (t) && (TYPE_LENGTH (t) == 8))


... here ...

> +        return 1;
> +      return 0;
> +    }
> +  if (sparc_floating_p (type) && (TYPE_LENGTH (type) == 16))

... here ...

> +    return 1;
> +  return sparc_structure_or_union_p (type);
> +}
> +
> +static int
> +sparc_arg_on_registers_p (const struct type *type)
> +{
> +  if ((TYPE_CODE (type) == TYPE_CODE_ARRAY) && (TYPE_LENGTH (type) <= 8))

... here ...

> +    {
> +      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
> +
> +      if (sparc_floating_p (t) && (TYPE_LENGTH (t) == 8))

... here ...

> +        return 0;
> +      return 1;
> +    }
> +  if (sparc_structure_or_union_p (type) || sparc_complex_floating_p (type)
> +      || (sparc_floating_p (type) && (TYPE_LENGTH (type) == 16)))

... here ...


> +    return 0;
> +  return 1;
> +}

> +
>  /* Register information.  */
>  #define SPARC32_FPU_REGISTERS                             \
>    "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",         \
> @@ -569,9 +602,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
>        struct type *type = value_type (args[i]);
>        int len = TYPE_LENGTH (type);
>  
> -      if (sparc_structure_or_union_p (type)
> -	  || (sparc_floating_p (type) && len == 16)
> -	  || sparc_complex_floating_p (type))
> +      if (!sparc_arg_on_registers_p (type))
>  	{
>  	  /* Structure, Union and Quad-Precision Arguments.  */
>  	  sp -= len;
> @@ -593,11 +624,8 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
>        else
>  	{
>  	  /* Integral and pointer arguments.  */
> -	  gdb_assert (sparc_integral_or_pointer_p (type));
> -
> -	  if (len < 4)
> -	    args[i] = value_cast (builtin_type (gdbarch)->builtin_int32,
> -				  args[i]);
> +	  gdb_assert (sparc_integral_or_pointer_p (type) ||

|| goes on the next line.

> +                      ((TYPE_CODE (type) == TYPE_CODE_ARRAY) && (len <= 8)));

Redundant parens.

>  	  num_elements += ((len + 3) / 4);
>  	}
>      }
> @@ -619,6 +647,15 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
>        const bfd_byte *valbuf = value_contents (args[i]);
>        struct type *type = value_type (args[i]);
>        int len = TYPE_LENGTH (type);
> +      gdb_byte buf[4];
> +
> +      if (len < 4)
> +        {
> +          memset (buf, 0, 4 - len);
> +          memcpy (buf + 4 - len, valbuf, len);
> +          valbuf = buf;
> +          len = 4;
> +        }
>  
>        gdb_assert (len == 4 || len == 8);
>  
> @@ -1344,10 +1381,10 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
>    int len = TYPE_LENGTH (type);
>    gdb_byte buf[32];
>  
> -  gdb_assert (!sparc_structure_or_union_p (type));
> -  gdb_assert (!(sparc_floating_p (type) && len == 16));
> +  gdb_assert (!sparc_structure_return_p (type));
>  
> -  if (sparc_floating_p (type) || sparc_complex_floating_p (type))
> +  if (sparc_floating_p (type) || sparc_complex_floating_p (type) ||
> +      (TYPE_CODE (type) == TYPE_CODE_ARRAY))

Redundant parens.  "||" goes on next line.  When we have several conditions
that don't fit on one line, I think it reads better to then put each
on its own line:

  if (sparc_floating_p (type) 
      || sparc_complex_floating_p (type) 
      || TYPE_CODE (type) == TYPE_CODE_ARRAY)


>      {
>        /* Floating return values.  */
>        regcache_cooked_read (regcache, SPARC_F0_REGNUM, buf);
> @@ -1396,11 +1433,9 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
>  			    const gdb_byte *valbuf)
>  {
>    int len = TYPE_LENGTH (type);
> -  gdb_byte buf[8];
> +  gdb_byte buf[32];

Is it obvious for someone looking at this function what this
"32" is and whether it is big enough?

>  
> -  gdb_assert (!sparc_structure_or_union_p (type));
> -  gdb_assert (!(sparc_floating_p (type) && len == 16));
> -  gdb_assert (len <= 8);
> +  gdb_assert (!sparc_structure_return_p (type));
>  
>    if (sparc_floating_p (type) || sparc_complex_floating_p (type))
>      {
> @@ -1456,8 +1491,7 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
>       guarantees that we can always find the return value, not just
>       before the function returns.  */
>  
> -  if (sparc_structure_or_union_p (type)
> -      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
> +  if (sparc_structure_return_p (type))
>      {
>        ULONGEST sp;
>        CORE_ADDR addr;
> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index bf0da18..0b1a0ee 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -669,6 +669,12 @@ static const struct frame_base sparc64_frame_base =
>  static int
>  sparc64_16_byte_align_p (struct type *type)
>  {
> +  if ((TYPE_CODE (type) == TYPE_CODE_ARRAY)) {

Redundant parens.

{ does on the next line.


> +    struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
> +
> +    if (sparc64_floating_p (t))
> +      return 1;
> +  }
>    if (sparc64_floating_p (type) && TYPE_LENGTH (type) == 16)
>      return 1;
>  
> @@ -703,7 +709,23 @@ sparc64_store_floating_fields (struct regcache *regcache, struct type *type,
>  
>    gdb_assert (element < 16);
>  
> -  if (sparc64_floating_p (type)
> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
> +    {
> +      gdb_byte buf[8];
> +      int regnum = SPARC_F0_REGNUM + element * 2 + bitpos / 32;
> +
> +      valbuf += (bitpos / 8);

Redundant parens.

> +      if (len < 8)
> +        {
> +          memset (buf, 0, 8 - len);
> +          memcpy (buf + 8 - len, valbuf, len);
> +          valbuf = buf;
> +          len = 8;
> +        }
> +      for (int n = 0; n < ((len + 3) / 4); n++)

Ditto.

> +        regcache_cooked_write (regcache, regnum + n, valbuf + n * 4);
> +    }
> +  else if (sparc64_floating_p (type)
>        || (sparc64_complex_floating_p (type) && len <= 16))

Ditto.

>      {
>        int regnum;
> @@ -776,7 +798,23 @@ sparc64_extract_floating_fields (struct regcache *regcache, struct type *type,
>  {
>    struct gdbarch *gdbarch = get_regcache_arch (regcache);
>  
> -  if (sparc64_floating_p (type))
> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
> +    {
> +      int len = TYPE_LENGTH (type);
> +      int regnum =  SPARC_F0_REGNUM + (bitpos / 32);

Spurious space after " = ".  Redundant parens.

> +
> +      valbuf += (bitpos / 8);
> +      if (len < 4)
> +        {
> +          gdb_byte buf[4];
> +          regcache_cooked_read (regcache, regnum, buf);
> +          memcpy (valbuf, buf + 4 - len, len);
> +        }
> +      else
> +        for (int i = 0; i < ((len + 3) / 4); i++)
> +          regcache_cooked_read (regcache, regnum + i, valbuf + i * 4);


Redundant parens multiple times.

> +    }
> +  else if (sparc64_floating_p (type))
>      {
>        int len = TYPE_LENGTH (type);
>        int regnum;
> 

Please resubmit a v2 patch with these issues addressed.

Thanks,
Pedro Alves

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-04-18 14:54 ` Pedro Alves
@ 2017-04-18 21:28   ` Jose E. Marchesi
  2017-04-18 22:20     ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Jose E. Marchesi @ 2017-04-18 21:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: vladimir.mezentsev, gdb-patches


Hi Pedro.

    Sorry for the delay.  I think everyone was hoping that someone
    that actually groks SPARC stuff would take a look at this.
    I don't think we have any maintainer currently actively caring for
    SPARC in particular.  Maybe you guys (Oracle folks) could help
    review each others' patches?  I think that'd help get these
    patches in quicker.

We (the Oracle toolchain team) actually review the GDB patches
internally from a functional perspective before they are sent upstream.

We can make it more explicit in our future submissions with a note in
the patch submission.

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-04-18 21:28   ` Jose E. Marchesi
@ 2017-04-18 22:20     ` Pedro Alves
  2017-04-19  9:01       ` Jose E. Marchesi
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2017-04-18 22:20 UTC (permalink / raw)
  To: Jose E. Marchesi; +Cc: vladimir.mezentsev, gdb-patches

Hi Jose,

On 04/18/2017 10:28 PM, Jose E. Marchesi wrote:
> 
> Hi Pedro.
> 
>     Sorry for the delay.  I think everyone was hoping that someone
>     that actually groks SPARC stuff would take a look at this.
>     I don't think we have any maintainer currently actively caring for
>     SPARC in particular.  Maybe you guys (Oracle folks) could help
>     review each others' patches?  I think that'd help get these
>     patches in quicker.
> 
> We (the Oracle toolchain team) actually review the GDB patches
> internally from a functional perspective before they are sent upstream.
> 
> We can make it more explicit in our future submissions with a note in
> the patch submission.

Internal review is of course fine and appreciated, but I still think
it better to do at least some of it in the open.  With a bit of visible
involvement, it'll be natural for one (or more) of you guys to
end up being gdb/SPARC maintainer.  (As you know, maintainership
status is individual, not through affiliation.)

Thanks,
Pedro Alves

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-04-18 22:20     ` Pedro Alves
@ 2017-04-19  9:01       ` Jose E. Marchesi
  0 siblings, 0 replies; 11+ messages in thread
From: Jose E. Marchesi @ 2017-04-19  9:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: vladimir.mezentsev, gdb-patches


    > We (the Oracle toolchain team) actually review the GDB patches
    > internally from a functional perspective before they are sent upstream.
    > 
    > We can make it more explicit in our future submissions with a note in
    > the patch submission.
    
    Internal review is of course fine and appreciated, but I still think
    it better to do at least some of it in the open.
    
Yeah, I agree: the more done in the open the better.

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-05-19  9:57   ` Pedro Alves
@ 2017-05-19 10:19     ` Jose E. Marchesi
  0 siblings, 0 replies; 11+ messages in thread
From: Jose E. Marchesi @ 2017-05-19 10:19 UTC (permalink / raw)
  To: Pedro Alves; +Cc: vladimir.mezentsev, gdb-patches


    On 05/19/2017 10:44 AM, Jose E. Marchesi wrote:
    > 
    >     From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
    >     
    >     gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
    >     (https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
    >     TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.
    >     
    >     Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
    >     No regressions.
    >     
    >     gdb/ChangeLog:
    >     2017-05-12  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
    >     
    >     	* sparc-tdep.c (sparc_structure_return_p)
    >     	(sparc_arg_on_registers_p): New functions.
    >     	(sparc32_store_arguments): Use them.
    >     	* sparc64-tdep.c (sparc64_16_byte_align_p)
    >     	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
    >     	Handle TYPE_CODE_ARRAY.
    > 
    > 
    > LGTM.  Ok for master?
    > 
    
    OK with the commit log tweak mentioned at:
    
     https://sourceware.org/ml/gdb-patches/2017-04/msg00509.html


Applied as below.
Thanks.


commit 1933fd8ee01ad2e74a9c6341bc40f54962a8f889
Author: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
Date:   Fri May 19 03:06:19 2017 -0700

    gdb: fix TYPE_CODE_ARRAY handling in sparc targets
    
    gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
    (https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
    TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes
    on Sparc machines.
    
    Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
    
    6 tests ( from gdb/testsuite/gdb.base/gnu_vector.exp) failed on
    sparc64-Linux and on sparc-Solaris in 32- and 64-bit mode.  Now all
    these tests passed.  gdb/testsuite/gdb.base/gnu_vector.exp has 117
    different cases for small (and not small) arrays and structures.
    
    No regressions.
    
    gdb/ChangeLog:
    
    2017-05-19  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
    
    	* sparc-tdep.c (sparc_structure_return_p)
    	(sparc_arg_on_registers_p): New functions.
    	(sparc32_store_arguments): Use them.
    	* sparc64-tdep.c (sparc64_16_byte_align_p)
    	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
    	Handle TYPE_CODE_ARRAY.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fdc2a40..1273ed8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2017-05-19  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
+
+	* sparc-tdep.c (sparc_structure_return_p)
+	(sparc_arg_on_registers_p): New functions.
+	(sparc32_store_arguments): Use them.
+	* sparc64-tdep.c (sparc64_16_byte_align_p)
+	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
+	Handle TYPE_CODE_ARRAY.
+
 2017-05-17  Yao Qi  <yao.qi@linaro.org>
 
 	* cli/cli-decode.c (add_alias_cmd): New function.
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index d1e64b4..fa0e04e 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -298,6 +298,43 @@ sparc_structure_or_union_p (const struct type *type)
   return 0;
 }
 
+/* Check whether TYPE is returned on registers.  */
+
+static bool
+sparc_structure_return_p (const struct type *type)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8)
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc_floating_p (t) && TYPE_LENGTH (t) == 8)
+        return true;
+      return false;
+    }
+  if (sparc_floating_p (type) && TYPE_LENGTH (type) == 16)
+    return true;
+  return sparc_structure_or_union_p (type);
+}
+
+/* Check whether TYPE is passed on registers.  */
+
+static bool
+sparc_arg_on_registers_p (const struct type *type)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8)
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc_floating_p (t) && TYPE_LENGTH (t) == 8)
+        return false;
+      return true;
+    }
+  if (sparc_structure_or_union_p (type) || sparc_complex_floating_p (type)
+      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+    return false;
+  return true;
+}
+
 /* Register information.  */
 #define SPARC32_FPU_REGISTERS                             \
   "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",         \
@@ -570,9 +607,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       struct type *type = value_type (args[i]);
       int len = TYPE_LENGTH (type);
 
-      if (sparc_structure_or_union_p (type)
-	  || (sparc_floating_p (type) && len == 16)
-	  || sparc_complex_floating_p (type))
+      if (!sparc_arg_on_registers_p (type))
 	{
 	  /* Structure, Union and Quad-Precision Arguments.  */
 	  sp -= len;
@@ -594,11 +629,8 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       else
 	{
 	  /* Integral and pointer arguments.  */
-	  gdb_assert (sparc_integral_or_pointer_p (type));
-
-	  if (len < 4)
-	    args[i] = value_cast (builtin_type (gdbarch)->builtin_int32,
-				  args[i]);
+	  gdb_assert (sparc_integral_or_pointer_p (type)
+	              || (TYPE_CODE (type) == TYPE_CODE_ARRAY && len <= 8));
 	  num_elements += ((len + 3) / 4);
 	}
     }
@@ -620,6 +652,15 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       const bfd_byte *valbuf = value_contents (args[i]);
       struct type *type = value_type (args[i]);
       int len = TYPE_LENGTH (type);
+      gdb_byte buf[4];
+
+      if (len < 4)
+        {
+          memset (buf, 0, 4 - len);
+          memcpy (buf + 4 - len, valbuf, len);
+          valbuf = buf;
+          len = 4;
+        }
 
       gdb_assert (len == 4 || len == 8);
 
@@ -1345,10 +1386,10 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
   int len = TYPE_LENGTH (type);
   gdb_byte buf[32];
 
-  gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
+  gdb_assert (!sparc_structure_return_p (type));
 
-  if (sparc_floating_p (type) || sparc_complex_floating_p (type))
+  if (sparc_floating_p (type) || sparc_complex_floating_p (type)
+      || TYPE_CODE (type) == TYPE_CODE_ARRAY)
     {
       /* Floating return values.  */
       regcache_cooked_read (regcache, SPARC_F0_REGNUM, buf);
@@ -1397,11 +1438,9 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
 			    const gdb_byte *valbuf)
 {
   int len = TYPE_LENGTH (type);
-  gdb_byte buf[8];
+  gdb_byte buf[32];
 
-  gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
-  gdb_assert (len <= 8);
+  gdb_assert (!sparc_structure_return_p (type));
 
   if (sparc_floating_p (type) || sparc_complex_floating_p (type))
     {
@@ -1457,8 +1496,7 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
      guarantees that we can always find the return value, not just
      before the function returns.  */
 
-  if (sparc_structure_or_union_p (type)
-      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+  if (sparc_structure_return_p (type))
     {
       ULONGEST sp;
       CORE_ADDR addr;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 58f5bf0..9e0e6b5 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -669,6 +669,13 @@ static const struct frame_base sparc64_frame_base =
 static int
 sparc64_16_byte_align_p (struct type *type)
 {
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc64_floating_p (t))
+        return 1;
+    }
   if (sparc64_floating_p (type) && TYPE_LENGTH (type) == 16)
     return 1;
 
@@ -703,7 +710,23 @@ sparc64_store_floating_fields (struct regcache *regcache, struct type *type,
 
   gdb_assert (element < 16);
 
-  if (sparc64_floating_p (type)
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      gdb_byte buf[8];
+      int regnum = SPARC_F0_REGNUM + element * 2 + bitpos / 32;
+
+      valbuf += bitpos / 8;
+      if (len < 8)
+        {
+          memset (buf, 0, 8 - len);
+          memcpy (buf + 8 - len, valbuf, len);
+          valbuf = buf;
+          len = 8;
+        }
+      for (int n = 0; n < (len + 3) / 4; n++)
+        regcache_cooked_write (regcache, regnum + n, valbuf + n * 4);
+    }
+  else if (sparc64_floating_p (type)
       || (sparc64_complex_floating_p (type) && len <= 16))
     {
       int regnum;
@@ -776,7 +799,23 @@ sparc64_extract_floating_fields (struct regcache *regcache, struct type *type,
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
 
-  if (sparc64_floating_p (type))
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      int len = TYPE_LENGTH (type);
+      int regnum =  SPARC_F0_REGNUM + bitpos / 32;
+
+      valbuf += bitpos / 8;
+      if (len < 4)
+        {
+          gdb_byte buf[4];
+          regcache_cooked_read (regcache, regnum, buf);
+          memcpy (valbuf, buf + 4 - len, len);
+        }
+      else
+        for (int i = 0; i < (len + 3) / 4; i++)
+          regcache_cooked_read (regcache, regnum + i, valbuf + i * 4);
+    }
+  else if (sparc64_floating_p (type))
     {
       int len = TYPE_LENGTH (type);
       int regnum;

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-05-19  9:44 ` Jose E. Marchesi
@ 2017-05-19  9:57   ` Pedro Alves
  2017-05-19 10:19     ` Jose E. Marchesi
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2017-05-19  9:57 UTC (permalink / raw)
  To: Jose E. Marchesi, vladimir.mezentsev; +Cc: gdb-patches

On 05/19/2017 10:44 AM, Jose E. Marchesi wrote:
> 
>     From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
>     
>     gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
>     (https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
>     TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.
>     
>     Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
>     No regressions.
>     
>     gdb/ChangeLog:
>     2017-05-12  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
>     
>     	* sparc-tdep.c (sparc_structure_return_p)
>     	(sparc_arg_on_registers_p): New functions.
>     	(sparc32_store_arguments): Use them.
>     	* sparc64-tdep.c (sparc64_16_byte_align_p)
>     	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
>     	Handle TYPE_CODE_ARRAY.
> 
> 
> LGTM.  Ok for master?
> 

OK with the commit log tweak mentioned at:

 https://sourceware.org/ml/gdb-patches/2017-04/msg00509.html

Thanks,
Pedro Alves

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-05-16 16:16 vladimir.mezentsev
@ 2017-05-19  9:44 ` Jose E. Marchesi
  2017-05-19  9:57   ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Jose E. Marchesi @ 2017-05-19  9:44 UTC (permalink / raw)
  To: vladimir.mezentsev; +Cc: gdb-patches


    From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
    
    gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
    (https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
    TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.
    
    Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
    No regressions.
    
    gdb/ChangeLog:
    2017-05-12  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>
    
    	* sparc-tdep.c (sparc_structure_return_p)
    	(sparc_arg_on_registers_p): New functions.
    	(sparc32_store_arguments): Use them.
    	* sparc64-tdep.c (sparc64_16_byte_align_p)
    	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
    	Handle TYPE_CODE_ARRAY.


LGTM.  Ok for master?

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

* [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
@ 2017-05-16 16:16 vladimir.mezentsev
  2017-05-19  9:44 ` Jose E. Marchesi
  0 siblings, 1 reply; 11+ messages in thread
From: vladimir.mezentsev @ 2017-05-16 16:16 UTC (permalink / raw)
  To: gdb-patches

From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
(https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.

Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
No regressions.

gdb/ChangeLog:
2017-05-12  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>

	* sparc-tdep.c (sparc_structure_return_p)
	(sparc_arg_on_registers_p): New functions.
	(sparc32_store_arguments): Use them.
	* sparc64-tdep.c (sparc64_16_byte_align_p)
	(sparc64_store_floating_fields, sparc64_extract_floating_fields):
	Handle TYPE_CODE_ARRAY.
---
 gdb/sparc-tdep.c   |   72 +++++++++++++++++++++++++++++++++++++++------------
 gdb/sparc64-tdep.c |   43 +++++++++++++++++++++++++++++-
 2 files changed, 96 insertions(+), 19 deletions(-)

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index d1e64b4..fa0e04e 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -298,6 +298,43 @@ sparc_structure_or_union_p (const struct type *type)
   return 0;
 }
 
+/* Check whether TYPE is returned on registers.  */
+
+static bool
+sparc_structure_return_p (const struct type *type)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8)
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc_floating_p (t) && TYPE_LENGTH (t) == 8)
+        return true;
+      return false;
+    }
+  if (sparc_floating_p (type) && TYPE_LENGTH (type) == 16)
+    return true;
+  return sparc_structure_or_union_p (type);
+}
+
+/* Check whether TYPE is passed on registers.  */
+
+static bool
+sparc_arg_on_registers_p (const struct type *type)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8)
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc_floating_p (t) && TYPE_LENGTH (t) == 8)
+        return false;
+      return true;
+    }
+  if (sparc_structure_or_union_p (type) || sparc_complex_floating_p (type)
+      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+    return false;
+  return true;
+}
+
 /* Register information.  */
 #define SPARC32_FPU_REGISTERS                             \
   "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",         \
@@ -570,9 +607,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       struct type *type = value_type (args[i]);
       int len = TYPE_LENGTH (type);
 
-      if (sparc_structure_or_union_p (type)
-	  || (sparc_floating_p (type) && len == 16)
-	  || sparc_complex_floating_p (type))
+      if (!sparc_arg_on_registers_p (type))
 	{
 	  /* Structure, Union and Quad-Precision Arguments.  */
 	  sp -= len;
@@ -594,11 +629,8 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       else
 	{
 	  /* Integral and pointer arguments.  */
-	  gdb_assert (sparc_integral_or_pointer_p (type));
-
-	  if (len < 4)
-	    args[i] = value_cast (builtin_type (gdbarch)->builtin_int32,
-				  args[i]);
+	  gdb_assert (sparc_integral_or_pointer_p (type)
+	              || (TYPE_CODE (type) == TYPE_CODE_ARRAY && len <= 8));
 	  num_elements += ((len + 3) / 4);
 	}
     }
@@ -620,6 +652,15 @@ sparc32_store_arguments (struct regcache *regcache, int nargs,
       const bfd_byte *valbuf = value_contents (args[i]);
       struct type *type = value_type (args[i]);
       int len = TYPE_LENGTH (type);
+      gdb_byte buf[4];
+
+      if (len < 4)
+        {
+          memset (buf, 0, 4 - len);
+          memcpy (buf + 4 - len, valbuf, len);
+          valbuf = buf;
+          len = 4;
+        }
 
       gdb_assert (len == 4 || len == 8);
 
@@ -1345,10 +1386,10 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
   int len = TYPE_LENGTH (type);
   gdb_byte buf[32];
 
-  gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
+  gdb_assert (!sparc_structure_return_p (type));
 
-  if (sparc_floating_p (type) || sparc_complex_floating_p (type))
+  if (sparc_floating_p (type) || sparc_complex_floating_p (type)
+      || TYPE_CODE (type) == TYPE_CODE_ARRAY)
     {
       /* Floating return values.  */
       regcache_cooked_read (regcache, SPARC_F0_REGNUM, buf);
@@ -1397,11 +1438,9 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
 			    const gdb_byte *valbuf)
 {
   int len = TYPE_LENGTH (type);
-  gdb_byte buf[8];
+  gdb_byte buf[32];
 
-  gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
-  gdb_assert (len <= 8);
+  gdb_assert (!sparc_structure_return_p (type));
 
   if (sparc_floating_p (type) || sparc_complex_floating_p (type))
     {
@@ -1457,8 +1496,7 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
      guarantees that we can always find the return value, not just
      before the function returns.  */
 
-  if (sparc_structure_or_union_p (type)
-      || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
+  if (sparc_structure_return_p (type))
     {
       ULONGEST sp;
       CORE_ADDR addr;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 58f5bf0..9e0e6b5 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -669,6 +669,13 @@ static const struct frame_base sparc64_frame_base =
 static int
 sparc64_16_byte_align_p (struct type *type)
 {
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      struct type *t = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (sparc64_floating_p (t))
+        return 1;
+    }
   if (sparc64_floating_p (type) && TYPE_LENGTH (type) == 16)
     return 1;
 
@@ -703,7 +710,23 @@ sparc64_store_floating_fields (struct regcache *regcache, struct type *type,
 
   gdb_assert (element < 16);
 
-  if (sparc64_floating_p (type)
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      gdb_byte buf[8];
+      int regnum = SPARC_F0_REGNUM + element * 2 + bitpos / 32;
+
+      valbuf += bitpos / 8;
+      if (len < 8)
+        {
+          memset (buf, 0, 8 - len);
+          memcpy (buf + 8 - len, valbuf, len);
+          valbuf = buf;
+          len = 8;
+        }
+      for (int n = 0; n < (len + 3) / 4; n++)
+        regcache_cooked_write (regcache, regnum + n, valbuf + n * 4);
+    }
+  else if (sparc64_floating_p (type)
       || (sparc64_complex_floating_p (type) && len <= 16))
     {
       int regnum;
@@ -776,7 +799,23 @@ sparc64_extract_floating_fields (struct regcache *regcache, struct type *type,
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
 
-  if (sparc64_floating_p (type))
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      int len = TYPE_LENGTH (type);
+      int regnum =  SPARC_F0_REGNUM + bitpos / 32;
+
+      valbuf += bitpos / 8;
+      if (len < 4)
+        {
+          gdb_byte buf[4];
+          regcache_cooked_read (regcache, regnum, buf);
+          memcpy (valbuf, buf + 4 - len, len);
+        }
+      else
+        for (int i = 0; i < (len + 3) / 4; i++)
+          regcache_cooked_read (regcache, regnum + i, valbuf + i * 4);
+    }
+  else if (sparc64_floating_p (type))
     {
       int len = TYPE_LENGTH (type);
       int regnum;
-- 
1.7.1

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
  2017-03-29 16:38   ` Vladimir Mezentsev
@ 2017-04-18 14:54     ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2017-04-18 14:54 UTC (permalink / raw)
  To: vladimir.mezentsev, gdb-patches

Hi Vladimir,

On 03/29/2017 05:38 PM, Vladimir Mezentsev wrote:
> On 03/29/2017 02:56 AM, Pedro Alves wrote:
>> Hi Vladimir,
>>
>> gdb development happens on a separate list - gdb-patches@.  Please resend
>> the patch there.  TIA!
> 
>  Done.
>>
>> A question I'll have is whether this had any observable effect in
>> testsuite
>> results, other than "no regressions".  I.e., whether there were
>> progressions,
>> or whether we're missing testsuite coverage.
> 
> 6 tests ( from gdb/testsuite/gdb.base/gnu_vector.exp) failed on
> sparc64-Linux  and on sparc-Solaris in 32- and 64-bit mode.
> Now all these tests passed.
> 
> gdb/testsuite/gdb.base/gnu_vector.exp has 117 different cases for small
> (and not small) arrays and structures.

This information should be included in the commit log.

> I think we don't need an additional test in gdb/testsuite.

Agreed.

Thanks,
Pedro Alves

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

* Re: [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc
       [not found] ` <5a3397ae-e986-8a36-13ba-ed2cfeb68a12@redhat.com>
@ 2017-03-29 16:38   ` Vladimir Mezentsev
  2017-04-18 14:54     ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Mezentsev @ 2017-03-29 16:38 UTC (permalink / raw)
  To: Pedro Alves, binutils, gdb-patches

On 03/29/2017 02:56 AM, Pedro Alves wrote:
> Hi Vladimir,
>
> gdb development happens on a separate list - gdb-patches@.  Please resend
> the patch there.  TIA!

  Done.
>
> A question I'll have is whether this had any observable effect in testsuite
> results, other than "no regressions".  I.e., whether there were progressions,
> or whether we're missing testsuite coverage.

6 tests ( from gdb/testsuite/gdb.base/gnu_vector.exp) failed on 
sparc64-Linux  and on sparc-Solaris in 32- and 64-bit mode.
Now all these tests passed.

gdb/testsuite/gdb.base/gnu_vector.exp has 117 different cases for small 
(and not small) arrays and structures.
I think we don't need an additional test in gdb/testsuite.

-Vladimir


>
> On 03/28/2017 06:21 PM, vladimir.mezentsev@oracle.com wrote:
>> From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
>>
>> 	gdb has a special type (TYPE_CODE_ARRAY) to support the gcc extension
>> 	(https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html).
>> 	TYPE_CODE_ARRAY is handled incorrectly for both (32- and 64-bit) modes on Sparc machines.
>>
>> 	Tested on sparc64-linux-gnu and sparc-solaris (32- and 64-bit mode).
>> 	No regressions.
>>
>> 	2017-03-27 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
>>
>> 	* gdb/sparc-tdep.c (sparc_structure_return_p): New function.
> Also, please drop "gdb/" here.  Filename entries in ChangeLogs
> are relative to the ChangeLog file path.
>
> Thanks,
> Pedro Alves
>

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

end of thread, other threads:[~2017-05-19 10:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-29 16:21 [PATCH] gdb passes and returns incorrect values when dealing with small array on Sparc vladimir.mezentsev
2017-04-18 14:54 ` Pedro Alves
2017-04-18 21:28   ` Jose E. Marchesi
2017-04-18 22:20     ` Pedro Alves
2017-04-19  9:01       ` Jose E. Marchesi
     [not found] <1490721669-12560-1-git-send-email-vladimir.mezentsev@oracle.com>
     [not found] ` <5a3397ae-e986-8a36-13ba-ed2cfeb68a12@redhat.com>
2017-03-29 16:38   ` Vladimir Mezentsev
2017-04-18 14:54     ` Pedro Alves
2017-05-16 16:16 vladimir.mezentsev
2017-05-19  9:44 ` Jose E. Marchesi
2017-05-19  9:57   ` Pedro Alves
2017-05-19 10:19     ` Jose E. Marchesi

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