[PATCH] Fix bug in fixed-point handling

[PATCH] Fix bug in fixed-point handling

[Tom Tromey]

Alexandre Oliva found a bug in gdb's handling of fixed-point -- a
certain Ada fixed-point type would be misintepreted.  The bug was that
the DW_AT_small looked like:

 <1><13cd>: Abbrev Number: 16 (DW_TAG_constant)
    <13ce>   DW_AT_GNU_numerator: 1
    <13cf>   DW_AT_GNU_denominator: 0x8000000000000000

... but gdb interpreted the denominator as a negative value.
---
 gdb/dwarf2/read.c                                   | 2 ++
 gdb/testsuite/gdb.ada/fixed_points.exp              | 3 +++
 gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3508f2c29ee..c198f6c5857 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14867,6 +14867,8 @@ get_mpz (struct dwarf2_cu *cu, gdb_mpz *value, struct attribute *attr)
 		   ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE,
 		   true);
     }
+  else if (attr->form_is_unsigned ())
+    *value = gdb_mpz (attr->as_unsigned ());
   else
     *value = gdb_mpz (attr->constant_value (1));
 }
diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
index ed61cab4bec..2edc63f4071 100644
--- a/gdb/testsuite/gdb.ada/fixed_points.exp
+++ b/gdb/testsuite/gdb.ada/fixed_points.exp
@@ -91,4 +91,7 @@ foreach_with_prefix scenario {all minimal} {
     if {$scenario == "minimal" && [test_compiler_info {gcc-11-*}]} {
 	gdb_test "print fp5_var" " = 3e-19"
     }
+
+    gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
+	"value of another_fixed"
 }
diff --git a/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb b/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
index 67d38f2770b..edcbac80ef9 100644
--- a/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
+++ b/gdb/testsuite/gdb.ada/fixed_points/fixed_points.adb
@@ -57,6 +57,13 @@ procedure Fixed_Points is
 
    FP5_Var : FP5_Type := 3 * Delta5;
 
+
+   Another_Delta : constant := 1.0/(2**63);
+   type Another_Type is delta Another_Delta range -1.0 .. (1.0 - Another_Delta);
+   for  Another_Type'small use Another_Delta;
+   for  Another_Type'size  use 64;
+   Another_Fixed : Another_Type := Another_Delta * 5;
+
 begin
    Base_Object := 1.0/16.0;   -- Set breakpoint here
    Subtype_Object := 1.0/16.0;
@@ -67,4 +74,5 @@ begin
    Do_Nothing (FP3_Var'Address);
    Do_Nothing (FP4_Var'Address);
    Do_Nothing (FP5_Var'Address);
+   Do_Nothing (Another_Fixed'Address);
 end Fixed_Points;
-- 
2.40.1

Re: [PATCH] Fix bug in fixed-point handling

[Tom Tromey]

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Alexandre Oliva found a bug in gdb's handling of fixed-point -- a
Tom> certain Ada fixed-point type would be misintepreted.  The bug was that
Tom> the DW_AT_small looked like:

Tom>  <1><13cd>: Abbrev Number: 16 (DW_TAG_constant)
Tom>     <13ce>   DW_AT_GNU_numerator: 1
Tom>     <13cf>   DW_AT_GNU_denominator: 0x8000000000000000

Tom> ... but gdb interpreted the denominator as a negative value.

I'm checking this in.

Tom

Re: [PATCH] Fix bug in fixed-point handling

[Tom de Vries]

On 7/31/23 18:38, Tom Tromey via Gdb-patches wrote:
>>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Alexandre Oliva found a bug in gdb's handling of fixed-point -- a
> Tom> certain Ada fixed-point type would be misintepreted.  The bug was that
> Tom> the DW_AT_small looked like:
> 
> Tom>  <1><13cd>: Abbrev Number: 16 (DW_TAG_constant)
> Tom>     <13ce>   DW_AT_GNU_numerator: 1
> Tom>     <13cf>   DW_AT_GNU_denominator: 0x8000000000000000
> 
> Tom> ... but gdb interpreted the denominator as a negative value.
> 
> I'm checking this in.

Test-case fails with gcc-9 and earlier:
...
(gdb) PASS: gdb.ada/fixed_points.exp: scenario=all: print fp4_var / 1
p Float(Another_Fixed) = Float(Another_Delta * 5)^M
No definition of "another_delta" in current context.^M
(gdb) FAIL: gdb.ada/fixed_points.exp: scenario=all: value of another_fixed
...

Thanks,
- Tom

Re: [PATCH] Fix bug in fixed-point handling

[Tom Tromey]

Tom> Test-case fails with gcc-9 and earlier:
Tom> ...
Tom> (gdb) PASS: gdb.ada/fixed_points.exp: scenario=all: print fp4_var / 1
Tom> p Float(Another_Fixed) = Float(Another_Delta * 5)^M
Tom> No definition of "another_delta" in current context.^M
Tom> (gdb) FAIL: gdb.ada/fixed_points.exp: scenario=all: value of another_fixed
Tom> ...

Did you happen to try with GCC 10?

I'm wondering if the appended is enough or if it should be conditional
on gcc 10 specifically.

Tom

diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
index 2edc63f4071..12f1adfc249 100644
--- a/gdb/testsuite/gdb.ada/fixed_points.exp
+++ b/gdb/testsuite/gdb.ada/fixed_points.exp
@@ -90,8 +90,8 @@ foreach_with_prefix scenario {all minimal} {
     # This only started working in GCC 11.
     if {$scenario == "minimal" && [test_compiler_info {gcc-11-*}]} {
 	gdb_test "print fp5_var" " = 3e-19"
-    }
 
-    gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
-	"value of another_fixed"
+	gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
+	    "value of another_fixed"
+    }
 }

Re: [PATCH] Fix bug in fixed-point handling

[Tom de Vries]

On 8/1/23 16:35, Tom Tromey wrote:
> Tom> Test-case fails with gcc-9 and earlier:
> Tom> ...
> Tom> (gdb) PASS: gdb.ada/fixed_points.exp: scenario=all: print fp4_var / 1
> Tom> p Float(Another_Fixed) = Float(Another_Delta * 5)^M
> Tom> No definition of "another_delta" in current context.^M
> Tom> (gdb) FAIL: gdb.ada/fixed_points.exp: scenario=all: value of another_fixed
> Tom> ...
> 
> Did you happen to try with GCC 10?
> 

Yes, the test-cases passes for gcc 10, 11 and 12, and fails for gcc 7, 8 
and 9.

> I'm wondering if the appended is enough or if it should be conditional
> on gcc 10 specifically.
> 

With the patch applied, it passes for all the above.

Thanks,
- Tom

> diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
> index 2edc63f4071..12f1adfc249 100644
> --- a/gdb/testsuite/gdb.ada/fixed_points.exp
> +++ b/gdb/testsuite/gdb.ada/fixed_points.exp
> @@ -90,8 +90,8 @@ foreach_with_prefix scenario {all minimal} {
>       # This only started working in GCC 11.
>       if {$scenario == "minimal" && [test_compiler_info {gcc-11-*}]} {
>   	gdb_test "print fp5_var" " = 3e-19"
> -    }
>   
> -    gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
> -	"value of another_fixed"
> +	gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
> +	    "value of another_fixed"
> +    }
>   }

Re: [PATCH] Fix bug in fixed-point handling

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Yes, the test-cases passes for gcc 10, 11 and 12, and fails for gcc 7,
Tom> 8 and 9.

>> I'm wondering if the appended is enough or if it should be conditional
>> on gcc 10 specifically.
>> 

Tom> With the patch applied, it passes for all the above.

Would you mind trying the appended?  It tightens the test to make it
specific to the "all" scenario and to be skipped only for GCC < 10.

thanks,
Tom

diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
index 2edc63f4071..05e86b9d0ed 100644
--- a/gdb/testsuite/gdb.ada/fixed_points.exp
+++ b/gdb/testsuite/gdb.ada/fixed_points.exp
@@ -92,6 +92,9 @@ foreach_with_prefix scenario {all minimal} {
 	gdb_test "print fp5_var" " = 3e-19"
     }
 
-    gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
-	"value of another_fixed"
+    # This failed before GCC 10.
+    if {$scenario == "all" &&  [test_compiler_info {gcc-10-*}]} {
+	gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
+	    "value of another_fixed"
+    }
 }

Re: [PATCH] Fix bug in fixed-point handling

[Tom de Vries]

On 8/2/23 17:10, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Yes, the test-cases passes for gcc 10, 11 and 12, and fails for gcc 7,
> Tom> 8 and 9.
> 
>>> I'm wondering if the appended is enough or if it should be conditional
>>> on gcc 10 specifically.
>>>
> 
> Tom> With the patch applied, it passes for all the above.
> 
> Would you mind trying the appended?  It tightens the test to make it
> specific to the "all" scenario and to be skipped only for GCC < 10.
> 

Tested with gcc 7 - 12, only expected passes.

Thanks,
- Tom

> thanks,
> Tom
> 
> diff --git a/gdb/testsuite/gdb.ada/fixed_points.exp b/gdb/testsuite/gdb.ada/fixed_points.exp
> index 2edc63f4071..05e86b9d0ed 100644
> --- a/gdb/testsuite/gdb.ada/fixed_points.exp
> +++ b/gdb/testsuite/gdb.ada/fixed_points.exp
> @@ -92,6 +92,9 @@ foreach_with_prefix scenario {all minimal} {
>   	gdb_test "print fp5_var" " = 3e-19"
>       }
>   
> -    gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
> -	"value of another_fixed"
> +    # This failed before GCC 10.
> +    if {$scenario == "all" &&  [test_compiler_info {gcc-10-*}]} {
> +	gdb_test "p Float(Another_Fixed) = Float(Another_Delta * 5)" "true" \
> +	    "value of another_fixed"
> +    }
>   }

Re: [PATCH] Fix bug in fixed-point handling

[Tom Tromey]

Tom> Tested with gcc 7 - 12, only expected passes.

Thanks, I'll write a commit message and check it in shortly.

Tom