[patch] Fix internal error on optimized-out values (regression by me)

[patch] Fix internal error on optimized-out values (regression by me)

[Jan Kratochvil]

Hi,

since:
	Re: [patch] Code cleanup: Introduce allocate_optimized_out_value
	http://sourceware.org/ml/gdb-patches/2011-07/msg00327.html
	acfe85f56075910a3ba5e8b76189e0770079b8d1
can occur:
	(gdb) p p->f
	valops.c:1118: internal-error: Unexpected lazy value type.
	A problem internal to GDB has been detected,

in that mail referenced above:
# It is true it would be definitely a bug in a code incompatible with such
# change so one may rather fix that possible regression there.

so this testcase exploits such case.  The problem is formerly the code
allocated (in some cases) optimized out values as non-lazy ones.  Now they are
allocated all as lazy (*) which breaks some code not expecting lazy values.

(*) Such lazy optimized out value still gets silently allocate_value_contents
    by value_fetch_lazy, allocate_value_contents should be suppressed in such
    case; such more radical change has never been made.

Formerly (incl. gdb-7.3) did:
	(gdb) p p->f
	$1 = 0
which was also wrong, ((struct *) <optimized out>)->field should be IMO still
<optimized out>; just it became internal-error now.

GDB usually does require_not_optimized_out throwing error() when it meets any
first <optimized out> value.  I have just produced new optimized value
instead; require_not_optimized_out is not exported and I find it better this
way.

Following the rule that it is better if GDB does something wrong rather then
it throws internal-error to make the patch from July above fully safe one
would have to change allocate_optimized_out_value to allocate everything as
non-lazy.  I find that as a too ugly workaround of the codebase.

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.


Andre, do you still see the bug even with this patch?


Thanks,
Jan


gdb/
2011-09-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix internal error regression.
	* value.c (value_primitive_field): Handle value_optimized_out.  Move
	packed bitfields comment.

gdb/testsuite/
2011-09-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix internal error regression.
	* gdb.dwarf2/implptr-optimized-out.S: New file.
	* gdb.dwarf2/implptr-optimized-out.exp: New file.

--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2482,16 +2482,19 @@ value_primitive_field (struct value *arg1, int offset,
      description correctly.  */
   check_typedef (type);
 
-  /* Handle packed fields */
-
-  if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
+  if (value_optimized_out (arg1))
+    v = allocate_optimized_out_value (type);
+  else if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
     {
-      /* Create a new value for the bitfield, with bitpos and bitsize
+      /* Handle packed fields.
+
+	 Create a new value for the bitfield, with bitpos and bitsize
 	 set.  If possible, arrange offset and bitpos so that we can
 	 do a single aligned read of the size of the containing type.
 	 Otherwise, adjust offset to the byte containing the first
 	 bit.  Assume that the address, offset, and embedded offset
 	 are sufficiently aligned.  */
+
       int bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
       int container_bitsize = TYPE_LENGTH (type) * 8;
 
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/implptr-optimized-out.S
@@ -0,0 +1,166 @@
+/* Copyright 2010, 2011 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/>.  */
+
+	.section	.debug_info
+d:
+	.long	debug_end - 1f	/* Length of Compilation Unit Info */
+1:
+	.2byte	0x3	/* DWARF version number */
+	.long	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+	.ascii "GNU C 4.4.3\0"	/* DW_AT_producer */
+	.byte	0x1	/* DW_AT_language */
+	.ascii "1.c\0"	/* DW_AT_name */
+
+.Ltype_int:
+	.uleb128 0x7	/* DW_TAG_base_type */
+	.byte	0x4	/* DW_AT_byte_size */
+	.byte	0x5	/* DW_AT_encoding */
+	.ascii "int\0"	/* DW_AT_name */
+
+.Ltype_struct:
+	.uleb128 0x2	/* DW_TAG_structure_type */
+	.ascii "s\0"	/* DW_AT_name */
+	.byte	4	/* DW_AT_byte_size */
+
+	.uleb128 0x3	/* DW_TAG_member */
+	.ascii "f\0"	/* DW_AT_name */
+	.4byte	.Ltype_int - d	/* DW_AT_type */
+	.byte	0	/* DW_AT_data_member_location */
+
+	.byte	0x0	/* end of children of DW_TAG_structure_type */
+
+	.uleb128	6			/* Abbrev: DW_TAG_subprogram */
+	.ascii		"main\0"		/* DW_AT_name */
+	.4byte		main			/* DW_AT_low_pc */
+	.4byte		main + 0x100		/* DW_AT_high_pc */
+	.4byte		.Ltype_int - d		/* DW_AT_type */
+	.byte		1			/* DW_AT_external */
+
+.Ltype_structptr:
+	.uleb128 0x5	/* DW_TAG_pointer_type */
+	.byte	0x4	/* DW_AT_byte_size */
+	.long	.Ltype_struct - d	/* DW_AT_type */
+
+.Lvar_out:
+	.uleb128 0x4	/* (DW_TAG_variable) */
+	.ascii "v\0"	/* DW_AT_name */
+	.byte	0	/* DW_AT_location: DW_FORM_block1 */
+	.4byte	.Ltype_struct - d	/* DW_AT_type */
+
+	.uleb128 0x4	/* (DW_TAG_variable) */
+	.ascii "p\0"	/* DW_AT_name */
+	.byte	2f - 1f	/* DW_AT_location: DW_FORM_block1 */
+1:
+	.byte	0xf2	/* DW_OP_GNU_implicit_pointer */
+	.4byte	.Lvar_out - d	/* referenced DIE */
+	.sleb128	0	/* offset */
+2:
+	.4byte	.Ltype_structptr - d	/* DW_AT_type */
+
+	.byte	0x0	/* end of children of main */
+
+	.byte	0x0	/* end of children of CU */
+debug_end:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x13	/* (TAG: DW_TAG_structure_type) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.byte	0
+	.byte	0
+
+	.uleb128 0x3	/* (abbrev code) */
+	.uleb128 0xd	/* (TAG: DW_TAG_member) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x49	/* (DW_AT_type) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x38	/* (DW_AT_data_member_location) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.byte	0
+	.byte	0
+
+	.uleb128 0x4	/* (abbrev code) */
+	.uleb128 0x34	/* (TAG: DW_TAG_variable) */
+	.byte	0x0	/* DW_children_yes */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x02	/* (DW_AT_location) */
+	.uleb128 0xa	/* (DW_FORM_block1) */
+	.uleb128 0x49	/* (DW_AT_type) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128 0x5	/* (abbrev code) */
+	.uleb128 0xf	/* (TAG: DW_TAG_pointer_type) */
+	.byte	0x0	/* DW_children_no */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x49	/* (DW_AT_type) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128	6			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128 0x7	/* (abbrev code) */
+	.uleb128 0x24	/* (TAG: DW_TAG_base_type) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3e	/* (DW_AT_encoding) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0
+	.byte	0
+
+	.byte	0x0
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/implptr-optimized-out.exp
@@ -0,0 +1,37 @@
+# Copyright 2011 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
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+set testfile "implptr-optimized-out"
+set srcfile ${testfile}.S
+set mainfile main.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+if [prepare_for_testing ${testfile}.exp $executable "${srcfile} ${mainfile}" {}] {
+    return -1
+}
+
+# DW_OP_GNU_implicit_pointer implementation requires a valid frame.
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "p p->f" " = <optimized out>"

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Jan Kratochvil]

On Mon, 26 Sep 2011 21:11:32 +0200, Jan Kratochvil wrote:
> Formerly (incl. gdb-7.3) did:
> 	(gdb) p p->f
> 	$1 = 0
> which was also wrong, ((struct *) <optimized out>)->field should be IMO still
> <optimized out>; just it became internal-error now.

Mail text fix up:

((struct) <optimized out>).field should be IMO still <optimized out>; just it
became internal-error now.

For ((struct *) <optimized out>)->field GDB still correctly throws by
require_not_optimized_out error (_("value has been optimized out"));
It would be probably incorrect to dereference <optimized out> pointer into
anything.



Regards,
Jan

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> ((struct) <optimized out>).field should be IMO still <optimized
Jan> out>; just it became internal-error now.

Maybe I am misunderstanding what you are saying -- I think this should
also throw.  I think the rule should be that any attempt to access any
"invalid" contents of a value, for purposes of computation, should throw
an exception.  I say "purposes of computation" because printing must
explicitly be excluded here.  I think this rule applies to both forms of
"invalid" -- both optimized-out and unavailable.

Tom

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Jan Kratochvil]

On Mon, 03 Oct 2011 21:34:16 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> ((struct) <optimized out>).field should be IMO still <optimized
> Jan> out>; just it became internal-error now.
> 
> Maybe I am misunderstanding what you are saying -- I think this should
> also throw.  I think the rule should be that any attempt to access any
> "invalid" contents of a value, for purposes of computation, should throw
> an exception.

Here is problematic the term "for purposes of computation".

I agree that any computation with content of <optimized out> must throw.

But here the content is not interpreted in any way.  Only a smaller subset of
it is used.


But I do not have any argument why the former <optimized out> value is better
than this "value has been optimized out", unaware if there is a precedent for
either way in current codebase.  Changed it as you suggest.

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.


Thanks,
Jan


gdb/
2011-10-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix internal error regression.
	* value.c (value_primitive_field): Add require_not_optimized_out.

gdb/testsuite/
2011-10-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix internal error regression.
	* gdb.dwarf2/implptr-optimized-out.S: New file.
	* gdb.dwarf2/implptr-optimized-out.exp: New file.

--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2477,6 +2477,8 @@ value_primitive_field (struct value *arg1, int offset,
   struct value *v;
   struct type *type;
 
+  require_not_optimized_out (arg1);
+
   CHECK_TYPEDEF (arg_type);
   type = TYPE_FIELD_TYPE (arg_type, fieldno);
 
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/implptr-optimized-out.S
@@ -0,0 +1,166 @@
+/* Copyright 2010, 2011 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/>.  */
+
+	.section	.debug_info
+d:
+	.long	debug_end - 1f	/* Length of Compilation Unit Info */
+1:
+	.2byte	0x3	/* DWARF version number */
+	.long	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+	.ascii "GNU C 4.4.3\0"	/* DW_AT_producer */
+	.byte	0x1	/* DW_AT_language */
+	.ascii "1.c\0"	/* DW_AT_name */
+
+.Ltype_int:
+	.uleb128 0x7	/* DW_TAG_base_type */
+	.byte	0x4	/* DW_AT_byte_size */
+	.byte	0x5	/* DW_AT_encoding */
+	.ascii "int\0"	/* DW_AT_name */
+
+.Ltype_struct:
+	.uleb128 0x2	/* DW_TAG_structure_type */
+	.ascii "s\0"	/* DW_AT_name */
+	.byte	4	/* DW_AT_byte_size */
+
+	.uleb128 0x3	/* DW_TAG_member */
+	.ascii "f\0"	/* DW_AT_name */
+	.4byte	.Ltype_int - d	/* DW_AT_type */
+	.byte	0	/* DW_AT_data_member_location */
+
+	.byte	0x0	/* end of children of DW_TAG_structure_type */
+
+	.uleb128	6			/* Abbrev: DW_TAG_subprogram */
+	.ascii		"main\0"		/* DW_AT_name */
+	.4byte		main			/* DW_AT_low_pc */
+	.4byte		main + 0x100		/* DW_AT_high_pc */
+	.4byte		.Ltype_int - d		/* DW_AT_type */
+	.byte		1			/* DW_AT_external */
+
+.Ltype_structptr:
+	.uleb128 0x5	/* DW_TAG_pointer_type */
+	.byte	0x4	/* DW_AT_byte_size */
+	.long	.Ltype_struct - d	/* DW_AT_type */
+
+.Lvar_out:
+	.uleb128 0x4	/* (DW_TAG_variable) */
+	.ascii "v\0"	/* DW_AT_name */
+	.byte	0	/* DW_AT_location: DW_FORM_block1 */
+	.4byte	.Ltype_struct - d	/* DW_AT_type */
+
+	.uleb128 0x4	/* (DW_TAG_variable) */
+	.ascii "p\0"	/* DW_AT_name */
+	.byte	2f - 1f	/* DW_AT_location: DW_FORM_block1 */
+1:
+	.byte	0xf2	/* DW_OP_GNU_implicit_pointer */
+	.4byte	.Lvar_out - d	/* referenced DIE */
+	.sleb128	0	/* offset */
+2:
+	.4byte	.Ltype_structptr - d	/* DW_AT_type */
+
+	.byte	0x0	/* end of children of main */
+
+	.byte	0x0	/* end of children of CU */
+debug_end:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x13	/* (TAG: DW_TAG_structure_type) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.byte	0
+	.byte	0
+
+	.uleb128 0x3	/* (abbrev code) */
+	.uleb128 0xd	/* (TAG: DW_TAG_member) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x49	/* (DW_AT_type) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.uleb128 0x38	/* (DW_AT_data_member_location) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.byte	0
+	.byte	0
+
+	.uleb128 0x4	/* (abbrev code) */
+	.uleb128 0x34	/* (TAG: DW_TAG_variable) */
+	.byte	0x0	/* DW_children_yes */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x02	/* (DW_AT_location) */
+	.uleb128 0xa	/* (DW_FORM_block1) */
+	.uleb128 0x49	/* (DW_AT_type) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128 0x5	/* (abbrev code) */
+	.uleb128 0xf	/* (TAG: DW_TAG_pointer_type) */
+	.byte	0x0	/* DW_children_no */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x49	/* (DW_AT_type) */
+	.uleb128 0x13	/* (DW_FORM_ref4) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128	6			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128 0x7	/* (abbrev code) */
+	.uleb128 0x24	/* (TAG: DW_TAG_base_type) */
+	.byte	0	/* DW_children_no */
+	.uleb128 0xb	/* (DW_AT_byte_size) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3e	/* (DW_AT_encoding) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0
+	.byte	0
+
+	.byte	0x0
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/implptr-optimized-out.exp
@@ -0,0 +1,37 @@
+# Copyright 2011 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
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+set testfile "implptr-optimized-out"
+set srcfile ${testfile}.S
+set mainfile main.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+if [prepare_for_testing ${testfile}.exp $executable "${srcfile} ${mainfile}" {}] {
+    return -1
+}
+
+# DW_OP_GNU_implicit_pointer implementation requires a valid frame.
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "p p->f" "value has been optimized out"

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Pedro Alves]

On Monday 10 October 2011 21:54:08, Jan Kratochvil wrote:
> On Mon, 03 Oct 2011 21:34:16 +0200, Tom Tromey wrote:
> > >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> > 
> > Jan> ((struct) <optimized out>).field should be IMO still <optimized
> > Jan> out>; just it became internal-error now.
> > 
> > Maybe I am misunderstanding what you are saying -- I think this should
> > also throw.  I think the rule should be that any attempt to access any
> > "invalid" contents of a value, for purposes of computation, should throw
> > an exception.
> 
> Here is problematic the term "for purposes of computation".
> 
> I agree that any computation with content of <optimized out> must throw.
> 
> But here the content is not interpreted in any way.  Only a smaller subset of
> it is used.
> 
> 
> But I do not have any argument why the former <optimized out> value is better
> than this "value has been optimized out", unaware if there is a precedent for
> either way in current codebase.  Changed it as you suggest.

IMO, this is just like "p s.f", printing <unavailable> when the whole 
of `s' is unavailable.  From the unavailable.exp test:

print globalstruct.memberf
$7 = <unavailable>
(gdb) PASS: gdb.trace/unavailable.exp: collect globals: print globalstruct.memberf
print globalstruct.memberd
print globalstruct
$9 = {memberc = <unavailable>, memberi = <unavailable>, memberf = <unavailable>, memberd = <unavailable>}

It just happens that today, we only support either wholy
optimized-out values, or wholly not optimized-out values.  A
compiler can flatten out structures and optimize out just some
unused fields (of local vars, most usefully).  When
we get to support that, it'll follow naturally that a single
optimized out flag per value isn't sufficient, and that
((struct) <optimized out>).field will need to be able to
be <optimized out>.

-- 
Pedro Alves

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> It just happens that today, we only support either wholy
Pedro> optimized-out values, or wholly not optimized-out values.  A
Pedro> compiler can flatten out structures and optimize out just some
Pedro> unused fields (of local vars, most usefully).  When
Pedro> we get to support that, it'll follow naturally that a single
Pedro> optimized out flag per value isn't sufficient, and that
Pedro> ((struct) <optimized out>).field will need to be able to
Pedro> be <optimized out>.

Actually I think GCC can already do all of this, and GDB can
(supposedly) handle it.  There are a couple of lval_computed methods
used to handle this -- check_validity and check_any_valid.

I don't remember how value_primitive_field was supposed to work with
this exactly, but I think the key is that set_value_component_location
on the new value correctly inherits the 'computed' vtable; so the
optimized-out status bits are preserved by this operation.

Given this I tend to think that the patch that went in is incorrect, in
that it may break operations on an SRA'd structure where some bits are
optimized away.  I thought I added tests for this, not sure though.

Tom

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Jan Kratochvil]

On Tue, 11 Oct 2011 19:02:45 +0200, Tom Tromey wrote:
> Given this I tend to think that the patch that went in is incorrect,

Which patch do you refer to here?  Neither of the two proposed patch of mine
in this threads went it yet.


> in
> that it may break operations on an SRA'd structure where some bits are
> optimized away.  I thought I added tests for this, not sure though.

Missing DW_AT_data_member_location is defined as offset 0.  Not sure how an
optimized out field should look like.  Empty DWARF block will just not modify
the struct base address, therefore it may mean also the offset 0 instead of
optimized-out value.  And popping the base address keeping the stack empty is
not an empty DWARF block then.

I tried some testcase but I failed to force GCC making a field optimized out.


Thanks,
Jan

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Jan Kratochvil]

On Mon, 10 Oct 2011 23:40:28 +0200, Pedro Alves wrote:
> IMO, this is just like "p s.f", printing <unavailable> when the whole 
> of `s' is unavailable.  From the unavailable.exp test:
> 
> print globalstruct.memberf
> $7 = <unavailable>
> (gdb) PASS: gdb.trace/unavailable.exp: collect globals: print globalstruct.memberf
> print globalstruct.memberd
> print globalstruct
> $9 = {memberc = <unavailable>, memberi = <unavailable>, memberf = <unavailable>, memberd = <unavailable>}
> 
> It just happens that today, we only support either wholy
> optimized-out values, or wholly not optimized-out values.

This would suggest the user-visible behavior should be more the former patch:
	http://sourceware.org/ml/gdb-patches/2011-09/msg00449.html
	printing <optimized out>

In either patch case when partially-<optimized out> values get implemented the
code needs to be changed anyway.


> and that ((struct) <optimized out>).field will need to be able to be
> <optimized out>.

Yes.


Thanks,
jan

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Tom Tromey]

Jan> Which patch do you refer to here?  Neither of the two proposed
Jan> patch of mine in this threads went it yet.

Oops, sorry.

>> in
>> that it may break operations on an SRA'd structure where some bits are
>> optimized away.  I thought I added tests for this, not sure though.

Jan> Missing DW_AT_data_member_location is defined as offset 0.  Not
Jan> sure how an optimized out field should look like.

It would be an empty DW_OP_piece.

Jan>  Empty DWARF block will just not modify the struct base address,
Jan> therefore it may mean also the offset 0 instead of optimized-out
Jan> value.  And popping the base address keeping the stack empty is not
Jan> an empty DWARF block then.

Based on this I think I probably am missing some context here.  So you
can just ignore me if you want :)

Jan> I tried some testcase but I failed to force GCC making a field
Jan> optimized out.

pieces.exp tests this.  See pieces_test_f6.

Tom

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Pedro Alves]

On Wednesday 12 October 2011 21:33:29, Jan Kratochvil wrote:
> On Mon, 10 Oct 2011 23:40:28 +0200, Pedro Alves wrote:
> > IMO, this is just like "p s.f", printing <unavailable> when the whole 
> > of `s' is unavailable.  From the unavailable.exp test:
> > 
> > print globalstruct.memberf
> > $7 = <unavailable>
> > (gdb) PASS: gdb.trace/unavailable.exp: collect globals: print globalstruct.memberf
> > print globalstruct.memberd
> > print globalstruct
> > $9 = {memberc = <unavailable>, memberi = <unavailable>, memberf = <unavailable>, memberd = <unavailable>}
> > 
> > It just happens that today, we only support either wholy
> > optimized-out values, or wholly not optimized-out values.
> 
> This would suggest the user-visible behavior should be more the former patch:
> 	http://sourceware.org/ml/gdb-patches/2011-09/msg00449.html
> 	printing <optimized out>

Yes.

-- 
Pedro Alves

Re: [patch] Fix internal error on optimized-out values (regression by me)

[Jan Kratochvil]

On Thu, 13 Oct 2011 17:18:27 +0200, Tom Tromey wrote:
> Jan> Missing DW_AT_data_member_location is defined as offset 0.  Not
> Jan> sure how an optimized out field should look like.
> 
> It would be an empty DW_OP_piece.

I forgot, OK, GCC makes optimized out the value bits, not the type definition.

That is still compatible with the first patch.


On Thu, 13 Oct 2011 17:27:34 +0200, Pedro Alves wrote:
> On Wednesday 12 October 2011 21:33:29, Jan Kratochvil wrote:
> > This would suggest the user-visible behavior should be more the former patch:
> > 	http://sourceware.org/ml/gdb-patches/2011-09/msg00449.html
> > 	printing <optimized out>
> 
> Yes.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2011-10/msg00114.html


Thanks,
Jan