public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Tweak gdb.base/async.exp
@ 2014-06-06  7:46 Yao Qi
  2014-06-06 10:15 ` Pedro Alves
  0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2014-06-06  7:46 UTC (permalink / raw)
  To: gdb-patches

I see two fails in async.exp on arm-none-eabi target:

nexti&^M
(gdb) 0x000001ba        14       x = 5; x = 5;^M
completed.^M
FAIL: gdb.base/async.exp: nexti&
finish&^M
Run till exit from #0  0x000001ba in foo () at /scratch/yqi/arm-none-eabi-lite/src/gdb-trunk/gdb/testsuite/gdb.base/async.c:14^M
(gdb) 0x000001e6 in main () at /scratch/yqi/arm-none-eabi-lite/src/gdb-trunk/gdb/testsuite/gdb.base/async.c:32^M
32       y = foo ();^M
Value returned is $1 = 8^M
completed.^M
FAIL: gdb.base/async.exp: finish&

The corresponding test is "test_background "nexti&" "" ".*y = 3.*"",
and it assumes that GDB "nexti" into the next source line.  It is wrong
on arm.  After "nexti", it still stops at the same source line, and it
fails.

When gdb does "finish", if the PC is in the middle of a source line,
the PC address is printed too.  See stack.c:print_frame,

  if (opts.addressprint)
    if (!sal.symtab
	|| frame_show_address (frame, sal)
	|| print_what == LOC_AND_ADDRESS)
      {
	annotate_frame_address ();
	if (pc_p)
	  ui_out_field_core_addr (uiout, "addr", gdbarch, pc);
	else
	  ui_out_field_string (uiout, "addr", "<unavailable>");
	annotate_frame_address_end ();
	ui_out_text (uiout, " in ");
      }

frame_show_address checks whether PC is the middle of a source line.
Since after "nexti", the inferior stops at the middle of a source line,
when we do "finish" the PC address is displayed.

In sum, GDB works well, but test case needs update.  This patch is to
match the output when the inferior doesn't go to the new line after
"nexti", and match the hex address the output of "finish" optionally.

gdb/testsuite:

2014-06-06  Yao Qi  <yao@codesourcery.com>

	* gdb.base/async.exp: Get the next instruction address and
	match the output of "nexti" either by instruction address or
	by source line.  Optionally match the address in the output
	of "finish".
---
 gdb/testsuite/gdb.base/async.exp | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/async.exp b/gdb/testsuite/gdb.base/async.exp
index 0f99b01..c5a8cdf 100644
--- a/gdb/testsuite/gdb.base/async.exp
+++ b/gdb/testsuite/gdb.base/async.exp
@@ -75,10 +75,24 @@ test_background "step&" "" " foo \\(\\) at .*async.c.*x = 5.*" "step& #2"
 
 test_background "stepi&" "" ".*$hex.*x = 5.*"
 
-test_background "nexti&" "" ".*y = 3.*"
+# Get the next instruction address.
+set next_insn_addr ""
+set test "get next insn"
+gdb_test_multiple {x/2i $pc} "$test" {
+    -re "=> $hex .* 0x(\[0-9a-f\]*) .*$gdb_prompt $" {
+	set next_insn_addr $expect_out(1,string)
+	pass "$test"
+    }
+}
+
+# We may nexti into the same source line or into the next source line.
+# In the former case, the current PC is printed out.  We match either
+# of them.
+test_background "nexti&" "" ".*( 0x0*$next_insn_addr|y = 3).*"
 
+# The PC address is displayed if PC is in the middle of a source line.
 test_background "finish&" \
-    "Run till exit from #0  foo \\(\\) at.*async.c.*\r\n" \
+    "Run till exit from #0  ($hex in )?foo \\(\\) at.*async.c.*\r\n" \
     ".*$hex in main \\(\\) at.*async.c.*y = foo \\(\\).*Value returned is.*= 8.*"
 
 set jump_here [gdb_get_line_number "jump here"]
-- 
1.9.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Tweak gdb.base/async.exp
  2014-06-06  7:46 [PATCH] Tweak gdb.base/async.exp Yao Qi
@ 2014-06-06 10:15 ` Pedro Alves
  2014-06-06 12:33   ` Yao Qi
  0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2014-06-06 10:15 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 06/06/2014 08:44 AM, Yao Qi wrote:
> I see two fails in async.exp on arm-none-eabi target:
> 
> nexti&^M
> (gdb) 0x000001ba        14       x = 5; x = 5;^M
> completed.^M
> FAIL: gdb.base/async.exp: nexti&
> finish&^M
> Run till exit from #0  0x000001ba in foo () at /scratch/yqi/arm-none-eabi-lite/src/gdb-trunk/gdb/testsuite/gdb.base/async.c:14^M
> (gdb) 0x000001e6 in main () at /scratch/yqi/arm-none-eabi-lite/src/gdb-trunk/gdb/testsuite/gdb.base/async.c:32^M
> 32       y = foo ();^M
> Value returned is $1 = 8^M
> completed.^M
> FAIL: gdb.base/async.exp: finish&
> 
> The corresponding test is "test_background "nexti&" "" ".*y = 3.*"",
> and it assumes that GDB "nexti" into the next source line.  It is wrong
> on arm.  After "nexti", it still stops at the same source line, and it
> fails.

Note: "nexti" is supposed to skip function calls, but the test
isn't actually testing that works.  It'd be nice if it
did.  E.g., we'd add:

for (i = 0; i < 2; i++)
  foo ();

and "stepi" the first loop iteration looking for the
first address that has a "backtrace" with two frames.  The
"call/jmp" instruction that nexti& should step over would
be the address the program was stopped before the last
stepi.  We'd run to the address again (for the second iteration),
and do a "nexti&", making sure we land on the next
instruction, after the call returns.

But TBC, since the test doesn't do this today, it's fine to
fix it assuming nexti is just like stepi.  With in mind ...

> +# We may nexti into the same source line or into the next source line.
> +# In the former case, the current PC is printed out.  We match either
> +# of them.
> +test_background "nexti&" "" ".*( 0x0*$next_insn_addr|y = 3).*"

I'd rather always only check by address.  It's simpler and it'd
prevent a bug where we should still end on the same line, like
on ARM, but nexti steps too much and only stops on the
next line.

I think we just need to do:

- x = 5; x = 5;
+ x = 5; x = 5; x = 5;

in the C file?  'x' is volatile, so that'd always makes us land
in the middle of the line.

> +# The PC address is displayed if PC is in the middle of a source line.
>  test_background "finish&" \
> -    "Run till exit from #0  foo \\(\\) at.*async.c.*\r\n" \
> +    "Run till exit from #0  ($hex in )?foo \\(\\) at.*async.c.*\r\n" \

This can then be unconditional too.

>      ".*$hex in main \\(\\) at.*async.c.*y = foo \\(\\).*Value returned is.*= 8.*"
>  
>  set jump_here [gdb_get_line_number "jump here"]

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Tweak gdb.base/async.exp
  2014-06-06 10:15 ` Pedro Alves
@ 2014-06-06 12:33   ` Yao Qi
  2014-06-06 12:48     ` Pedro Alves
  0 siblings, 1 reply; 4+ messages in thread
From: Yao Qi @ 2014-06-06 12:33 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 06/06/2014 06:15 PM, Pedro Alves wrote:
> Note: "nexti" is supposed to skip function calls, but the test
> isn't actually testing that works.  It'd be nice if it
> did.  E.g., we'd add:
> 
> for (i = 0; i < 2; i++)
>   foo ();
> 
> and "stepi" the first loop iteration looking for the
> first address that has a "backtrace" with two frames.  The
> "call/jmp" instruction that nexti& should step over would
> be the address the program was stopped before the last
> stepi.  We'd run to the address again (for the second iteration),
> and do a "nexti&", making sure we land on the next
> instruction, after the call returns.
> 
> But TBC, since the test doesn't do this today, it's fine to
> fix it assuming nexti is just like stepi.  With in mind ...

Yes, we should cover nexti and stepi for function call.  However,
I don't have extra cycles on that at this moment, because I am triaging
new fails since 7.7 release on arm targets.  We can revisit it after
7.8 release.

> 
>> > +# We may nexti into the same source line or into the next source line.
>> > +# In the former case, the current PC is printed out.  We match either
>> > +# of them.
>> > +test_background "nexti&" "" ".*( 0x0*$next_insn_addr|y = 3).*"
> I'd rather always only check by address.  It's simpler and it'd
> prevent a bug where we should still end on the same line, like
> on ARM, but nexti steps too much and only stops on the
> next line.
> 
> I think we just need to do:
> 
> - x = 5; x = 5;
> + x = 5; x = 5; x = 5;
> 
> in the C file?  'x' is volatile, so that'd always makes us land
> in the middle of the line.
> 

Yes, that is sufficient.  The patch is updated.

-- 
Yao (齐尧)

Subject: [PATCH] Tweak gdb.base/async.exp

I see two fails in async.exp on arm-none-eabi target:

nexti&^M
(gdb) 0x000001ba        14       x = 5; x = 5;^M
completed.^M
FAIL: gdb.base/async.exp: nexti&
finish&^M
Run till exit from #0  0x000001ba in foo () at /scratch/yqi/arm-none-eabi-lite/src/gdb-trunk/gdb/testsuite/gdb.base/async.c:14^M
(gdb) 0x000001e6 in main () at /scratch/yqi/arm-none-eabi-lite/src/gdb-trunk/gdb/testsuite/gdb.base/async.c:32^M
32       y = foo ();^M
Value returned is $1 = 8^M
completed.^M
FAIL: gdb.base/async.exp: finish&

The corresponding test is "test_background "nexti&" "" ".*y = 3.*"",
and it assumes that GDB "nexti" into the next source line.  It is wrong
on arm.  After "nexti", it still stops at the same source line, and it
fails.

When gdb does "finish", if the PC is in the middle of a source line,
the PC address is printed too.  See stack.c:print_frame,

  if (opts.addressprint)
    if (!sal.symtab
	|| frame_show_address (frame, sal)
	|| print_what == LOC_AND_ADDRESS)
      {
	annotate_frame_address ();
	if (pc_p)
	  ui_out_field_core_addr (uiout, "addr", gdbarch, pc);
	else
	  ui_out_field_string (uiout, "addr", "<unavailable>");
	annotate_frame_address_end ();
	ui_out_text (uiout, " in ");
      }

frame_show_address checks whether PC is the middle of a source line.
Since after "nexti", the inferior stops at the middle of a source line,
when we do "finish" the PC address is displayed.

In sum, GDB works well, but test case needs update.  This patch is to
add a statement at the same line to make sure "nexti" doesn't go to
the new line, match the next instruction address in the output and
match the hex address the output of "finish".

gdb/testsuite:

2014-06-06  Yao Qi  <yao@codesourcery.com>

	* gdb.base/async.c (foo): Add one statement.
	* gdb.base/async.exp: Get the next instruction address and
	match the output of "nexti" by instruction address.  Match
	the hex address in the output of "finish".
---
 gdb/testsuite/gdb.base/async.c   |  2 +-
 gdb/testsuite/gdb.base/async.exp | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/async.c b/gdb/testsuite/gdb.base/async.c
index 76ce8be..649e9e6 100644
--- a/gdb/testsuite/gdb.base/async.c
+++ b/gdb/testsuite/gdb.base/async.c
@@ -11,7 +11,7 @@ foo ()
  int y;
  volatile int x;
 
- x = 5; x = 5;
+ x = 5; x = 5; x = 5;
  y = 3;
 
  return x + y;
diff --git a/gdb/testsuite/gdb.base/async.exp b/gdb/testsuite/gdb.base/async.exp
index 0f99b01..cce8b5b 100644
--- a/gdb/testsuite/gdb.base/async.exp
+++ b/gdb/testsuite/gdb.base/async.exp
@@ -75,10 +75,22 @@ test_background "step&" "" " foo \\(\\) at .*async.c.*x = 5.*" "step& #2"
 
 test_background "stepi&" "" ".*$hex.*x = 5.*"
 
-test_background "nexti&" "" ".*y = 3.*"
+# Get the next instruction address.
+set next_insn_addr ""
+set test "get next insn"
+gdb_test_multiple {x/2i $pc} "$test" {
+    -re "=> $hex .* 0x(\[0-9a-f\]*) .*$gdb_prompt $" {
+	set next_insn_addr $expect_out(1,string)
+	pass "$test"
+    }
+}
+
+# We nexti into the same source line.  The current PC is printed out.
+test_background "nexti&" "" ".* 0x0*$next_insn_addr.* x = 5; .*"
 
+# PC is in the middle of a source line, so the PC address is displayed.
 test_background "finish&" \
-    "Run till exit from #0  foo \\(\\) at.*async.c.*\r\n" \
+    "Run till exit from #0  $hex in foo \\(\\) at.*async.c.*\r\n" \
     ".*$hex in main \\(\\) at.*async.c.*y = foo \\(\\).*Value returned is.*= 8.*"
 
 set jump_here [gdb_get_line_number "jump here"]
-- 
1.9.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Tweak gdb.base/async.exp
  2014-06-06 12:33   ` Yao Qi
@ 2014-06-06 12:48     ` Pedro Alves
  0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2014-06-06 12:48 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 06/06/2014 01:31 PM, Yao Qi wrote:

> Yes, that is sufficient.  The patch is updated.

OK.

Thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-06-06 12:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-06  7:46 [PATCH] Tweak gdb.base/async.exp Yao Qi
2014-06-06 10:15 ` Pedro Alves
2014-06-06 12:33   ` Yao Qi
2014-06-06 12:48     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).