Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Fix DW_OP_GNU_regval_type with FP registers
@ 2013-11-14 15:21 Joel Brobecker
  2013-11-14 20:12 ` Pedro Alves
  2013-11-14 21:02 ` Tom Tromey
  0 siblings, 2 replies; 9+ messages in thread
From: Joel Brobecker @ 2013-11-14 15:21 UTC (permalink / raw)
  To: gdb-patches

Hello,

Consider the following code, compiled at -O2 on ppc-linux:

    procedure Increment (Val : in out Float; Msg : String);

The implementation does not really matter in this case). In our example,
this function is being called from a function with Param_1 set to 99.0.
Trying to break inside that function, and running until reaching that
breakpoint yields:

    (gdb) b increment
    Breakpoint 1 at 0x100014b4: file callee.adb, line 6.
    (gdb) run
    Starting program: /[...]/foo

    Breakpoint 1, callee.increment (val=99.0, val@entry=0.0, msg=...)
        at callee.adb:6
    6             if Val > 200.0 then

The @entry value for parameter "val" is incorrect, it should be 99.0.

The associated call-site parameter DIE looks like this:

        .uleb128 0xc     # (DIE (0x115) DW_TAG_GNU_call_site_parameter)
        .byte   0x2      # DW_AT_location
        .byte   0x90     # DW_OP_regx
        .uleb128 0x21
        .byte   0x3      # DW_AT_GNU_call_site_value
        .byte   0xf5     # DW_OP_GNU_regval_type
        .uleb128 0x3f
        .uleb128 0x25

The DW_AT_GNU_call_site_value uses a DW_OP_GNU_regval_type
operation, referencing register 0x3f=63, which is $f31,
an 8-byte floating register. In that register, the value is
stored using the usual 8-byte float format:

    (gdb) info float
    f31            99.0 (raw 0x4058c00000000000)

The current code evaluating DW_OP_GNU_regval_type operations
currently is (dwarf2expr.c:execute_stack_op):

            result = (ctx->funcs->read_reg) (ctx->baton, reg);
            result_val = value_from_ulongest (address_type, result);
            result_val = value_from_contents (type,
                                              value_contents_all (result_val));

What the ctx->funcs->read_reg function does is read the contents
of the register as if it contained an address. The rest of the code
continues that assumption, thinking it's OK to then use that to
create an address/ulongest struct value, which we then re-type
to the type specified by DW_OP_GNU_regval_type.

We're getting 0.0 above because the read_reg implementations
end up treating the contents of the FP register as an integral,
reading only 4 out of the 8 bytes. Being a big-endian target,
we read the high-order ones, which gives us zero.

This patch fixes the problem by introducing a new callback to
read the contents of a register as a given type, and then adjust
the handling of DW_OP_GNU_regval_type to use that new callback.

gdb/ChangeLog:

        * dwarf2expr.h (struct dwarf_expr_context_funcs) <read_reg>:
        Extend the documentation a bit.
        <get_reg_value>: New field.
        * dwarf2loc.c (dwarf_expr_get_reg_value)
        (needs_frame_get_reg_value): New functions.
        (dwarf_expr_ctx_funcs, needs_frame_ctx_funcs): Add "get_reg_value"
        callback.
        * dwarf2-frame.c (get_reg_value): New function.
        (dwarf2_frame_ctx_funcs): Add "get_reg_value" callback.
        * dwarf2expr.c (execute_stack_op) <DW_OP_GNU_regval_type>:
        Use new callback to compute result_val.

gdb/testsuite/ChangeLog:

        * O2_float_param: New testcase.

tested on x86_64-linux and ppc-linux.
OK to commit?

Thank you,
-- 
Joel

---
 gdb/dwarf2-frame.c                              |   13 +++++++++
 gdb/dwarf2expr.c                                |    5 +---
 gdb/dwarf2expr.h                                |    9 ++++++-
 gdb/dwarf2loc.c                                 |   26 +++++++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param.exp        |   31 +++++++++++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/callee.adb |   26 +++++++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/callee.ads |   18 +++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/caller.adb |   26 +++++++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/caller.ads |   19 ++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/foo.adb    |   22 ++++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/io.adb     |   21 +++++++++++++++
 gdb/testsuite/gdb.ada/O2_float_param/io.ads     |   18 +++++++++++++
 12 files changed, 229 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param.exp
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/callee.adb
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/callee.ads
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/caller.adb
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/caller.ads
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/foo.adb
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/io.adb
 create mode 100644 gdb/testsuite/gdb.ada/O2_float_param/io.ads

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index e05236f..b425d94 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -306,6 +306,18 @@ read_reg (void *baton, int reg)
   return unpack_long (register_type (gdbarch, regnum), buf);
 }
 
+/* Implement struct dwarf_expr_context_funcs' "get_reg_value" callback.  */
+
+static struct value *
+get_reg_value (void *baton, struct type *type, int reg)
+{
+  struct frame_info *this_frame = (struct frame_info *) baton;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, reg);
+
+  return value_from_register (type, regnum, this_frame);
+}
+
 static void
 read_mem (void *baton, gdb_byte *buf, CORE_ADDR addr, size_t len)
 {
@@ -347,6 +359,7 @@ register %s (#%d) at %s"),
 static const struct dwarf_expr_context_funcs dwarf2_frame_ctx_funcs =
 {
   read_reg,
+  get_reg_value,
   read_mem,
   ctx_no_get_frame_base,
   ctx_no_get_frame_cfa,
diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
index 752d782..25e9dc4 100644
--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -1439,10 +1439,7 @@ execute_stack_op (struct dwarf_expr_context *ctx,
 	    type_die.cu_off = uoffset;
 
 	    type = dwarf_get_base_type (ctx, type_die, 0);
-	    result = (ctx->funcs->read_reg) (ctx->baton, reg);
-	    result_val = value_from_ulongest (address_type, result);
-	    result_val = value_from_contents (type,
-					      value_contents_all (result_val));
+	    result_val = ctx->funcs->get_reg_value (ctx->baton, type, reg);
 	  }
 	  break;
 
diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h
index e85486a..b4c943e 100644
--- a/gdb/dwarf2expr.h
+++ b/gdb/dwarf2expr.h
@@ -31,9 +31,16 @@ struct dwarf_expr_context;
 
 struct dwarf_expr_context_funcs
 {
-  /* Return the value of register number REGNUM.  */
+  /* Return the value of register number REGNUM (a DWARF register number),
+     read as an address.  */
   CORE_ADDR (*read_reg) (void *baton, int regnum);
 
+  /* Return a value of type TYPE, stored in register number REGNUM
+     of the frame associated to the given BATON.
+
+     REGNUM is a DWARF register number.  */
+  struct value *(*get_reg_value) (void *baton, struct type *type, int regnum);
+
   /* Read LENGTH bytes at ADDR into BUF.  */
   void (*read_mem) (void *baton, gdb_byte *buf, CORE_ADDR addr, size_t length);
 
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 8242dca..8b6eb66 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -326,6 +326,18 @@ dwarf_expr_read_reg (void *baton, int dwarf_regnum)
   return result;
 }
 
+/* Implement struct dwarf_expr_context_funcs' "get_reg_value" callback.  */
+
+static struct value *
+dwarf_expr_get_reg_value (void *baton, struct type *type, int dwarf_regnum)
+{
+  struct dwarf_expr_baton *debaton = (struct dwarf_expr_baton *) baton;
+  struct gdbarch *gdbarch = get_frame_arch (debaton->frame);
+  int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, dwarf_regnum);
+
+  return value_from_register (type, regnum, debaton->frame);
+}
+
 /* Read memory at ADDR (length LEN) into BUF.  */
 
 static void
@@ -2182,6 +2194,7 @@ static const struct lval_funcs pieced_value_funcs = {
 static const struct dwarf_expr_context_funcs dwarf_expr_ctx_funcs =
 {
   dwarf_expr_read_reg,
+  dwarf_expr_get_reg_value,
   dwarf_expr_read_mem,
   dwarf_expr_frame_base,
   dwarf_expr_frame_cfa,
@@ -2435,6 +2448,18 @@ needs_frame_read_reg (void *baton, int regnum)
   return 1;
 }
 
+/* struct dwarf_expr_context_funcs' "get_reg_value" callback:
+   Reads from registers do require a frame.  */
+
+static struct value *
+needs_frame_get_reg_value (void *baton, struct type *type, int regnum)
+{
+  struct needs_frame_baton *nf_baton = baton;
+
+  nf_baton->needs_frame = 1;
+  return value_zero (type, not_lval);
+}
+
 /* Reads from memory do not require a frame.  */
 static void
 needs_frame_read_mem (void *baton, gdb_byte *buf, CORE_ADDR addr, size_t len)
@@ -2516,6 +2541,7 @@ needs_get_addr_index (void *baton, unsigned int index)
 static const struct dwarf_expr_context_funcs needs_frame_ctx_funcs =
 {
   needs_frame_read_reg,
+  needs_frame_get_reg_value,
   needs_frame_read_mem,
   needs_frame_frame_base,
   needs_frame_frame_cfa,
diff --git a/gdb/testsuite/gdb.ada/O2_float_param.exp b/gdb/testsuite/gdb.ada/O2_float_param.exp
new file mode 100644
index 0000000..8b891e8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param.exp
@@ -0,0 +1,31 @@
+# Copyright 2013 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug optimize=-O2}] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+runto "increment"
+
+gdb_test "frame" \
+         "#0\\s+callee\\.increment \\(val(=val@entry)?=99\\.0, msg=\\.\\.\\.\\).*"
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/callee.adb b/gdb/testsuite/gdb.ada/O2_float_param/callee.adb
new file mode 100644
index 0000000..0ce4024
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/callee.adb
@@ -0,0 +1,26 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with IO; use IO;
+
+package body Callee is
+   procedure Increment (Val : in out Float; Msg: String) is
+   begin
+      if Val > 200.0 then
+         Put_Line (Msg);
+      end if;
+      Val := Val + 1.0;
+   end Increment;
+end Callee;
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/callee.ads b/gdb/testsuite/gdb.ada/O2_float_param/callee.ads
new file mode 100644
index 0000000..92d97c3
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/callee.ads
@@ -0,0 +1,18 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Callee is
+   procedure Increment (Val : in out Float; Msg : String);
+end Callee;
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/caller.adb b/gdb/testsuite/gdb.ada/O2_float_param/caller.adb
new file mode 100644
index 0000000..36666c2
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/caller.adb
@@ -0,0 +1,26 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with IO; use IO;
+with Callee; use Callee;
+
+package body Caller is
+   procedure Verbose_Increment (Val : in out Float; Msg : String) is
+   begin
+      Put_Line ("DEBUG: " & Msg);
+      Increment (Val, "Verbose_Increment");
+   end Verbose_Increment;
+end Caller;
+
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/caller.ads b/gdb/testsuite/gdb.ada/O2_float_param/caller.ads
new file mode 100644
index 0000000..e44f62b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/caller.ads
@@ -0,0 +1,19 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Caller is
+   procedure Verbose_Increment (Val : in out Float; Msg : String);
+end Caller;
+
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/foo.adb b/gdb/testsuite/gdb.ada/O2_float_param/foo.adb
new file mode 100644
index 0000000..d6289ff
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/foo.adb
@@ -0,0 +1,22 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Caller; use Caller;
+
+procedure Foo is
+   Num : Float := 99.0;
+begin
+   Verbose_Increment (Num, "Foo");
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/io.adb b/gdb/testsuite/gdb.ada/O2_float_param/io.adb
new file mode 100644
index 0000000..7ef1689
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/io.adb
@@ -0,0 +1,21 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body IO is
+   procedure Put_Line (S : String) is
+   begin
+      null;
+   end Put_Line;
+end IO;
diff --git a/gdb/testsuite/gdb.ada/O2_float_param/io.ads b/gdb/testsuite/gdb.ada/O2_float_param/io.ads
new file mode 100644
index 0000000..a09f4fe
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/O2_float_param/io.ads
@@ -0,0 +1,18 @@
+--  Copyright 2013 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package IO is
+   procedure Put_Line (S : String);
+end IO;
-- 
1.7.0.4


^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-11-18 12:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-14 15:21 [RFA] Fix DW_OP_GNU_regval_type with FP registers Joel Brobecker
2013-11-14 20:12 ` Pedro Alves
2013-11-14 21:02 ` Tom Tromey
2013-11-15 12:19   ` Joel Brobecker
2013-11-15 14:54     ` Pedro Alves
2013-11-16  5:46       ` [RFA] Rename "read_reg" into "read_addr_from_reg" in struct dwarf_expr_context_funcs Joel Brobecker
2013-11-17  3:02         ` Pedro Alves
2013-11-17  9:43           ` pushed: " Joel Brobecker
2013-11-18 12:39             ` [OB] Simplify dwarf2-frame.c:read_addr_from_reg. (was: [RFA] Rename "read_reg" into "read_addr_from_reg" in struct dwarf_expr_context_funcs) Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox