[PATCH] Set entryval_error to NULL if entryval is set

[PATCH] Set entryval_error to NULL if entryval is set

[Yao Qi]

Hi,
I get an internal error when 'set print entry-values' to 'both' and
then type command 'tfind 0', shown as below,

(gdb) target remote :1234
Remote debugging using :1234
(gdb) b main
Breakpoint 1 at 0x80483eb: file ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c, line 41.
(gdb) c
Continuing.

Breakpoint 1,
main () at ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c:41
41        bar (4, s);

(gdb) trace bar
Tracepoint 3 at 0x80483ca: file ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c, line 22.
(gdb) actions
Enter actions for tracepoint 3, one per line.
End with a line saying just "end".
 >collect array
 >collect j
 >end
(gdb) tstart
(gdb) b marker
Breakpoint 4 at 0x80483e3: file ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c, line 34.
(gdb) c
Continuing.
Breakpoint 4, marker () at ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c:34
34      {}
(gdb) tstop
(gdb) set print entry-values both
(gdb) tfind 0
../../../git/gdb/stack.c:223: internal-error: print_frame_arg: Assertion `!arg->val || !arg->error' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? "

In print_frame_arg, this assert indicates that either arg->val or
arg->error should be NULL.  It looks right to me.  This patch is to
set 'entryval_error' NULL when 'entryval' is set (non-NULL).

The test case is modified to expose this internal error, and without
fix in read_frame_arg, we can get the internal error when running
gdb.trace/collection.exp

(gdb) PASS: gdb.trace/collection.exp: collect args collectively: tfind test frame
tfind -1^M
No longer looking at any trace frame^M
79      }^M
(gdb) set print entry-values only^M
(gdb) tfind 0^M
Found trace frame 0, tracepoint 4^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
Quit this debugging session? (y or n) FAIL: gdb.trace/collection.exp: collect args collectively: tfind 0 with entry-values only (GDB internal error)

This internal error is fixed when the patch is applied.  The patch is
tested on x86_64-linux with board file unix.exp and
native-gdbserver.exp respectively.

gdb:

2013-08-05  Yao Qi  <yao@codesourcery.com>

	* stack.c (read_frame_arg): Set 'entryval_error' to NULL if
	'entryval' is set.

gdb/testsuite:

2013-08-05  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/collection.exp (gdb_collect_args_test): Set
	"only" and "both" to 'print entry-values' before selecting
	trace frame.
---
 gdb/stack.c                            |    5 ++++-
 gdb/testsuite/gdb.trace/collection.exp |   16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index 3177877..9e9ebc1 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -475,7 +475,10 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
 	  || print_entry_values == print_entry_values_both
 	  || (print_entry_values == print_entry_values_preferred
 	      && (!val || value_optimized_out (val))))
-	entryval = allocate_optimized_out_value (SYMBOL_TYPE (sym));
+	{
+	  entryval = allocate_optimized_out_value (SYMBOL_TYPE (sym));
+	  entryval_error = NULL;
+	}
     }
   if ((print_entry_values == print_entry_values_compact
        || print_entry_values == print_entry_values_if_needed
diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
index f6d44ce..844cbb5 100644
--- a/gdb/testsuite/gdb.trace/collection.exp
+++ b/gdb/testsuite/gdb.trace/collection.exp
@@ -128,6 +128,22 @@ proc gdb_collect_args_test { myargs msg } {
     # Begin the test.
     run_trace_experiment $msg args_test_func
 
+    # Frame arguments and their entry values are displaced correctly with
+    # various values of "print entry-values" when a trace frame is
+    # selected.
+
+    gdb_test "tfind -1" ".*" ""
+    gdb_test_no_output "set print entry-values only" ""
+    gdb_test "tfind 0" \
+	" \\(argc@entry=\[^,\]*, argi@entry=\[^,\]*, argf@entry=\[^,\]*, argd@entry=\[^,\]*, argstruct@entry=\[^,\]*, argarray@entry=\[^,\]*\\) .*" \
+	"collect $msg: tfind 0 with entry-values only"
+
+    gdb_test "tfind -1" ".*" ""
+    gdb_test_no_output "set print entry-values both" ""
+    gdb_test "tfind 0" \
+	" \\(argc=\[^,\]*, argc@entry=\[^,\]*, argi=\[^,\]*, argi@entry=\[^,\]*, argf=\[^,\]*, argf@entry=\[^,\]*, argd=\[^,\]*, argd@entry=\[^,\]*, argstruct=\[^,\]*, argstruct@entry=\[^,\]*, argarray=\[^,\]*, argarray@entry=\[^,\]*\\) .*" \
+	"collect $msg: tfind 0 with entry-values both"
+
     gdb_test "print argc" \
 	    "\\$\[0-9\]+ = 1 '.001'$cr" \
 	    "collect $msg: collected arg char"
-- 
1.7.7.6

RE: [PATCH] Set entryval_error to NULL if entryval is set

[Agovic, Sanimir]

lgtm. Unfortunately I cannot approve your patch.

 -Sanimir

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Yao Qi
> Sent: Monday, August 05, 2013 10:16 AM
> To: gdb-patches@sourceware.org
> Subject: [PATCH] Set entryval_error to NULL if entryval is set
> 
> Hi,
> I get an internal error when 'set print entry-values' to 'both' and
> then type command 'tfind 0', shown as below,
> 
> (gdb) target remote :1234
> Remote debugging using :1234
> (gdb) b main
> Breakpoint 1 at 0x80483eb: file ../../../../git/gdb/testsuite/gdb.trace/trace-
> unavailable.c, line 41.
> (gdb) c
> Continuing.
> 
> Breakpoint 1,
> main () at ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c:41
> 41        bar (4, s);
> 
> (gdb) trace bar
> Tracepoint 3 at 0x80483ca: file ../../../../git/gdb/testsuite/gdb.trace/trace-
> unavailable.c, line 22.
> (gdb) actions
> Enter actions for tracepoint 3, one per line.
> End with a line saying just "end".
>  >collect array
>  >collect j
>  >end
> (gdb) tstart
> (gdb) b marker
> Breakpoint 4 at 0x80483e3: file ../../../../git/gdb/testsuite/gdb.trace/trace-
> unavailable.c, line 34.
> (gdb) c
> Continuing.
> Breakpoint 4, marker () at ../../../../git/gdb/testsuite/gdb.trace/trace-unavailable.c:34
> 34      {}
> (gdb) tstop
> (gdb) set print entry-values both
> (gdb) tfind 0
> ../../../git/gdb/stack.c:223: internal-error: print_frame_arg: Assertion `!arg->val ||
> !arg->error' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? "
> 
> In print_frame_arg, this assert indicates that either arg->val or
> arg->error should be NULL.  It looks right to me.  This patch is to
> set 'entryval_error' NULL when 'entryval' is set (non-NULL).
> 
> The test case is modified to expose this internal error, and without
> fix in read_frame_arg, we can get the internal error when running
> gdb.trace/collection.exp
> 
> (gdb) PASS: gdb.trace/collection.exp: collect args collectively: tfind test frame
> tfind -1^M
> No longer looking at any trace frame^M
> 79      }^M
> (gdb) set print entry-values only^M
> (gdb) tfind 0^M
> Found trace frame 0, tracepoint 4^M
> A problem internal to GDB has been detected,^M
> further debugging may prove unreliable.^M
> Quit this debugging session? (y or n) FAIL: gdb.trace/collection.exp: collect args
> collectively: tfind 0 with entry-values only (GDB internal error)
> 
> This internal error is fixed when the patch is applied.  The patch is
> tested on x86_64-linux with board file unix.exp and
> native-gdbserver.exp respectively.
> 
> gdb:
> 
> 2013-08-05  Yao Qi  <yao@codesourcery.com>
> 
> 	* stack.c (read_frame_arg): Set 'entryval_error' to NULL if
> 	'entryval' is set.
> 
> gdb/testsuite:
> 
> 2013-08-05  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.trace/collection.exp (gdb_collect_args_test): Set
> 	"only" and "both" to 'print entry-values' before selecting
> 	trace frame.
> ---
>  gdb/stack.c                            |    5 ++++-
>  gdb/testsuite/gdb.trace/collection.exp |   16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 1 deletions(-)
> 
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 3177877..9e9ebc1 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -475,7 +475,10 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
>  	  || print_entry_values == print_entry_values_both
>  	  || (print_entry_values == print_entry_values_preferred
>  	      && (!val || value_optimized_out (val))))
> -	entryval = allocate_optimized_out_value (SYMBOL_TYPE (sym));
> +	{
> +	  entryval = allocate_optimized_out_value (SYMBOL_TYPE (sym));
> +	  entryval_error = NULL;
> +	}
>      }
>    if ((print_entry_values == print_entry_values_compact
>         || print_entry_values == print_entry_values_if_needed
> diff --git a/gdb/testsuite/gdb.trace/collection.exp
> b/gdb/testsuite/gdb.trace/collection.exp
> index f6d44ce..844cbb5 100644
> --- a/gdb/testsuite/gdb.trace/collection.exp
> +++ b/gdb/testsuite/gdb.trace/collection.exp
> @@ -128,6 +128,22 @@ proc gdb_collect_args_test { myargs msg } {
>      # Begin the test.
>      run_trace_experiment $msg args_test_func
> 
> +    # Frame arguments and their entry values are displaced correctly with
> +    # various values of "print entry-values" when a trace frame is
> +    # selected.
> +
> +    gdb_test "tfind -1" ".*" ""
> +    gdb_test_no_output "set print entry-values only" ""
> +    gdb_test "tfind 0" \
> +	" \\(argc@entry=\[^,\]*, argi@entry=\[^,\]*, argf@entry=\[^,\]*, argd@entry=\[^,\]*,
> argstruct@entry=\[^,\]*, argarray@entry=\[^,\]*\\) .*" \
> +	"collect $msg: tfind 0 with entry-values only"
> +
> +    gdb_test "tfind -1" ".*" ""
> +    gdb_test_no_output "set print entry-values both" ""
> +    gdb_test "tfind 0" \
> +	" \\(argc=\[^,\]*, argc@entry=\[^,\]*, argi=\[^,\]*, argi@entry=\[^,\]*,
> argf=\[^,\]*, argf@entry=\[^,\]*, argd=\[^,\]*, argd@entry=\[^,\]*, argstruct=\[^,\]*,
> argstruct@entry=\[^,\]*, argarray=\[^,\]*, argarray@entry=\[^,\]*\\) .*" \
> +	"collect $msg: tfind 0 with entry-values both"
> +
>      gdb_test "print argc" \
>  	    "\\$\[0-9\]+ = 1 '.001'$cr" \
>  	    "collect $msg: collected arg char"
> --
> 1.7.7.6

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH] Set entryval_error to NULL if entryval is set

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2013-08-05  Yao Qi  <yao@codesourcery.com>
Yao> 	* stack.c (read_frame_arg): Set 'entryval_error' to NULL if
Yao> 	'entryval' is set.

Yao> 2013-08-05  Yao Qi  <yao@codesourcery.com>
Yao> 	* gdb.trace/collection.exp (gdb_collect_args_test): Set
Yao> 	"only" and "both" to 'print entry-values' before selecting
Yao> 	trace frame.

The bug fix part looks ok to me, but I think you need a special test
case in order to really test entry values.  Or to put it another way,
what happens if this test is run with a compiler that doesn't emit entry
value information?

Tom

Re: [PATCH] Set entryval_error to NULL if entryval is set

[Pedro Alves]

On 08/05/2013 09:16 AM, Yao Qi wrote:
> +    # Frame arguments and their entry values are displaced correctly with
> +    # various values of "print entry-values" when a trace frame is
> +    # selected.

typo: s/displaced/displayed.

-- 
Pedro Alves

Re: [PATCH] Set entryval_error to NULL if entryval is set

[Yao Qi]

On 08/05/2013 10:06 PM, Tom Tromey wrote:
> The bug fix part looks ok to me, but I think you need a special test
> case in order to really test entry values.  Or to put it another way,
> what happens if this test is run with a compiler that doesn't emit entry
> value information?

Tom,
Patch in gdb.trace/collection.exp is to test arguments and/or entry
values are shown properly, without the internal error.  Testing the
contents of entry values is not the point of this test and the patch.

AFAIK, entry values is tested by gdb.arch/amd64-entry-value.exp.

-- 
Yao (齐尧)

Re: [PATCH] Set entryval_error to NULL if entryval is set

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

>> The bug fix part looks ok to me, but I think you need a special test
>> case in order to really test entry values.  Or to put it another way,
>> what happens if this test is run with a compiler that doesn't emit entry
>> value information?

Yao> Patch in gdb.trace/collection.exp is to test arguments and/or entry
Yao> values are shown properly, without the internal error.  Testing the
Yao> contents of entry values is not the point of this test and the patch.

I've re-read the patch and I think that I was misunderstanding what it
was doing.  In particular I missed the "set print entry-values only" bit.

The patch is ok.

thanks,
Tom

Re: [PATCH] Set entryval_error to NULL if entryval is set

[Yao Qi]

On 08/09/2013 03:56 AM, Tom Tromey wrote:
> The patch is ok.

Thanks.  Patch is committed with typo "displaced" fixed.

-- 
Yao (齐尧)

Re: [PATCH] Set entryval_error to NULL if entryval is set

[Jan Kratochvil]

On Mon, 05 Aug 2013 10:16:06 +0200, Yao Qi wrote:
> 2013-08-05  Yao Qi  <yao@codesourcery.com>
> 
> 	* stack.c (read_frame_arg): Set 'entryval_error' to NULL if
> 	'entryval' is set.

thanks for catching it, I sure also agree with the fix.


Jan