[PING] Re: [PATCH] [1/2] value_fetch_lazy - ensure parent is not lazy before accessing.

["Andrew Burgess" <aburgess at broadcom dot com>]

Any feedback on the below?

Thanks,
Andrew

On 11/07/2013 3:34 PM, Andrew Burgess wrote:
> Hi,
> 
> Within value_fetch_lazy, for bit field values we
> make use of the parent value without first checking
> that the parent is not lazy.
> 
> Ideally we'd only fetch that portion of the parent value
> that was required to get the contents of the sub-field
> value, but I don't believe this is currently supported,
> so I offer the patch below.
> 
> You might think that the expected test results are a 
> little strange, in some cases we report <optimized out>,
> and in other cases we throw an error, patch [2/2] to
> follow shortly.
> 
> OK to apply?
> 
> 
> Thanks,
> Andrew
> 
> gdb/ChangeLog
> 
> 2013-07-11  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* value.c (value_fetch_lazy): Ensure parent value is not lazy
> 	before checking which bits of the parent are valid.
> 
> gdb/testsuite/ChangeLog
> 
> 2013-07-11  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* gdb.dwarf2/pieces-optimized-out.exp: New file.
> 	* gdb.dwarf2/pieces-optimized-out.c: New file.
> 	* gdb.dwarf2/pieces-optimized-out.S: New file.
> 
> diff --git a/gdb/value.c b/gdb/value.c
> index 04f325f..8e0f8c4 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -3433,7 +3433,10 @@ value_fetch_lazy (struct value *val)
>        LONGEST offset = value_offset (val);
>        LONGEST num;
>  
> -      if (!value_bits_valid (val,
> +      if (value_lazy (parent))
> +	value_fetch_lazy (parent);
> +
> +      if (!value_bits_valid (parent,
>  			     TARGET_CHAR_BIT * offset + value_bitpos (val),
>  			     value_bitsize (val)))
>  	error (_("value has been optimized out"));
> diff --git a/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.S b/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.S
> new file mode 100644
> index 0000000..06aa707
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.S
> @@ -0,0 +1,548 @@
> +/*
> +   Copyright 2010-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/>.  */
> +
> +/* This was compiled with gcc version 4.7.2 using:
> +        gcc -O3 -m32 -g -o pieces-optimized-out.S -S pieces-optimized-out.c
> +   The contents of this file are otherwise unmodified.  */
> +
> +        .file	"pieces-optimized-out.c"
> +	.text
> +.Ltext0:
> +	.p2align 4,,15
> +	.globl	foo
> +	.type	foo, @function
> +foo:
> +.LFB0:
> +	.file 1 "pieces-optimized-out.c"
> +	.loc 1 33 0
> +.LVL0:
> +	.loc 1 35 0
> +	movl	4(%esp), %eax
> +	ret
> +.LFE0:
> +	.size	foo, .-foo
> +	.section	.text.startup,"ax",@progbits
> +	.p2align 4,,15
> +	.globl	main
> +	.type	main, @function
> +main:
> +.LFB1:
> +	.loc 1 39 0
> +.LVL1:
> +	subl	$4, %esp
> +.LCFI0:
> +	.loc 1 44 0
> +	movl	$10, (%esp)
> +	call	foo
> +.LVL2:
> +	.loc 1 46 0
> +	addl	$4, %esp
> +.LCFI1:
> +	.loc 1 45 0
> +	addl	$20, %eax
> +.LVL3:
> +	.loc 1 46 0
> +	ret
> +.LFE1:
> +	.size	main, .-main
> +	.section	.debug_frame,"",@progbits
> +.Lframe0:
> +	.long	.LECIE0-.LSCIE0
> +.LSCIE0:
> +	.long	0xffffffff
> +	.byte	0x1
> +	.string	""
> +	.uleb128 0x1
> +	.sleb128 -4
> +	.byte	0x8
> +	.byte	0xc
> +	.uleb128 0x4
> +	.uleb128 0x4
> +	.byte	0x88
> +	.uleb128 0x1
> +	.align 4
> +.LECIE0:
> +.LSFDE0:
> +	.long	.LEFDE0-.LASFDE0
> +.LASFDE0:
> +	.long	.Lframe0
> +	.long	.LFB0
> +	.long	.LFE0-.LFB0
> +	.align 4
> +.LEFDE0:
> +.LSFDE2:
> +	.long	.LEFDE2-.LASFDE2
> +.LASFDE2:
> +	.long	.Lframe0
> +	.long	.LFB1
> +	.long	.LFE1-.LFB1
> +	.byte	0x4
> +	.long	.LCFI0-.LFB1
> +	.byte	0xe
> +	.uleb128 0x8
> +	.byte	0x4
> +	.long	.LCFI1-.LCFI0
> +	.byte	0xe
> +	.uleb128 0x4
> +	.align 4
> +.LEFDE2:
> +	.section	.eh_frame,"a",@progbits
> +.Lframe1:
> +	.long	.LECIE1-.LSCIE1
> +.LSCIE1:
> +	.long	0
> +	.byte	0x1
> +	.string	""
> +	.uleb128 0x1
> +	.sleb128 -4
> +	.byte	0x8
> +	.byte	0xc
> +	.uleb128 0x4
> +	.uleb128 0x4
> +	.byte	0x88
> +	.uleb128 0x1
> +	.align 4
> +.LECIE1:
> +.LSFDE5:
> +	.long	.LEFDE5-.LASFDE5
> +.LASFDE5:
> +	.long	.LASFDE5-.Lframe1
> +	.long	.LFB0
> +	.long	.LFE0-.LFB0
> +	.align 4
> +.LEFDE5:
> +.LSFDE7:
> +	.long	.LEFDE7-.LASFDE7
> +.LASFDE7:
> +	.long	.LASFDE7-.Lframe1
> +	.long	.LFB1
> +	.long	.LFE1-.LFB1
> +	.byte	0x4
> +	.long	.LCFI0-.LFB1
> +	.byte	0xe
> +	.uleb128 0x8
> +	.byte	0x4
> +	.long	.LCFI1-.LCFI0
> +	.byte	0xe
> +	.uleb128 0x4
> +	.align 4
> +.LEFDE7:
> +	.text
> +.Letext0:
> +	.section	.debug_info,"",@progbits
> +.Ldebug_info0:
> +	.long	0xe4
> +	.value	0x2
> +	.long	.Ldebug_abbrev0
> +	.byte	0x4
> +	.uleb128 0x1
> +	.long	.LASF0
> +	.byte	0x1
> +	.long	.LASF1
> +	.long	.LASF2
> +	.long	.Ldebug_ranges0+0
> +	.long	0
> +	.long	0
> +	.long	.Ldebug_line0
> +	.uleb128 0x2
> +	.string	"str"
> +	.byte	0xc
> +	.byte	0x1
> +	.byte	0x17
> +	.long	0x6c
> +	.uleb128 0x3
> +	.string	"a"
> +	.byte	0x1
> +	.byte	0x19
> +	.long	0x6c
> +	.byte	0x2
> +	.byte	0x23
> +	.uleb128 0
> +	.uleb128 0x3
> +	.string	"b"
> +	.byte	0x1
> +	.byte	0x1a
> +	.long	0x6c
> +	.byte	0x2
> +	.byte	0x23
> +	.uleb128 0x4
> +	.uleb128 0x4
> +	.string	"c"
> +	.byte	0x1
> +	.byte	0x1b
> +	.long	0x6c
> +	.byte	0x4
> +	.byte	0x3
> +	.byte	0x1d
> +	.byte	0x2
> +	.byte	0x23
> +	.uleb128 0x8
> +	.uleb128 0x4
> +	.string	"d"
> +	.byte	0x1
> +	.byte	0x1c
> +	.long	0x6c
> +	.byte	0x4
> +	.byte	0x3
> +	.byte	0x1a
> +	.byte	0x2
> +	.byte	0x23
> +	.uleb128 0x8
> +	.byte	0
> +	.uleb128 0x5
> +	.byte	0x4
> +	.byte	0x5
> +	.string	"int"
> +	.uleb128 0x6
> +	.byte	0x1
> +	.string	"foo"
> +	.byte	0x1
> +	.byte	0x20
> +	.byte	0x1
> +	.long	0x6c
> +	.long	.LFB0
> +	.long	.LFE0
> +	.byte	0x2
> +	.byte	0x74
> +	.sleb128 4
> +	.byte	0x1
> +	.long	0x9f
> +	.uleb128 0x7
> +	.string	"arg"
> +	.byte	0x1
> +	.byte	0x20
> +	.long	0x6c
> +	.byte	0x2
> +	.byte	0x91
> +	.sleb128 0
> +	.byte	0
> +	.uleb128 0x8
> +	.byte	0x1
> +	.long	.LASF3
> +	.byte	0x1
> +	.byte	0x26
> +	.byte	0x1
> +	.long	0x6c
> +	.long	.LFB1
> +	.long	.LFE1
> +	.long	.LLST0
> +	.byte	0x1
> +	.uleb128 0x9
> +	.string	"s"
> +	.byte	0x1
> +	.byte	0x28
> +	.long	0x29
> +	.byte	0x6
> +	.byte	0x35
> +	.byte	0x9f
> +	.byte	0x93
> +	.uleb128 0x4
> +	.byte	0x93
> +	.uleb128 0x8
> +	.uleb128 0xa
> +	.string	"v"
> +	.byte	0x1
> +	.byte	0x29
> +	.long	0x6c
> +	.long	.LLST1
> +	.uleb128 0xb
> +	.long	.LVL2
> +	.long	0x73
> +	.uleb128 0xc
> +	.byte	0x2
> +	.byte	0x74
> +	.sleb128 0
> +	.byte	0x1
> +	.byte	0x3a
> +	.byte	0
> +	.byte	0
> +	.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 0x55
> +	.uleb128 0x6
> +	.uleb128 0x11
> +	.uleb128 0x1
> +	.uleb128 0x52
> +	.uleb128 0x1
> +	.uleb128 0x10
> +	.uleb128 0x6
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x2
> +	.uleb128 0x13
> +	.byte	0x1
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0xb
> +	.uleb128 0xb
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x1
> +	.uleb128 0x13
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x3
> +	.uleb128 0xd
> +	.byte	0
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0x38
> +	.uleb128 0xa
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x4
> +	.uleb128 0xd
> +	.byte	0
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0xb
> +	.uleb128 0xb
> +	.uleb128 0xd
> +	.uleb128 0xb
> +	.uleb128 0xc
> +	.uleb128 0xb
> +	.uleb128 0x38
> +	.uleb128 0xa
> +	.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
> +	.uleb128 0x6
> +	.uleb128 0x2e
> +	.byte	0x1
> +	.uleb128 0x3f
> +	.uleb128 0xc
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x27
> +	.uleb128 0xc
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0x11
> +	.uleb128 0x1
> +	.uleb128 0x12
> +	.uleb128 0x1
> +	.uleb128 0x40
> +	.uleb128 0xa
> +	.uleb128 0x2117
> +	.uleb128 0xc
> +	.uleb128 0x1
> +	.uleb128 0x13
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x7
> +	.uleb128 0x5
> +	.byte	0
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0x2
> +	.uleb128 0xa
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x8
> +	.uleb128 0x2e
> +	.byte	0x1
> +	.uleb128 0x3f
> +	.uleb128 0xc
> +	.uleb128 0x3
> +	.uleb128 0xe
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x27
> +	.uleb128 0xc
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0x11
> +	.uleb128 0x1
> +	.uleb128 0x12
> +	.uleb128 0x1
> +	.uleb128 0x40
> +	.uleb128 0x6
> +	.uleb128 0x2117
> +	.uleb128 0xc
> +	.byte	0
> +	.byte	0
> +	.uleb128 0x9
> +	.uleb128 0x34
> +	.byte	0
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0x2
> +	.uleb128 0xa
> +	.byte	0
> +	.byte	0
> +	.uleb128 0xa
> +	.uleb128 0x34
> +	.byte	0
> +	.uleb128 0x3
> +	.uleb128 0x8
> +	.uleb128 0x3a
> +	.uleb128 0xb
> +	.uleb128 0x3b
> +	.uleb128 0xb
> +	.uleb128 0x49
> +	.uleb128 0x13
> +	.uleb128 0x2
> +	.uleb128 0x6
> +	.byte	0
> +	.byte	0
> +	.uleb128 0xb
> +	.uleb128 0x4109
> +	.byte	0x1
> +	.uleb128 0x11
> +	.uleb128 0x1
> +	.uleb128 0x31
> +	.uleb128 0x13
> +	.byte	0
> +	.byte	0
> +	.uleb128 0xc
> +	.uleb128 0x410a
> +	.byte	0
> +	.uleb128 0x2
> +	.uleb128 0xa
> +	.uleb128 0x2111
> +	.uleb128 0xa
> +	.byte	0
> +	.byte	0
> +	.byte	0
> +	.section	.debug_loc,"",@progbits
> +.Ldebug_loc0:
> +.LLST0:
> +	.long	.LFB1
> +	.long	.LCFI0
> +	.value	0x2
> +	.byte	0x74
> +	.sleb128 4
> +	.long	.LCFI0
> +	.long	.LCFI1
> +	.value	0x2
> +	.byte	0x74
> +	.sleb128 8
> +	.long	.LCFI1
> +	.long	.LFE1
> +	.value	0x2
> +	.byte	0x74
> +	.sleb128 4
> +	.long	0
> +	.long	0
> +.LLST1:
> +	.long	.LVL1
> +	.long	.LVL2
> +	.value	0x2
> +	.byte	0x3a
> +	.byte	0x9f
> +	.long	.LVL2
> +	.long	.LVL3
> +	.value	0x3
> +	.byte	0x70
> +	.sleb128 10
> +	.byte	0x9f
> +	.long	.LVL3
> +	.long	.LFE1
> +	.value	0x3
> +	.byte	0x70
> +	.sleb128 -10
> +	.byte	0x9f
> +	.long	0
> +	.long	0
> +	.section	.debug_aranges,"",@progbits
> +	.long	0x24
> +	.value	0x2
> +	.long	.Ldebug_info0
> +	.byte	0x4
> +	.byte	0
> +	.value	0
> +	.value	0
> +	.long	.Ltext0
> +	.long	.Letext0-.Ltext0
> +	.long	.LFB1
> +	.long	.LFE1-.LFB1
> +	.long	0
> +	.long	0
> +	.section	.debug_ranges,"",@progbits
> +.Ldebug_ranges0:
> +	.long	.Ltext0
> +	.long	.Letext0
> +	.long	.LFB1
> +	.long	.LFE1
> +	.long	0
> +	.long	0
> +	.section	.debug_line,"",@progbits
> +.Ldebug_line0:
> +	.section	.debug_str,"MS",@progbits,1
> +.LASF2:
> +	.string	"/home/username/"
> +.LASF1:
> +	.string	"pieces-optimized-out.c"
> +.LASF0:
> +	.string	"GNU C 4.7.2"
> +.LASF3:
> +	.string	"main"
> +	.ident	"GCC: (GNU) 4.7.2"
> +	.section	.note.GNU-stack,"",@progbits
> diff --git a/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.c b/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.c
> new file mode 100644
> index 0000000..2bf9cf8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.c
> @@ -0,0 +1,47 @@
> +/* Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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/>.  */
> +
> +/* This file is not actually compiled, the .S file is committed alongside
> +   this file.  The reason is that changes to the compiler might result
> +   in different debug information being created, this could break the
> +   test.  */
> +
> +struct str
> +{
> +  int a;
> +  int b;
> +  int c : 3;
> +  int d : 3;
> +};
> +
> +int __attribute__ ((noinline))
> +foo (int arg)
> +{  
> +  return arg;
> +}
> +
> +int
> +main ( void )
> +{
> +  struct str s = {5, 7, 1, 2};
> +  int v;
> +
> +  v = (s.a << 1);
> +  v += foo (v);
> +  return v + 5 + s.a;
> +}
> +
> diff --git a/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.exp b/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.exp
> new file mode 100644
> index 0000000..2e4d028
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/pieces-optimized-out.exp
> @@ -0,0 +1,49 @@
> +# 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
> +# Test some DWARF piece operators.
> +
> +# 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 targets.
> +if {![is_x86_like_target]} {
> +    return 0  
> +}
> +
> +standard_testfile .S
> +set csrcfile ${testfile}.c
> +
> +if {[prepare_for_testing ${testfile}.exp ${testfile} $srcfile]} {
> +    return -1
> +}
> +
> +if ![runto foo] {
> +    return -1
> +}
> +
> +# Move back up to main.
> +gdb_test "up" "#1  $hex in main.*" "move up a frame"
> +
> +# Now take a look at the 's' structure.
> +gdb_test "p s" \
> +    " = {a = 5, b = <optimized out>, c = <optimized out>, d = <optimized out>}" \
> +    "print s"
> +gdb_test "p s.a" " = 5" "print s.a"
> +gdb_test "p s.b" " = <optimized out>" "print s.b"
> +gdb_test "p s.c" "value has been optimized out" "print s.c"
> +gdb_test "p s.d" "value has been optimized out" "print s.d"
> +
> 
> 
> 
>