public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix fixed-point regression with recent GCC
@ 2021-01-22 20:40 Tom Tromey
  2021-01-25  7:52 ` Joel Brobecker
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Tromey @ 2021-01-22 20:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

A recent version of GCC changed how fixed-point types are described.
For example, a denominator in one test case now looks like:

    GNU_denominator      (exprloc)
     [ 0] implicit_value: 16 byte block: 00 00 b8 9d 0d 69 55 a0 01 00 00 00 00 00 00 00

... the difference being that this now uses exprloc and emits a
DW_OP_implicit_value for the 16-byte block.  (DWARF 5 still uses
DW_FORM_data16.)

This change was made here:

    https://gcc.gnu.org/pipermail/gcc-patches/2020-December/560897.html

This patch updates gdb to handle this situation.

Note that, before GCC 11, this test would not give the same answer.
Earlier versions of GCC fell back to GNAT encodings for this case.

gdb/ChangeLog
2021-01-22  Tom Tromey  <tromey@adacore.com>

	* dwarf2/read.c (get_mpz): New function.
	(get_dwarf2_rational_constant): Use it.

gdb/testsuite/ChangeLog
2021-01-22  Tom Tromey  <tromey@adacore.com>

	* gdb.ada/fixed_points.exp: Add regression test.
	* gdb.ada/fixed_points/fixed_points.adb (FP5_Var): New variable.
	* gdb.ada/fixed_points/pck.adb (Delta5, FP5_Type): New.
---
 gdb/ChangeLog                                 |  5 ++
 gdb/dwarf2/read.c                             | 61 +++++++++++++------
 gdb/testsuite/ChangeLog                       |  6 ++
 gdb/testsuite/gdb.ada/fixed_points.exp        |  5 ++
 .../gdb.ada/fixed_points/fixed_points.adb     |  3 +
 gdb/testsuite/gdb.ada/fixed_points/pck.ads    |  4 ++
 6 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 309ff8331e7..ae95c650f1e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18228,6 +18228,46 @@ read_typedef (struct die_info *die, struct dwarf2_cu *cu)
   return this_type;
 }
 
+/* Helper for get_dwarf2_rational_constant that computes the value of
+   a given gmp_mpz given an attribute.  */
+
+static void
+get_mpz (struct dwarf2_cu *cu, gdb_mpz *value, struct attribute *attr)
+{
+  /* GCC will sometimes emit a 16-byte constant value as a DWARF
+     location expression that pushes an implicit value.  */
+  if (attr->form == DW_FORM_exprloc)
+    {
+      dwarf_block *blk = attr->as_block ();
+      if (blk->size > 0 && blk->data[0] == DW_OP_implicit_value)
+	{
+	  uint64_t len;
+	  const gdb_byte *ptr = safe_read_uleb128 (blk->data + 1,
+						   blk->data + blk->size,
+						   &len);
+	  if (ptr - blk->data + len <= blk->size)
+	    {
+	      mpz_import (value->val, len,
+			  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
+			  1, 0, 0, ptr);
+	      return;
+	    }
+	}
+
+      /* On failure set it to 1.  */
+      *value = gdb_mpz (1);
+    }
+  else if (attr->form_is_block ())
+    {
+      dwarf_block *blk = attr->as_block ();
+      mpz_import (value->val, blk->size,
+		  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
+		  1, 0, 0, blk->data);
+    }
+  else
+    *value = gdb_mpz (attr->constant_value (1));
+}
+
 /* Assuming DIE is a rational DW_TAG_constant, read the DIE's
    numerator and denominator into NUMERATOR and DENOMINATOR (resp).
 
@@ -18254,25 +18294,8 @@ get_dwarf2_rational_constant (struct die_info *die, struct dwarf2_cu *cu,
   if (num_attr == nullptr || denom_attr == nullptr)
     return;
 
-  if (num_attr->form_is_block ())
-    {
-      dwarf_block *blk = num_attr->as_block ();
-      mpz_import (numerator->val, blk->size,
-		  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
-		  1, 0, 0, blk->data);
-    }
-  else
-    *numerator = gdb_mpz (num_attr->constant_value (1));
-
-  if (denom_attr->form_is_block ())
-    {
-      dwarf_block *blk = denom_attr->as_block ();
-      mpz_import (denominator->val, blk->size,
-		  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
-		  1, 0, 0, blk->data);
-    }
-  else
-    *denominator = gdb_mpz (denom_attr->constant_value (1));
+  get_mpz (cu, numerator, num_attr);
+  get_mpz (cu, denominator, denom_attr);
 }
 
 /* Same as get_dwarf2_rational_constant, but extracting an unsigned
diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
index 0d244534975..ac45ef92ce9 100644
--- a/gdb/testsuite/gdb.ada/fixed_points.exp
+++ b/gdb/testsuite/gdb.ada/fixed_points.exp
@@ -123,5 +123,10 @@ foreach_with_prefix scenario {all minimal} {
 	gdb_test "print fp4_var * 1" $fp4
 	gdb_test "print 1 * fp4_var" $fp4
 	gdb_test "print fp4_var / 1" $fp4
+
+	# This only started working in GCC 11.
+	if {[test_compiler_info {gcc-11-*}]} {
+	    gdb_test "print fp5_var" " = 3e-19"
+	}
     }
 }
diff --git a/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb b/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
index cc2c6377761..4298cdf899e 100644
--- a/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
+++ b/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
@@ -55,6 +55,8 @@ procedure Fixed_Points is
    Overprecise_Object : Overprecise_Fixed_Point :=
      Overprecise_Fixed_Point'Small;
 
+   FP5_Var : FP5_Type := 3 * Delta5;
+
 begin
    Base_Object := 1.0/16.0;   -- Set breakpoint here
    Subtype_Object := 1.0/16.0;
@@ -64,4 +66,5 @@ begin
    Do_Nothing (FP2_Var'Address);
    Do_Nothing (FP3_Var'Address);
    Do_Nothing (FP4_Var'Address);
+   Do_Nothing (FP5_Var'Address);
 end Fixed_Points;
diff --git a/gdb/testsuite/gdb.ada/fixed_points/pck.ads b/gdb/testsuite/gdb.ada/fixed_points/pck.ads
index b5c1bc01c44..3bdf0595076 100644
--- a/gdb/testsuite/gdb.ada/fixed_points/pck.ads
+++ b/gdb/testsuite/gdb.ada/fixed_points/pck.ads
@@ -30,6 +30,10 @@ package Pck is
       with Small => Delta4 / 3.0;
    FP4_Var : FP4_Type := 2 * Delta4;
 
+   Delta5 : constant := 0.000_000_000_000_000_000_1;
+   type FP5_Type is delta Delta5 range 0.0 .. Delta5 * 10
+      with Small => Delta5 / 3.0;
+
    procedure Do_Nothing (A : System.Address);
 end pck;
 
-- 
2.26.2


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

* Re: [PATCH] Fix fixed-point regression with recent GCC
  2021-01-22 20:40 [PATCH] Fix fixed-point regression with recent GCC Tom Tromey
@ 2021-01-25  7:52 ` Joel Brobecker
  0 siblings, 0 replies; 2+ messages in thread
From: Joel Brobecker @ 2021-01-25  7:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hello,

On Fri, Jan 22, 2021 at 01:40:32PM -0700, Tom Tromey wrote:
> A recent version of GCC changed how fixed-point types are described.
> For example, a denominator in one test case now looks like:
> 
>     GNU_denominator      (exprloc)
>      [ 0] implicit_value: 16 byte block: 00 00 b8 9d 0d 69 55 a0 01 00 00 00 00 00 00 00
> 
> ... the difference being that this now uses exprloc and emits a
> DW_OP_implicit_value for the 16-byte block.  (DWARF 5 still uses
> DW_FORM_data16.)
> 
> This change was made here:
> 
>     https://gcc.gnu.org/pipermail/gcc-patches/2020-December/560897.html
> 
> This patch updates gdb to handle this situation.
> 
> Note that, before GCC 11, this test would not give the same answer.
> Earlier versions of GCC fell back to GNAT encodings for this case.
> 
> gdb/ChangeLog
> 2021-01-22  Tom Tromey  <tromey@adacore.com>
> 
> 	* dwarf2/read.c (get_mpz): New function.
> 	(get_dwarf2_rational_constant): Use it.
> 
> gdb/testsuite/ChangeLog
> 2021-01-22  Tom Tromey  <tromey@adacore.com>
> 
> 	* gdb.ada/fixed_points.exp: Add regression test.
> 	* gdb.ada/fixed_points/fixed_points.adb (FP5_Var): New variable.
> 	* gdb.ada/fixed_points/pck.adb (Delta5, FP5_Type): New.

FWIW, I suspect Tom is putting this patch up for review because
it touches areas outside ada-*, but I wanted to mention also that
I reviewed this patch when it was pushed internally at AdaCore,
and it looked good to me.

> ---
>  gdb/ChangeLog                                 |  5 ++
>  gdb/dwarf2/read.c                             | 61 +++++++++++++------
>  gdb/testsuite/ChangeLog                       |  6 ++
>  gdb/testsuite/gdb.ada/fixed_points.exp        |  5 ++
>  .../gdb.ada/fixed_points/fixed_points.adb     |  3 +
>  gdb/testsuite/gdb.ada/fixed_points/pck.ads    |  4 ++
>  6 files changed, 65 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 309ff8331e7..ae95c650f1e 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -18228,6 +18228,46 @@ read_typedef (struct die_info *die, struct dwarf2_cu *cu)
>    return this_type;
>  }
>  
> +/* Helper for get_dwarf2_rational_constant that computes the value of
> +   a given gmp_mpz given an attribute.  */
> +
> +static void
> +get_mpz (struct dwarf2_cu *cu, gdb_mpz *value, struct attribute *attr)
> +{
> +  /* GCC will sometimes emit a 16-byte constant value as a DWARF
> +     location expression that pushes an implicit value.  */
> +  if (attr->form == DW_FORM_exprloc)
> +    {
> +      dwarf_block *blk = attr->as_block ();
> +      if (blk->size > 0 && blk->data[0] == DW_OP_implicit_value)
> +	{
> +	  uint64_t len;
> +	  const gdb_byte *ptr = safe_read_uleb128 (blk->data + 1,
> +						   blk->data + blk->size,
> +						   &len);
> +	  if (ptr - blk->data + len <= blk->size)
> +	    {
> +	      mpz_import (value->val, len,
> +			  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
> +			  1, 0, 0, ptr);
> +	      return;
> +	    }
> +	}
> +
> +      /* On failure set it to 1.  */
> +      *value = gdb_mpz (1);
> +    }
> +  else if (attr->form_is_block ())
> +    {
> +      dwarf_block *blk = attr->as_block ();
> +      mpz_import (value->val, blk->size,
> +		  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
> +		  1, 0, 0, blk->data);
> +    }
> +  else
> +    *value = gdb_mpz (attr->constant_value (1));
> +}
> +
>  /* Assuming DIE is a rational DW_TAG_constant, read the DIE's
>     numerator and denominator into NUMERATOR and DENOMINATOR (resp).
>  
> @@ -18254,25 +18294,8 @@ get_dwarf2_rational_constant (struct die_info *die, struct dwarf2_cu *cu,
>    if (num_attr == nullptr || denom_attr == nullptr)
>      return;
>  
> -  if (num_attr->form_is_block ())
> -    {
> -      dwarf_block *blk = num_attr->as_block ();
> -      mpz_import (numerator->val, blk->size,
> -		  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
> -		  1, 0, 0, blk->data);
> -    }
> -  else
> -    *numerator = gdb_mpz (num_attr->constant_value (1));
> -
> -  if (denom_attr->form_is_block ())
> -    {
> -      dwarf_block *blk = denom_attr->as_block ();
> -      mpz_import (denominator->val, blk->size,
> -		  bfd_big_endian (cu->per_objfile->objfile->obfd) ? 1 : -1,
> -		  1, 0, 0, blk->data);
> -    }
> -  else
> -    *denominator = gdb_mpz (denom_attr->constant_value (1));
> +  get_mpz (cu, numerator, num_attr);
> +  get_mpz (cu, denominator, denom_attr);
>  }
>  
>  /* Same as get_dwarf2_rational_constant, but extracting an unsigned
> diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
> index 0d244534975..ac45ef92ce9 100644
> --- a/gdb/testsuite/gdb.ada/fixed_points.exp
> +++ b/gdb/testsuite/gdb.ada/fixed_points.exp
> @@ -123,5 +123,10 @@ foreach_with_prefix scenario {all minimal} {
>  	gdb_test "print fp4_var * 1" $fp4
>  	gdb_test "print 1 * fp4_var" $fp4
>  	gdb_test "print fp4_var / 1" $fp4
> +
> +	# This only started working in GCC 11.
> +	if {[test_compiler_info {gcc-11-*}]} {
> +	    gdb_test "print fp5_var" " = 3e-19"
> +	}
>      }
>  }
> diff --git a/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb b/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
> index cc2c6377761..4298cdf899e 100644
> --- a/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
> +++ b/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
> @@ -55,6 +55,8 @@ procedure Fixed_Points is
>     Overprecise_Object : Overprecise_Fixed_Point :=
>       Overprecise_Fixed_Point'Small;
>  
> +   FP5_Var : FP5_Type := 3 * Delta5;
> +
>  begin
>     Base_Object := 1.0/16.0;   -- Set breakpoint here
>     Subtype_Object := 1.0/16.0;
> @@ -64,4 +66,5 @@ begin
>     Do_Nothing (FP2_Var'Address);
>     Do_Nothing (FP3_Var'Address);
>     Do_Nothing (FP4_Var'Address);
> +   Do_Nothing (FP5_Var'Address);
>  end Fixed_Points;
> diff --git a/gdb/testsuite/gdb.ada/fixed_points/pck.ads b/gdb/testsuite/gdb.ada/fixed_points/pck.ads
> index b5c1bc01c44..3bdf0595076 100644
> --- a/gdb/testsuite/gdb.ada/fixed_points/pck.ads
> +++ b/gdb/testsuite/gdb.ada/fixed_points/pck.ads
> @@ -30,6 +30,10 @@ package Pck is
>        with Small => Delta4 / 3.0;
>     FP4_Var : FP4_Type := 2 * Delta4;
>  
> +   Delta5 : constant := 0.000_000_000_000_000_000_1;
> +   type FP5_Type is delta Delta5 range 0.0 .. Delta5 * 10
> +      with Small => Delta5 / 3.0;
> +
>     procedure Do_Nothing (A : System.Address);
>  end pck;
>  
> -- 
> 2.26.2

-- 
Joel

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

end of thread, other threads:[~2021-01-25  7:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 20:40 [PATCH] Fix fixed-point regression with recent GCC Tom Tromey
2021-01-25  7:52 ` Joel Brobecker

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