Fix bug in fixed-point handling
Commit Message
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(+)
Comments
>>>>> "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
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
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"
+ }
}
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"
> + }
> }
>>>>> "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"
+ }
}
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"
> + }
> }
Tom> Tested with gcc 7 - 12, only expected passes.
Thanks, I'll write a commit message and check it in shortly.
Tom
@@ -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));
}
@@ -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"
}
@@ -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;