* [PATCH] optimized out registers in mi
@ 2013-07-05 13:28 Andrew Burgess
2013-07-05 14:03 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Burgess @ 2013-07-05 13:28 UTC (permalink / raw)
To: gdb-patches
Currently, the mi command -data-list-register-values
will given an error if any of the registers are optimized
out[1].
This patch changes to behaviour so the the value field
is returned containing the text "<optimized out>".
Ok to apply?
Thanks
Andrew
[1] I applied a patch yesterday that changed value_optimized_out,
before I applied yesterdays patch the behaviour of -data-list-registers
was a bit random, if the register value was lazy then you would get
the "<optimized out>" result, if the value was not lazy then you got
the error. Of the two behaviours I think returning the
"<optimized out>" string is by far the most helpful so that's
the behaviour we now get in all cases.
gdb/ChangeLog
2013-07-05 Andrew Burgess <aburgess@broadcom.com>
* mi/mi-main.c (get_register): Display optimized out register
using no special format rather than raising an error.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index d6c763e..01ea382 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1170,12 +1170,11 @@ output_register (struct frame_info *frame, int regnum, int format,
tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
ui_out_field_int (uiout, "number", regnum);
- if (format == 'N')
+ /* Displaying optimized out values using anything other than the default
+ format will result in the value 0 being shown. */
+ if (value_optimized_out (val) || format == 'N')
format = 0;
- if (value_optimized_out (val))
- error (_("Optimized out"));
-
if (format == 'r')
{
int j;
gdb/testsuite/ChangeLog
2013-07-05 Andrew Burgess <aburgess@broadcom.com>
* gdb.mi/mi-reg-undefined.exp: New file.
* gdb.mi/mi-reg-undefined.c: Likewise.
* gdb.mi/mi-reg-undefined.S: Likewise.
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.S b/gdb/testsuite/gdb.mi/mi-reg-undefined.S
new file mode 100644
index 0000000..d3d529a
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.S
@@ -0,0 +1,521 @@
+/* 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/>. */
+
+ /* The FDE entry for "stop_frame" in the .debug_frame section has
+ been hand modified to mark a set of registers as undefined.
+ Otherwise this file is as generated by gcc 4.7.2 for x86_64. */
+ .file "mi-reg-undefined.c"
+ .text
+.Ltext0:
+ .globl stop_frame
+ .type stop_frame, @function
+stop_frame:
+.LFB0:
+ .file 1 "mi-reg-undefined.c"
+ .loc 1 18 0
+ pushq %rbp
+.LCFI0:
+ movq %rsp, %rbp
+.LCFI1:
+ .loc 1 21 0
+ popq %rbp
+.LCFI2:
+ ret
+.LFE0:
+ .size stop_frame, .-stop_frame
+ .globl first_frame
+ .type first_frame, @function
+first_frame:
+.LFB1:
+ .loc 1 25 0
+ pushq %rbp
+.LCFI3:
+ movq %rsp, %rbp
+.LCFI4:
+ .loc 1 26 0
+ movl $0, %eax
+ call stop_frame
+ .loc 1 27 0
+ popq %rbp
+.LCFI5:
+ ret
+.LFE1:
+ .size first_frame, .-first_frame
+ .globl main
+ .type main, @function
+main:
+.LFB2:
+ .loc 1 31 0
+ pushq %rbp
+.LCFI6:
+ movq %rsp, %rbp
+.LCFI7:
+ .loc 1 32 0
+ movl $0, %eax
+ call first_frame
+ .loc 1 34 0
+ movl $0, %eax
+ .loc 1 35 0
+ popq %rbp
+.LCFI8:
+ ret
+.LFE2:
+ .size main, .-main
+ .section .debug_frame,"",@progbits
+.Lframe0:
+ .long .LECIE0-.LSCIE0
+.LSCIE0:
+ .long 0xffffffff
+ .byte 0x1
+ .string ""
+ .uleb128 0x1
+ .sleb128 -8
+ .byte 0x10
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .byte 0x90
+ .uleb128 0x1
+ .align 8
+.LECIE0:
+ /* This FDE entry, for stop_frame was modified to mark
+ registers 0 -> 6 as being undefined. */
+.LSFDE0:
+ .long .LEFDE0-.LASFDE0
+.LASFDE0:
+ .long .Lframe0
+ .quad .LFB0
+ .quad .LFE0-.LFB0
+
+ /* START OF NEW CONTENT. */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x0 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x1 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x2 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x3 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x4 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x5 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x6 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x7 /* ULEB128 register */
+ /* END OF NEW CONTENT. */
+
+ .byte 0x4
+ .long .LCFI0-.LFB0
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI1-.LCFI0
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI2-.LCFI1
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE0:
+.LSFDE2:
+ .long .LEFDE2-.LASFDE2
+.LASFDE2:
+ .long .Lframe0
+ .quad .LFB1
+ .quad .LFE1-.LFB1
+ .byte 0x4
+ .long .LCFI3-.LFB1
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI4-.LCFI3
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI5-.LCFI4
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE2:
+.LSFDE4:
+ .long .LEFDE4-.LASFDE4
+.LASFDE4:
+ .long .Lframe0
+ .quad .LFB2
+ .quad .LFE2-.LFB2
+ .byte 0x4
+ .long .LCFI6-.LFB2
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI7-.LCFI6
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI8-.LCFI7
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE4:
+ .section .eh_frame,"a",@progbits
+.Lframe1:
+ .long .LECIE1-.LSCIE1
+.LSCIE1:
+ .long 0
+ .byte 0x1
+ .string "zR"
+ .uleb128 0x1
+ .sleb128 -8
+ .byte 0x10
+ .uleb128 0x1
+ .byte 0x3
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .byte 0x90
+ .uleb128 0x1
+ .align 8
+.LECIE1:
+.LSFDE7:
+ .long .LEFDE7-.LASFDE7
+.LASFDE7:
+ .long .LASFDE7-.Lframe1
+ .long .LFB0
+ .long .LFE0-.LFB0
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI0-.LFB0
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI1-.LCFI0
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI2-.LCFI1
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE7:
+.LSFDE9:
+ .long .LEFDE9-.LASFDE9
+.LASFDE9:
+ .long .LASFDE9-.Lframe1
+ .long .LFB1
+ .long .LFE1-.LFB1
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI3-.LFB1
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI4-.LCFI3
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI5-.LCFI4
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE9:
+.LSFDE11:
+ .long .LEFDE11-.LASFDE11
+.LASFDE11:
+ .long .LASFDE11-.Lframe1
+ .long .LFB2
+ .long .LFE2-.LFB2
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI6-.LFB2
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI7-.LCFI6
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI8-.LCFI7
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE11:
+ .text
+.Letext0:
+ .section .debug_info,"",@progbits
+.Ldebug_info0:
+ .long 0x8c
+ .value 0x2
+ .long .Ldebug_abbrev0
+ .byte 0x8
+ .uleb128 0x1
+ .long .LASF2
+ .byte 0x1
+ .long .LASF3
+ .long .LASF4
+ .quad .Ltext0
+ .quad .Letext0
+ .long .Ldebug_line0
+ .uleb128 0x2
+ .byte 0x1
+ .long .LASF0
+ .byte 0x1
+ .byte 0x11
+ .quad .LFB0
+ .quad .LFE0
+ .long .LLST0
+ .byte 0x1
+ .uleb128 0x3
+ .byte 0x1
+ .long .LASF1
+ .byte 0x1
+ .byte 0x18
+ .quad .LFB1
+ .quad .LFE1
+ .long .LLST1
+ .byte 0x1
+ .uleb128 0x4
+ .byte 0x1
+ .long .LASF5
+ .byte 0x1
+ .byte 0x1e
+ .long 0x88
+ .quad .LFB2
+ .quad .LFE2
+ .long .LLST2
+ .byte 0x1
+ .uleb128 0x5
+ .byte 0x4
+ .byte 0x5
+ .string "int"
+ .byte 0
+ .section .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+ .uleb128 0x1
+ .uleb128 0x11
+ .byte 0x1
+ .uleb128 0x25
+ .uleb128 0xe
+ .uleb128 0x13
+ .uleb128 0xb
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x1b
+ .uleb128 0xe
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x10
+ .uleb128 0x6
+ .byte 0
+ .byte 0
+ .uleb128 0x2
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2117
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x3
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2116
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x4
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x49
+ .uleb128 0x13
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2116
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x5
+ .uleb128 0x24
+ .byte 0
+ .uleb128 0xb
+ .uleb128 0xb
+ .uleb128 0x3e
+ .uleb128 0xb
+ .uleb128 0x3
+ .uleb128 0x8
+ .byte 0
+ .byte 0
+ .byte 0
+ .section .debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+ .quad .LFB0-.Ltext0
+ .quad .LCFI0-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI0-.Ltext0
+ .quad .LCFI1-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI1-.Ltext0
+ .quad .LCFI2-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI2-.Ltext0
+ .quad .LFE0-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+.LLST1:
+ .quad .LFB1-.Ltext0
+ .quad .LCFI3-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI3-.Ltext0
+ .quad .LCFI4-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI4-.Ltext0
+ .quad .LCFI5-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI5-.Ltext0
+ .quad .LFE1-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+.LLST2:
+ .quad .LFB2-.Ltext0
+ .quad .LCFI6-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI6-.Ltext0
+ .quad .LCFI7-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI7-.Ltext0
+ .quad .LCFI8-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI8-.Ltext0
+ .quad .LFE2-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+ .section .debug_aranges,"",@progbits
+ .long 0x2c
+ .value 0x2
+ .long .Ldebug_info0
+ .byte 0x8
+ .byte 0
+ .value 0
+ .value 0
+ .quad .Ltext0
+ .quad .Letext0-.Ltext0
+ .quad 0
+ .quad 0
+ .section .debug_line,"",@progbits
+.Ldebug_line0:
+ .section .debug_str,"MS",@progbits,1
+.LASF0:
+ .string "stop_frame"
+.LASF2:
+ .string "GNU C 4.7.2"
+.LASF3:
+ .string "mi-reg-undefined.c"
+.LASF4:
+ .string "/home/username/src/gdb/testsuite/gdb.mi"
+.LASF1:
+ .string "first_frame"
+.LASF5:
+ .string "main"
+ .ident "GCC: (GNU) 4.7.2"
+ .section .note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.c b/gdb/testsuite/gdb.mi/mi-reg-undefined.c
new file mode 100644
index 0000000..0ba2f0a
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.c
@@ -0,0 +1,35 @@
+/* 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/>. */
+
+void
+stop_frame ()
+{
+ /* The debug information for this frame is modified in the accompanying
+ .S file, to mark a set of registers as being undefined. */
+}
+
+void
+first_frame ()
+{
+ stop_frame ();
+}
+
+int
+main ()
+{
+ first_frame ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
new file mode 100644
index 0000000..8bcbb21
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -0,0 +1,71 @@
+# 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 dwarf.exp
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# This test can only be run on x86_64 targets.
+if {![istarget "x86_64-*-*"] || ![is_lp64_target]} {
+ return 0
+}
+
+gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+standard_testfile .S
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested mi-reg-undefined.exp
+ return -1
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+if ![mi_runto stop_frame] {
+ perror "Failed to stop in stop_frame"
+ return -1
+}
+
+mi_gdb_test "111-stack-list-frames" \
+ "111\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"stop_frame\",.*\},frame=\{level=\"1\",addr=\"$hex\",func=\"first_frame\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
+ "stack frame listing"
+
+set opt_out_pattern "<optimized out>"
+
+for {set f 0} {$f < 3} {incr f} {
+ if {${f} == 0} {
+ set pattern_0_1_2 ${hex}
+ } else {
+ set pattern_0_1_2 ${opt_out_pattern}
+ }
+
+ mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
+ "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "register values, format x, frame ${f}"
+
+ mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
+ "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "register values, format r, frame ${f}"
+}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] optimized out registers in mi
2013-07-05 13:28 [PATCH] optimized out registers in mi Andrew Burgess
@ 2013-07-05 14:03 ` Pedro Alves
2013-07-05 16:38 ` Andrew Burgess
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2013-07-05 14:03 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/05/2013 02:27 PM, Andrew Burgess wrote:
> [1] I applied a patch yesterday that changed value_optimized_out,
> before I applied yesterdays patch the behaviour of -data-list-registers
> was a bit random, if the register value was lazy then you would get
> the "<optimized out>" result, if the value was not lazy then you got
> the error. Of the two behaviours I think returning the
> "<optimized out>" string is by far the most helpful so that's
> the behaviour we now get in all cases.
Agreed.
It'd be nice to handle partially optimized out registers
too, though I'm not sure gcc does something like that
currently. And that can be always handled later. This
is already a good step.
> gdb/ChangeLog
> - if (format == 'N')
> + /* Displaying optimized out values using anything other than the default
> + format will result in the value 0 being shown. */
Please make that s/will/would/. It's confusing as is; it made
me think that printing 0 was what the patch was meaning to do.
But, this looks hacky to me. Do we print 0 with the CLI as well?
If not, what's handling it that's missing in MI? Isn't this being
handled gracefully somewhere within val_print itself (I'd
think in val_print_scalar_formatted)?
--
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] optimized out registers in mi
2013-07-05 14:03 ` Pedro Alves
@ 2013-07-05 16:38 ` Andrew Burgess
2013-07-05 18:17 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Burgess @ 2013-07-05 16:38 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 05/07/2013 3:03 PM, Pedro Alves wrote:
> On 07/05/2013 02:27 PM, Andrew Burgess wrote:
>
>> [1] I applied a patch yesterday that changed value_optimized_out,
>> before I applied yesterdays patch the behaviour of -data-list-registers
>> was a bit random, if the register value was lazy then you would get
>> the "<optimized out>" result, if the value was not lazy then you got
>> the error. Of the two behaviours I think returning the
>> "<optimized out>" string is by far the most helpful so that's
>> the behaviour we now get in all cases.
>
> Agreed.
>
> It'd be nice to handle partially optimized out registers
> too, though I'm not sure gcc does something like that
> currently. And that can be always handled later. This
> is already a good step.
>
>> gdb/ChangeLog
>
>> - if (format == 'N')
>> + /* Displaying optimized out values using anything other than the default
>> + format will result in the value 0 being shown. */
>
> Please make that s/will/would/. It's confusing as is; it made
> me think that printing 0 was what the patch was meaning to do.
>
> But, this looks hacky to me. Do we print 0 with the CLI as well?
> If not, what's handling it that's missing in MI? Isn't this being
> handled gracefully somewhere within val_print itself (I'd
> think in val_print_scalar_formatted)?
The answer to this is a few lines down in the function, it is really the
'r' formatting case that I'm trying to avoid, as this is handled as a
special within the output_register function.
So, I could probably write:
if ((value_optimized_out (val) && format == 'r') || format == 'N')
NOTE: The format == 'N' stuff is not mine, and was there before.
I didn't write this as I figured the result would be the same whatever
format you send into the print code if the value is optimized_out.
Now, the next question that comes to me is, why do we handle 'r' as a
special case inside output_register.... I can't answer that one, maybe
we could / should move it into the generic print code, then you'd be
correct and the whole optimized_out check would not be needed. Would
your prefer me to try to move the format-'r' code, or is my original
patch ok?
Cheers,
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] optimized out registers in mi
2013-07-05 16:38 ` Andrew Burgess
@ 2013-07-05 18:17 ` Pedro Alves
0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2013-07-05 18:17 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/05/2013 05:37 PM, Andrew Burgess wrote:
> On 05/07/2013 3:03 PM, Pedro Alves wrote:
>> On 07/05/2013 02:27 PM, Andrew Burgess wrote:
>>
>>> [1] I applied a patch yesterday that changed value_optimized_out,
>>> before I applied yesterdays patch the behaviour of -data-list-registers
>>> was a bit random, if the register value was lazy then you would get
>>> the "<optimized out>" result, if the value was not lazy then you got
>>> the error. Of the two behaviours I think returning the
>>> "<optimized out>" string is by far the most helpful so that's
>>> the behaviour we now get in all cases.
>>
>> Agreed.
>>
>> It'd be nice to handle partially optimized out registers
>> too, though I'm not sure gcc does something like that
>> currently. And that can be always handled later. This
>> is already a good step.
>>
>>> gdb/ChangeLog
>>
>>> - if (format == 'N')
>>> + /* Displaying optimized out values using anything other than the default
>>> + format will result in the value 0 being shown. */
>>
>> Please make that s/will/would/. It's confusing as is; it made
>> me think that printing 0 was what the patch was meaning to do.
>>
>> But, this looks hacky to me. Do we print 0 with the CLI as well?
>> If not, what's handling it that's missing in MI? Isn't this being
>> handled gracefully somewhere within val_print itself (I'd
>> think in val_print_scalar_formatted)?
>
> The answer to this is a few lines down in the function, it is really the
> 'r' formatting case that I'm trying to avoid, as this is handled as a
> special within the output_register function.
Alright, then the comment was wrong.
>
> So, I could probably write:
>
> if ((value_optimized_out (val) && format == 'r') || format == 'N')
That'd already be much better.
>
> NOTE: The format == 'N' stuff is not mine, and was there before.
>
> I didn't write this as I figured the result would be the same whatever
> format you send into the print code if the value is optimized_out.
>
> Now, the next question that comes to me is, why do we handle 'r' as a
> special case inside output_register.... I can't answer that one, maybe
> we could / should move it into the generic print code, then you'd be
> correct and the whole optimized_out check would not be needed. Would
> your prefer me to try to move the format-'r' code, or is my original
> patch ok?
Well, I'm still correct. ;-) It's quite likely/possible that at
some point e.g., the t/Binary format will print the available
bits of partially optimized out registers (e.g., only-half-registers
pushed to stack). Better not frob the format the frontend requested.
Or the default for vector optimized out values could e.g.,
print half valid vector, half optimized vector, in natural format
(while we want raw).
I can't answer why we special case 'r'. If we don't push this
down to val_print, I'd prefer:
if (format == 'r')
{
if (value_optimized_out (val))
{
stb = mem_fileopen ();
old_chain = make_cleanup_ui_file_delete (stb);
val_print_optimized_out (stb);
ui_out_field_stream (uiout, "value", stb);
do_cleanups (old_chain);
}
else
{
// existing R code.
}
}
else
{
val_print
}
Actually, the 'r'/raw printing bits could be factored out
to a separate function and used by default_print_one_register_info
as well (the similar loop prints the register raw). Which BTW,
doesn't seem to be handling optimized out registers either?
(or rather, I couldn't find where it's handled, just by 2min
looking around.)
--
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-05 18:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-05 13:28 [PATCH] optimized out registers in mi Andrew Burgess
2013-07-05 14:03 ` Pedro Alves
2013-07-05 16:38 ` Andrew Burgess
2013-07-05 18:17 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox