public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Bruno Larsen <blarsen@redhat.com>
To: gdb-patches@sourceware.org
Cc: Bruno Larsen <blarsen@redhat.com>
Subject: [PATCH v2 3/4] gdb/testsuite: fix testing gdb.reverse/step-reverse.exp with clang
Date: Thu, 27 Jul 2023 09:41:17 +0200	[thread overview]
Message-ID: <20230727074118.1583199-4-blarsen@redhat.com> (raw)
In-Reply-To: <20230727074118.1583199-1-blarsen@redhat.com>

When testing using reverse-stepi to fully step through a function, the
code checks for an infinite loop by seeing if we land on the line that
contains the return statement multiple times. This assumption only works
if there is only one instruction associated with that line, which is how
GCC handles line information, but other compilers may handle it differently.
Clang-15, for instance, associates 6. Because of this, the inferior used
to get seriously out of sync with the test expectations, and result in 13
spurious failures. The same issue occurs with gdb.reverse/step-precsave.exp.

This commit changes the test so that we check for PC instead of line
number. The test still only happens when the same line is detected, to
simplify the resulting log. With this change, no new failures are
emitted when using clang.

It also adds a new parameter to get_hexadecimal_valueof, so that we can
use it without generating new passes, otherwise we'd get multiple
duplicate test names. This change shouldn't affect any other test using
this proc.
---
 gdb/testsuite/gdb.reverse/step-precsave.exp | 18 +++++++++++++++++-
 gdb/testsuite/gdb.reverse/step-reverse.exp  | 20 +++++++++++++++++++-
 gdb/testsuite/lib/gdb.exp                   |  6 ++++--
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.reverse/step-precsave.exp b/gdb/testsuite/gdb.reverse/step-precsave.exp
index 19cd5d9930e..da3a47e07e2 100644
--- a/gdb/testsuite/gdb.reverse/step-precsave.exp
+++ b/gdb/testsuite/gdb.reverse/step-precsave.exp
@@ -30,6 +30,16 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
     return -1
 }
 
+proc get_current_pc {} {
+    set pc 0
+    gdb_test_multiple "print \$pc" "" {
+	-re -wrap ".*0x(\[0-9a-f\]+).*" {
+	    set pc $expect_out(1,string)
+	}
+    }
+    return $pc
+}
+
 runto_main
 
 # Activate process record/replay
@@ -209,11 +219,17 @@ gdb_test_multiple "stepi" "$test_message" {
 
 # stepi backward out of a function call
 
+set start_pc [get_current_pc]
 set stepi_location  [gdb_get_line_number "STEPI TEST" "$srcfile"]
 set test_message "reverse stepi from a function call"
 gdb_test_multiple "stepi" "$test_message" {
     -re "ARRIVED IN CALLEE.*$gdb_prompt $" {
-	fail "$test_message (start statement)"
+	if { [get_current_pc] == $start_pc } {
+	    fail "$test_message (start statement)"
+	} else {
+	    send_gdb "stepi\n" 
+	    exp_continue
+	}
     }
     -re "ENTER CALLEE.*$gdb_prompt $" {
 	send_gdb "stepi\n" 
diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp b/gdb/testsuite/gdb.reverse/step-reverse.exp
index 4b78a8f8fb7..9ff97bfde42 100644
--- a/gdb/testsuite/gdb.reverse/step-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/step-reverse.exp
@@ -28,6 +28,16 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
     return -1
 }
 
+proc get_current_pc {} {
+    set pc 0
+    gdb_test_multiple "print \$pc" "" {
+	-re -wrap ".*0x(\[0-9a-f\]+).*" {
+	    set pc $expect_out(1,string)
+	}
+    }
+    return $pc
+}
+
 runto_main
 
 if [supports_process_record] {
@@ -174,11 +184,19 @@ gdb_test_multiple "stepi" "$test_message" {
 
 # stepi backward out of a function call
 
+# When testing stepi, we dont want to infinitely step if we're not moving
+# so we store the starting PC, in case we land on the same line as above
+set start_pc [get_hexadecimal_valueof "\$pc" 0 "" "false"]
 set stepi_location  [gdb_get_line_number "STEPI TEST" "$srcfile"]
 set test_message "reverse stepi from a function call"
 gdb_test_multiple "stepi" "$test_message" {
     -re "ARRIVED IN CALLEE.*$gdb_prompt $" {
-	fail "$test_message (start statement)"
+	if { [get_hexadecimal_valueof "\$pc" 0 "" "false"] == $start_pc } {
+	    fail "$test_message (start statement)"
+	} else {
+	    send_gdb "stepi\n" 
+	    exp_continue
+	}
     }
     -re "ENTER CALLEE.*$gdb_prompt $" {
 	send_gdb "stepi\n" 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 63b6291fc36..37342583e0a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7919,7 +7919,7 @@ proc get_integer_valueof { exp default {test ""} } {
 # TEST is the test message to use.  It can be omitted, in which case
 # a test message is built from EXP.
 
-proc get_hexadecimal_valueof { exp default {test ""} } {
+proc get_hexadecimal_valueof { exp default {test ""} {pass true} } {
     global gdb_prompt
 
     if {$test == ""} {
@@ -7930,7 +7930,9 @@ proc get_hexadecimal_valueof { exp default {test ""} } {
     gdb_test_multiple "print /x ${exp}" $test {
 	-re "\\$\[0-9\]* = (0x\[0-9a-zA-Z\]+).*$gdb_prompt $" {
 	    set val $expect_out(1,string)
-	    pass "$test"
+	    if { "$pass" == "true" } {
+		pass "$test"
+	    }
 	}
     }
     return ${val}
-- 
2.41.0


  parent reply	other threads:[~2023-07-27  7:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-25  9:58 [PATCH 0/4] Many fixes to gdb.reverse tests Bruno Larsen
2023-07-25  9:58 ` [PATCH 1/4] gdb/testsuite: Fix many errors in gdb.reverse with clang Bruno Larsen
2023-07-25  9:58 ` [PATCH 2/4] gdb/testsuite: fix gdb.reverse/solib-*.exp tests when using clang Bruno Larsen
2023-07-26 13:37   ` Tom Tromey
2023-07-25  9:58 ` [PATCH 3/4] gdb/testsuite: fix testing gdb.reverse/step-reverse.exp with clang Bruno Larsen
2023-07-26 13:39   ` Tom Tromey
2023-07-25  9:58 ` [PATCH 4/4] gdb/testsuite: Multiple improvements for gdb.reverse/insn-reverse.exp Bruno Larsen
2023-07-27  7:41 ` [PATCH v2 0/4] Many fixes to gdb.reverse tests Bruno Larsen
2023-07-27  7:41   ` [PATCH v2 1/4] gdb/testsuite: Fix many errors in gdb.reverse with clang Bruno Larsen
2023-07-27  7:41   ` [PATCH v2 2/4] gdb/testsuite: fix gdb.reverse/solib-*.exp tests when using clang Bruno Larsen
2023-07-27  7:41   ` Bruno Larsen [this message]
2023-07-28 13:14     ` [PATCH v2 3/4] gdb/testsuite: fix testing gdb.reverse/step-reverse.exp with clang Tom Tromey
2023-07-28 13:20       ` Bruno Larsen
2023-07-28 14:18         ` Tom Tromey
2023-07-28 14:20           ` Bruno Larsen
2023-07-27  7:41   ` [PATCH v2 4/4] gdb/testsuite: Multiple improvements for gdb.reverse/insn-reverse.exp Bruno Larsen

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=20230727074118.1583199-4-blarsen@redhat.com \
    --to=blarsen@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /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).