public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Tom de Vries <tdevries@suse.de>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH, testsuite] Allow function arguments in bp print match in selftest_setup
Date: Thu, 14 Jun 2018 10:46:00 -0000	[thread overview]
Message-ID: <51e2fa52-222b-6ec0-16d3-8c99fcc9a310@redhat.com> (raw)
In-Reply-To: <20180614102107.cgh6iixu324pmpvi@localhost.localdomain>

On 06/14/2018 11:21 AM, Tom de Vries wrote:

>> [1] Funny, it passes for me (with the fix for stopping at captured_main),
>> because the pattern below the one you're touching matches:
>>
>> 	-re "Starting program.*Breakpoint \[0-9\]+,.* at .*main.c:.*$function.*$gdb_prompt $" {
>> 	    # $function may be inlined, so the program stops at the line
>> 	    # calling $function.
>> 	    pass "$description"
>> 	}
>>
>> and it only happens to match because captured_main calls
>> captured_main_1 first thing, which coincidentally
>> matches "$function.*":
>>
>>  Breakpoint 1, captured_main (data=<optimized out>) at gdb/binutils-gdb/src/gdb/main.c:1147
>>  1147      captured_main_1 (context);
>>
>> That would probably be better "$function .*", with a space,
> 
> Right, I observed that as well, and was thinking of this type of fix.

Please don't be shy about stating those things upfront, makes
review easier.  :-)

> 
>> but I 
>> think that even better, we should try removing the "may be inlined" case
>> too now, because since ddfe970e6bec ("Don't elide all inlined frames") GDB
>> presents the stop at the inline function instead of at the caller.
>> Like below.  Comments?
>>
> 
> Agreed, that's a better solution.

Alright, I've pushed this, as below.

From 1d39de443a38448226ffc408290c17c54c598c39 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 14 Jun 2018 11:40:23 +0100
Subject: [PATCH] Remove stale inline function handling from selftest_setup

Before commit 70ee000084aa ("[gdb] Allow function arguments in bp
print match in selftest_setup"), this pattern in selftest_setup:

	-re "Starting program.*Breakpoint \[0-9\]+,.* at .*main.c:.*$function.*$gdb_prompt $" {
	    # $function may be inlined, so the program stops at the line
	    # calling $function.
	    pass "$description"
	}

happened to match if captured_main_1 was inlined and captured_main was
not, because captured_main calls captured_main_1 first thing, which
coincidentally matches "$function.*":

 Breakpoint 1, captured_main (data=<optimized out>) at src/gdb/main.c:1147
 1147      captured_main_1 (context);

That would probably be better "$function .*", with a space, but I
think that even better is to remove the "may be inlined" case too now,
because since ddfe970e6bec ("Don't elide all inlined frames") GDB
presents the stop at the inline function instead of at the caller.

gdb/testsuite/ChangeLog:
2018-06-14  Pedro Alves  <palves@redhat.com>

	* lib/selftest-support.exp (selftest_setup): Remove inlined
	function handling.
---
 gdb/testsuite/ChangeLog                | 5 +++++
 gdb/testsuite/lib/selftest-support.exp | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 723fa299a38..fd6b8fcbf2f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-14  Pedro Alves  <palves@redhat.com>
+
+	* lib/selftest-support.exp (selftest_setup): Remove inlined
+	function handling.
+
 2018-06-14  Tom de Vries  <tdevries@suse.de>
 
 	* lib/selftest-support.exp (selftest_setup): Allow function arguments in
diff --git a/gdb/testsuite/lib/selftest-support.exp b/gdb/testsuite/lib/selftest-support.exp
index 0542861e78b..887ea20d73a 100644
--- a/gdb/testsuite/lib/selftest-support.exp
+++ b/gdb/testsuite/lib/selftest-support.exp
@@ -94,11 +94,6 @@ proc selftest_setup { executable function } {
         -re "Starting program.*Breakpoint \[0-9\]+,.*$function \\(.*\\).*$gdb_prompt $" {
             xfail "$description (line numbers scrambled?)"
         }
-	-re "Starting program.*Breakpoint \[0-9\]+,.* at .*main.c:.*$function.*$gdb_prompt $" {
-	    # $function may be inlined, so the program stops at the line
-	    # calling $function.
-	    pass "$description"
-	}
         -re "vfork: No more processes.*$gdb_prompt $" {
             fail "$description (out of virtual memory)"
             set timeout $oldtimeout
-- 
2.14.3

  reply	other threads:[~2018-06-14 10:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-12 15:15 Tom de Vries
2018-06-13 12:56 ` Pedro Alves
2018-06-14 10:21   ` Tom de Vries
2018-06-14 10:46     ` Pedro Alves [this message]
2018-06-14 13:49       ` Tom de Vries
2018-06-14 13:52         ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51e2fa52-222b-6ec0-16d3-8c99fcc9a310@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tdevries@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).