* [PATCH 2/6] gdb.base/foll-fork.exp: remove gating based on target triplet
2021-09-10 20:53 [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Simon Marchi
@ 2021-09-10 20:53 ` Simon Marchi
2021-09-10 20:53 ` [PATCH 3/6] gdb.base/foll-fork.exp: refactor to restart GDB between each portion of the test Simon Marchi
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-10 20:53 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
It looks like this test has some code to check at runtime the support of
fork handling of the target (see check_fork_catchpoints). So, it seems
to me that the check based on target triplet at the beginning of the
test is not needed. This kind of gating is generally not desirable,
because we wouldn't think of updating it when adding fork support to a
target. For example, FreeBSD supports fork, but it wasn't listed here.
Change-Id: I6b55f2298edae6b37c3681fb8633d8ea1b5aabee
---
gdb/testsuite/gdb.base/foll-fork.exp | 7 -------
1 file changed, 7 deletions(-)
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index f777f2bda35..e6e84bd3bc0 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -13,13 +13,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# Until "set follow-fork-mode" and "catch fork" are implemented on
-# other targets...
-#
-if { ![istarget "*-*-linux*"] && ![istarget "*-*-openbsd*"] } then {
- continue
-}
-
# Test relies on checking follow-fork output. Do not run if gdb debug is
# enabled as it will be redirected to the log.
if [gdb_debug_enabled] {
--
2.33.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 3/6] gdb.base/foll-fork.exp: refactor to restart GDB between each portion of the test
2021-09-10 20:53 [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Simon Marchi
2021-09-10 20:53 ` [PATCH 2/6] gdb.base/foll-fork.exp: remove gating based on target triplet Simon Marchi
@ 2021-09-10 20:53 ` Simon Marchi
2021-09-10 20:54 ` [PATCH 4/6] gdb.base/foll-fork.exp: rename variables Simon Marchi
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-10 20:53 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
This test is difficult to follow and modify because the state of GDB is
preserved some tests. Add a setup proc, which starts a new GDB and runs
to main, and use it in all test procs. Use proc_with_prefix to avoid
duplicates.
The check_fork_catchpoints proc also seems used to check for follow-fork
support by checking if catchpoints are supported. If they are not, it
uses "return -code return", which makes its caller return. I find this
unnecessary complex, versus just returning a boolean. Modify it to do
so.
Change-Id: I23e62b204286c5e9c5c86d2727f7d33fb126ed08
---
gdb/testsuite/gdb.base/foll-fork.exp | 144 +++++++++++++++------------
1 file changed, 81 insertions(+), 63 deletions(-)
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index e6e84bd3bc0..4661bf56a52 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -22,13 +22,33 @@ if [gdb_debug_enabled] {
standard_testfile
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
return -1
}
-proc check_fork_catchpoints {} {
+# Restart GDB and run the inferior to main. Return 1 on success, 0 on failure.
+
+proc setup {} {
+ clean_restart $::testfile
+
+ if { ![runto_main] } {
+ fail "could not run to main"
+ return 0
+ }
+
+ return 1
+}
+
+# Check that fork catchpoints are supported, as an indicator for whether
+# fork-following is supported. Return 1 if they are, else 0.
+
+proc_with_prefix check_fork_catchpoints {} {
global gdb_prompt
+ if { ![setup] } {
+ return
+ }
+
# Verify that the system supports "catch fork".
gdb_test "catch fork" "Catchpoint \[0-9\]* \\(fork\\)" "insert first fork catchpoint"
set has_fork_catchpoints 0
@@ -42,10 +62,7 @@ proc check_fork_catchpoints {} {
}
}
- if {$has_fork_catchpoints == 0} {
- unsupported "fork catchpoints"
- return -code return
- }
+ return $has_fork_catchpoints
}
# Test follow-fork to ensure that the correct process is followed, that
@@ -57,7 +74,7 @@ proc check_fork_catchpoints {} {
# execute the program past the fork. If the value of WHO or DETACH is
# 'default', the corresponding GDB command is skipped for that test.
# The value of CMD must be either 'next 2' or 'continue'.
-proc test_follow_fork { who detach cmd } {
+proc_with_prefix test_follow_fork { who detach cmd } {
global gdb_prompt
global srcfile
global testfile
@@ -65,11 +82,8 @@ proc test_follow_fork { who detach cmd } {
with_test_prefix "follow $who, detach $detach, command \"$cmd\"" {
# Start a new debugger session each time so defaults are legitimate.
- clean_restart $testfile
-
- if ![runto_main] {
- untested "could not run to main"
- return -1
+ if { ![setup] } {
+ return
}
# The "Detaching..." and "Attaching..." messages may be hidden by
@@ -174,11 +188,18 @@ proc test_follow_fork { who detach cmd } {
set reading_in_symbols_re {(?:\r\nReading in symbols for [^\r\n]*)?}
-proc catch_fork_child_follow {} {
+# Test the ability to catch a fork, specify that the child be
+# followed, and continue. Make the catchpoint permanent.
+
+proc_with_prefix catch_fork_child_follow {} {
global gdb_prompt
global srcfile
global reading_in_symbols_re
+ if { ![setup] } {
+ return
+ }
+
set bp_after_fork [gdb_get_line_number "set breakpoint here"]
gdb_test "catch fork" \
@@ -224,10 +245,18 @@ proc catch_fork_child_follow {} {
"y"
}
-proc catch_fork_unpatch_child {} {
+# Test that parent breakpoints are successfully detached from the
+# child at fork time, even if the user removes them from the
+# breakpoints list after stopping at a fork catchpoint.
+
+proc_with_prefix catch_fork_unpatch_child {} {
global gdb_prompt
global srcfile
+ if { ![setup] } {
+ return
+ }
+
set bp_exit [gdb_get_line_number "at exit"]
gdb_test "break callee" "file .*$srcfile, line .*" \
@@ -271,11 +300,18 @@ proc catch_fork_unpatch_child {} {
}
}
-proc tcatch_fork_parent_follow {} {
+# Test the ability to catch a fork, specify via a -do clause that
+# the parent be followed, and continue. Make the catchpoint temporary.
+
+proc_with_prefix tcatch_fork_parent_follow {} {
global gdb_prompt
global srcfile
global reading_in_symbols_re
+ if { ![setup] } {
+ return
+ }
+
set bp_after_fork [gdb_get_line_number "set breakpoint here"]
gdb_test "catch fork" \
@@ -313,9 +349,10 @@ proc tcatch_fork_parent_follow {} {
"y"
}
-proc do_fork_tests {} {
- global gdb_prompt
- global testfile
+# Test simple things about the "set follow-fork-mode" command.
+
+proc_with_prefix test_set_follow_fork_command {} {
+ clean_restart
# Verify that help is available for "set follow-fork-mode".
#
@@ -342,56 +379,37 @@ By default, the debugger will follow the parent process..*"
"set follow-fork to nonsense is prohibited"
gdb_test_no_output "set follow-fork parent" "reset parent"
+}
- # Check that fork catchpoints are supported, as an indicator for whether
- # fork-following is supported.
- if [runto_main] then { check_fork_catchpoints }
+test_set_follow_fork_command
- # Test the basic follow-fork functionality using all combinations of
- # values for follow-fork-mode and detach-on-fork, using either a
- # breakpoint or single-step to execute past the fork.
- #
- # The first loop should be sufficient to test the defaults. There
- # is no need to test using the defaults in other permutations (e.g.
- # "default" "on", "parent" "default", etc.).
- foreach cmd {"next 2" "continue"} {
- test_follow_fork "default" "default" $cmd
- }
+if { ![check_fork_catchpoints] } {
+ untested "follow-fork not supported"
+ return
+}
- # Now test all explicit permutations.
- foreach who {"parent" "child"} {
- foreach detach {"on" "off"} {
- foreach cmd {"next 2" "continue"} {
- test_follow_fork $who $detach $cmd
- }
+# Test the basic follow-fork functionality using all combinations of
+# values for follow-fork-mode and detach-on-fork, using either a
+# breakpoint or single-step to execute past the fork.
+#
+# The first loop should be sufficient to test the defaults. There
+# is no need to test using the defaults in other permutations (e.g.
+# "default" "on", "parent" "default", etc.).
+foreach cmd {"next 2" "continue"} {
+ test_follow_fork "default" "default" $cmd
+}
+
+# Now test all explicit permutations.
+foreach who {"parent" "child"} {
+ foreach detach {"on" "off"} {
+ foreach cmd {"next 2" "continue"} {
+ test_follow_fork $who $detach $cmd
}
}
-
- # Catchpoint tests.
-
- # Restart to eliminate any effects of the follow-fork tests.
- clean_restart $testfile
- gdb_test_no_output "set verbose"
-
- # Test the ability to catch a fork, specify that the child be
- # followed, and continue. Make the catchpoint permanent.
- #
- if [runto_main] then { catch_fork_child_follow }
-
- # Test that parent breakpoints are successfully detached from the
- # child at fork time, even if the user removes them from the
- # breakpoints list after stopping at a fork catchpoint.
- if [runto_main] then { catch_fork_unpatch_child }
-
- # Test the ability to catch a fork, specify via a -do clause that
- # the parent be followed, and continue. Make the catchpoint temporary.
- #
- if [runto_main] then { tcatch_fork_parent_follow }
}
-# This is a test of gdb's ability to follow the parent, child or both
-# parent and child of a Unix fork() system call.
-#
-do_fork_tests
+# Catchpoint tests.
-return 0
+catch_fork_child_follow
+catch_fork_unpatch_child
+tcatch_fork_parent_follow
--
2.33.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 4/6] gdb.base/foll-fork.exp: rename variables
2021-09-10 20:53 [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Simon Marchi
2021-09-10 20:53 ` [PATCH 2/6] gdb.base/foll-fork.exp: remove gating based on target triplet Simon Marchi
2021-09-10 20:53 ` [PATCH 3/6] gdb.base/foll-fork.exp: refactor to restart GDB between each portion of the test Simon Marchi
@ 2021-09-10 20:54 ` Simon Marchi
2021-09-10 20:54 ` [PATCH 5/6] gdb.base/foll-fork.exp: use foreach_with_prefix to handle prefixes Simon Marchi
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-10 20:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Rename the variables / parameters used to match the corresponding GDB
setting name, I find that easier to follow.
Change-Id: Idcbddbbb369279fcf1e808b11a8c478f21b2a946
---
gdb/testsuite/gdb.base/foll-fork.exp | 40 ++++++++++++++--------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 4661bf56a52..3b90bb2e785 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -74,12 +74,12 @@ proc_with_prefix check_fork_catchpoints {} {
# execute the program past the fork. If the value of WHO or DETACH is
# 'default', the corresponding GDB command is skipped for that test.
# The value of CMD must be either 'next 2' or 'continue'.
-proc_with_prefix test_follow_fork { who detach cmd } {
+proc_with_prefix test_follow_fork { follow-fork-mode detach-on-fork cmd } {
global gdb_prompt
global srcfile
global testfile
- with_test_prefix "follow $who, detach $detach, command \"$cmd\"" {
+ with_test_prefix "follow ${follow-fork-mode}, detach ${detach-on-fork}, command \"$cmd\"" {
# Start a new debugger session each time so defaults are legitimate.
if { ![setup] } {
@@ -91,24 +91,24 @@ proc_with_prefix test_follow_fork { who detach cmd } {
gdb_test_no_output "set verbose"
# Set follow-fork-mode if we aren't using the default.
- if {$who == "default"} {
- set who "parent"
+ if {${follow-fork-mode} == "default"} {
+ set follow-fork-mode "parent"
} else {
- gdb_test_no_output "set follow-fork $who"
+ gdb_test_no_output "set follow-fork ${follow-fork-mode}"
}
gdb_test "show follow-fork" \
- "Debugger response to a program call of fork or vfork is \"$who\"."
+ "Debugger response to a program call of fork or vfork is \"${follow-fork-mode}\"."
# Set detach-on-fork mode if we aren't using the default.
- if {$detach == "default"} {
- set detach "on"
+ if {${detach-on-fork} == "default"} {
+ set detach-on-fork "on"
} else {
- gdb_test_no_output "set detach-on-fork $detach"
+ gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
}
gdb_test "show detach-on-fork" \
- "Whether gdb will detach.* fork is $detach."
+ "Whether gdb will detach.* fork is ${detach-on-fork}."
# Set a breakpoint after the fork if we aren't single-stepping
# past the fork.
@@ -121,13 +121,13 @@ proc_with_prefix test_follow_fork { who detach cmd } {
# Set up the output we expect to see after we run.
set expected_re ""
- if {$who == "child"} {
+ if {${follow-fork-mode} == "child"} {
set expected_re "\\\[Attaching after.* fork to.*"
- if {$detach == "on"} {
+ if {${detach-on-fork} == "on"} {
append expected_re "\\\[Detaching after fork from .*"
}
append expected_re "set breakpoint here.*"
- } elseif {$who == "parent" && $detach == "on"} {
+ } elseif {${follow-fork-mode} == "parent" && ${detach-on-fork} == "on"} {
set expected_re "\\\[Detaching after fork from .*set breakpoint here.*"
} else {
set expected_re ".*set breakpoint here.*"
@@ -140,12 +140,12 @@ proc_with_prefix test_follow_fork { who detach cmd } {
# Check that we have the inferiors arranged correctly after
# following the fork.
set resume_unfollowed 0
- if {$who == "parent" && $detach == "on"} {
+ if {${follow-fork-mode} == "parent" && ${detach-on-fork} == "on"} {
# Follow parent / detach child: the only inferior is the parent.
gdb_test "info inferiors" "\\* 1 .* process.*"
- } elseif {$who == "parent" && $detach == "off"} {
+ } elseif {${follow-fork-mode} == "parent" && ${detach-on-fork} == "off"} {
# Follow parent / keep child: two inferiors under debug, the
# parent is the current inferior.
@@ -154,14 +154,14 @@ proc_with_prefix test_follow_fork { who detach cmd } {
gdb_test "inferior 2" "Switching to inferior 2 .*"
set resume_unfollowed 1
- } elseif {$who == "child" && $detach == "on"} {
+ } elseif {${follow-fork-mode} == "child" && ${detach-on-fork} == "on"} {
# Follow child / detach parent: the child is under debug and is
# the current inferior. The parent is listed but is not under
# debug.
gdb_test "info inferiors" " 1 .*<null>.*\\* 2 .*process.*"
- } elseif {$who == "child" && $detach == "off"} {
+ } elseif {${follow-fork-mode} == "child" && ${detach-on-fork} == "off"} {
# Follow child / keep parent: two inferiors under debug, the
# child is the current inferior.
@@ -400,10 +400,10 @@ foreach cmd {"next 2" "continue"} {
}
# Now test all explicit permutations.
-foreach who {"parent" "child"} {
- foreach detach {"on" "off"} {
+foreach follow-fork-mode {"parent" "child"} {
+ foreach detach-on-fork {"on" "off"} {
foreach cmd {"next 2" "continue"} {
- test_follow_fork $who $detach $cmd
+ test_follow_fork ${follow-fork-mode} ${detach-on-fork} $cmd
}
}
}
--
2.33.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 5/6] gdb.base/foll-fork.exp: use foreach_with_prefix to handle prefixes
2021-09-10 20:53 [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Simon Marchi
` (2 preceding siblings ...)
2021-09-10 20:54 ` [PATCH 4/6] gdb.base/foll-fork.exp: rename variables Simon Marchi
@ 2021-09-10 20:54 ` Simon Marchi
2021-09-10 20:54 ` [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child" Simon Marchi
2021-09-23 19:23 ` [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Pedro Alves
5 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-10 20:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
No behavior change in the test expected, other than in the test names.
Change-Id: I111137483858ab0f23138439f2930009779a2b3d
---
gdb/testsuite/gdb.base/foll-fork.exp | 165 +++++++++++++--------------
1 file changed, 81 insertions(+), 84 deletions(-)
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 3b90bb2e785..3a0cc2fe456 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -79,109 +79,106 @@ proc_with_prefix test_follow_fork { follow-fork-mode detach-on-fork cmd } {
global srcfile
global testfile
- with_test_prefix "follow ${follow-fork-mode}, detach ${detach-on-fork}, command \"$cmd\"" {
-
- # Start a new debugger session each time so defaults are legitimate.
- if { ![setup] } {
- return
- }
+ # Start a new debugger session each time so defaults are legitimate.
+ if { ![setup] } {
+ return
+ }
- # The "Detaching..." and "Attaching..." messages may be hidden by
- # default.
- gdb_test_no_output "set verbose"
+ # The "Detaching..." and "Attaching..." messages may be hidden by
+ # default.
+ gdb_test_no_output "set verbose"
- # Set follow-fork-mode if we aren't using the default.
- if {${follow-fork-mode} == "default"} {
- set follow-fork-mode "parent"
- } else {
- gdb_test_no_output "set follow-fork ${follow-fork-mode}"
- }
+ # Set follow-fork-mode if we aren't using the default.
+ if {${follow-fork-mode} == "default"} {
+ set follow-fork-mode "parent"
+ } else {
+ gdb_test_no_output "set follow-fork ${follow-fork-mode}"
+ }
- gdb_test "show follow-fork" \
- "Debugger response to a program call of fork or vfork is \"${follow-fork-mode}\"."
+ gdb_test "show follow-fork" \
+ "Debugger response to a program call of fork or vfork is \"${follow-fork-mode}\"."
- # Set detach-on-fork mode if we aren't using the default.
- if {${detach-on-fork} == "default"} {
- set detach-on-fork "on"
- } else {
- gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
- }
+ # Set detach-on-fork mode if we aren't using the default.
+ if {${detach-on-fork} == "default"} {
+ set detach-on-fork "on"
+ } else {
+ gdb_test_no_output "set detach-on-fork ${detach-on-fork}"
+ }
- gdb_test "show detach-on-fork" \
- "Whether gdb will detach.* fork is ${detach-on-fork}."
+ gdb_test "show detach-on-fork" \
+ "Whether gdb will detach.* fork is ${detach-on-fork}."
- # Set a breakpoint after the fork if we aren't single-stepping
- # past the fork.
- if {$cmd == "continue"} {
- set bp_after_fork [gdb_get_line_number "set breakpoint here"]
- gdb_test "break ${srcfile}:$bp_after_fork" \
- "Breakpoint.*, line $bp_after_fork.*" \
- "set breakpoint after fork"
- }
+ # Set a breakpoint after the fork if we aren't single-stepping
+ # past the fork.
+ if {$cmd == "continue"} {
+ set bp_after_fork [gdb_get_line_number "set breakpoint here"]
+ gdb_test "break ${srcfile}:$bp_after_fork" \
+ "Breakpoint.*, line $bp_after_fork.*" \
+ "set breakpoint after fork"
+ }
- # Set up the output we expect to see after we run.
- set expected_re ""
- if {${follow-fork-mode} == "child"} {
- set expected_re "\\\[Attaching after.* fork to.*"
- if {${detach-on-fork} == "on"} {
- append expected_re "\\\[Detaching after fork from .*"
- }
- append expected_re "set breakpoint here.*"
- } elseif {${follow-fork-mode} == "parent" && ${detach-on-fork} == "on"} {
- set expected_re "\\\[Detaching after fork from .*set breakpoint here.*"
- } else {
- set expected_re ".*set breakpoint here.*"
+ # Set up the output we expect to see after we run.
+ set expected_re ""
+ if {${follow-fork-mode} == "child"} {
+ set expected_re "\\\[Attaching after.* fork to.*"
+ if {${detach-on-fork} == "on"} {
+ append expected_re "\\\[Detaching after fork from .*"
}
+ append expected_re "set breakpoint here.*"
+ } elseif {${follow-fork-mode} == "parent" && ${detach-on-fork} == "on"} {
+ set expected_re "\\\[Detaching after fork from .*set breakpoint here.*"
+ } else {
+ set expected_re ".*set breakpoint here.*"
+ }
- # Test running past and following the fork, using the parameters
- # set above.
- gdb_test $cmd $expected_re "$cmd past fork"
+ # Test running past and following the fork, using the parameters
+ # set above.
+ gdb_test $cmd $expected_re "$cmd past fork"
- # Check that we have the inferiors arranged correctly after
- # following the fork.
- set resume_unfollowed 0
- if {${follow-fork-mode} == "parent" && ${detach-on-fork} == "on"} {
+ # Check that we have the inferiors arranged correctly after
+ # following the fork.
+ set resume_unfollowed 0
+ if {${follow-fork-mode} == "parent" && ${detach-on-fork} == "on"} {
- # Follow parent / detach child: the only inferior is the parent.
- gdb_test "info inferiors" "\\* 1 .* process.*"
+ # Follow parent / detach child: the only inferior is the parent.
+ gdb_test "info inferiors" "\\* 1 .* process.*"
- } elseif {${follow-fork-mode} == "parent" && ${detach-on-fork} == "off"} {
+ } elseif {${follow-fork-mode} == "parent" && ${detach-on-fork} == "off"} {
- # Follow parent / keep child: two inferiors under debug, the
- # parent is the current inferior.
- gdb_test "info inferiors" "\\* 1 .*process.* 2 .*process.*"
+ # Follow parent / keep child: two inferiors under debug, the
+ # parent is the current inferior.
+ gdb_test "info inferiors" "\\* 1 .*process.* 2 .*process.*"
- gdb_test "inferior 2" "Switching to inferior 2 .*"
- set resume_unfollowed 1
+ gdb_test "inferior 2" "Switching to inferior 2 .*"
+ set resume_unfollowed 1
- } elseif {${follow-fork-mode} == "child" && ${detach-on-fork} == "on"} {
+ } elseif {${follow-fork-mode} == "child" && ${detach-on-fork} == "on"} {
- # Follow child / detach parent: the child is under debug and is
- # the current inferior. The parent is listed but is not under
- # debug.
- gdb_test "info inferiors" " 1 .*<null>.*\\* 2 .*process.*"
+ # Follow child / detach parent: the child is under debug and is
+ # the current inferior. The parent is listed but is not under
+ # debug.
+ gdb_test "info inferiors" " 1 .*<null>.*\\* 2 .*process.*"
- } elseif {${follow-fork-mode} == "child" && ${detach-on-fork} == "off"} {
+ } elseif {${follow-fork-mode} == "child" && ${detach-on-fork} == "off"} {
- # Follow child / keep parent: two inferiors under debug, the
- # child is the current inferior.
- gdb_test "info inferiors" " 1 .*process.*\\* 2 .*process.*"
+ # Follow child / keep parent: two inferiors under debug, the
+ # child is the current inferior.
+ gdb_test "info inferiors" " 1 .*process.*\\* 2 .*process.*"
- gdb_test "inferior 1" "Switching to inferior 1 .*"
- set resume_unfollowed 1
- }
+ gdb_test "inferior 1" "Switching to inferior 1 .*"
+ set resume_unfollowed 1
+ }
- if {$resume_unfollowed == 1} {
- if {$cmd == "next 2"} {
+ if {$resume_unfollowed == 1} {
+ if {$cmd == "next 2"} {
- gdb_continue_to_end "continue unfollowed inferior to end"
+ gdb_continue_to_end "continue unfollowed inferior to end"
- } elseif {$cmd == "continue"} {
+ } elseif {$cmd == "continue"} {
- gdb_continue_to_breakpoint \
- "continue unfollowed inferior to bp" \
- ".* set breakpoint here.*"
- }
+ gdb_continue_to_breakpoint \
+ "continue unfollowed inferior to bp" \
+ ".* set breakpoint here.*"
}
}
}
@@ -395,14 +392,14 @@ if { ![check_fork_catchpoints] } {
# The first loop should be sufficient to test the defaults. There
# is no need to test using the defaults in other permutations (e.g.
# "default" "on", "parent" "default", etc.).
-foreach cmd {"next 2" "continue"} {
+foreach_with_prefix cmd {"next 2" "continue"} {
test_follow_fork "default" "default" $cmd
}
# Now test all explicit permutations.
-foreach follow-fork-mode {"parent" "child"} {
- foreach detach-on-fork {"on" "off"} {
- foreach cmd {"next 2" "continue"} {
+foreach_with_prefix follow-fork-mode {"parent" "child"} {
+ foreach_with_prefix detach-on-fork {"on" "off"} {
+ foreach_with_prefix cmd {"next 2" "continue"} {
test_follow_fork ${follow-fork-mode} ${detach-on-fork} $cmd
}
}
--
2.33.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-10 20:53 [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Simon Marchi
` (3 preceding siblings ...)
2021-09-10 20:54 ` [PATCH 5/6] gdb.base/foll-fork.exp: use foreach_with_prefix to handle prefixes Simon Marchi
@ 2021-09-10 20:54 ` Simon Marchi
2021-09-10 23:33 ` John Baldwin
2021-09-23 19:23 ` [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Pedro Alves
5 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-09-10 20:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
We found that when handling forks, two inferiors can unexpectedly share
their program space and address space. To reproduce:
1. Using a test program that forks...
2. "set follow-fork-mode child"
3. "set detach-on-fork on" (the default)
4. run to a breakpoint somewhere after the fork
Step 4 should have created a new inferior:
(gdb) info inferiors
Num Description Connection Executable
1 <null> /home/smarchi/build/wt/amd/gdb/fork
* 2 process 251425 1 (native) /home/smarchi/build/wt/amd/gdb/fork
By inspecting the state of GDB, we can see that the two inferiors now
share one program space and one address space:
Inferior 1:
(top-gdb) p inferior_list.m_front.num
$2 = 1
(top-gdb) p inferior_list.m_front.aspace
$3 = (struct address_space *) 0x5595e2520400
(top-gdb) p inferior_list.m_front.pspace
$4 = (struct program_space *) 0x5595e2520440
Inferior 2:
(top-gdb) p inferior_list.m_front.next.num
$5 = 2
(top-gdb) p inferior_list.m_front.next.aspace
$6 = (struct address_space *) 0x5595e2520400
(top-gdb) p inferior_list.m_front.next.pspace
$7 = (struct program_space *) 0x5595e2520440
You can then run inferior 1 again and the two inferiors will still
erroneously share their spaces, but already at this point this is wrong.
The cause of the bad {a,p}space sharing is in follow_fork_inferior.
When following the child and detaching from the parent, we just re-use
the parent's spaces, rather than cloning them. When we switch back to
inferior 1 and run again, we find ourselves with two unrelated inferiors
sharing spaces.
Fix that by creating new spaces for the parent after having moved them
to the child. My initial implementation created new spaces for the
child instead. Doing this breaks doing "next" over fork(). When "next"
start, we record the symtab of the starting location. When the program
stops, we compare that symtab with the symtab the program has stopped
at. If the symtab or the line number has changed, we conclude the
"next" is done. If we create a new program space for the child and copy
the parent's program space to it with clone_program_space, it creates
new symtabs for the child as well. When the child stop, but still on
the fork() line, GDB thinks the "next" is done because the symtab
pointers no longer match. In reality they are two symtab instances that
represent the same file. But moving the spaces to the child and
creating new spaces for the parent, we avoid this problem.
Note that the problem described above happens today with "detach-on-fork
off" and "follow-fork-mode child", because we create new spaces for the
child. This will have to be addressed later.
Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
expected to have a location in each inferiors. Without the fix, when
the two inferiors erroneously share a program space, GDB reports a
single location.
Change-Id: Ifea76e14f87b9f7321fc3a766217061190e71c6e
---
gdb/infrun.c | 39 +++++++++++++++++++++-------
gdb/testsuite/gdb.base/foll-fork.exp | 18 +++++++++++++
2 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d1ac9b4cbbb..b80884322fa 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -539,25 +539,46 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->gdbarch = parent_inf->gdbarch;
copy_inferior_target_desc_info (child_inf, parent_inf);
- program_space *parent_pspace = parent_inf->pspace;
-
- /* If this is a vfork child, then the address-space is shared
- with the parent. If we detached from the parent, then we can
- reuse the parent's program/address spaces. */
- if (has_vforked || detach_fork)
+ if (has_vforked)
{
- child_inf->pspace = parent_pspace;
- child_inf->aspace = child_inf->pspace->aspace;
+ /* If this is a vfork child, then the address-space is shared
+ with the parent. */
+ child_inf->aspace = parent_inf->aspace;
+ child_inf->pspace = parent_inf->pspace;
exec_on_vfork (child_inf);
}
+ else if (detach_fork)
+ {
+ /* We follow the child and detach from the parent: move the parent's
+ program space to the child. This simplifies some things, like
+ doing "next" over fork() and landing on the expected line in the
+ child (note, that is broken with "set detach-on-fork off").
+
+ Before assigning brand new spaces for the parent, remove
+ breakpoints from it: because the new pspace won't match
+ currently inserted locations, the normal detach procedure
+ wouldn't remove them, and we would leave them inserted when
+ detaching. */
+ remove_breakpoints_inf (parent_inf);
+
+ child_inf->aspace = parent_inf->aspace;
+ child_inf->pspace = parent_inf->pspace;
+ parent_inf->aspace = new_address_space ();
+ parent_inf->pspace = new program_space (parent_inf->aspace);
+ clone_program_space (parent_inf->pspace, child_inf->pspace);
+
+ /* The parent inferior is still the current one, so keep things
+ in sync. */
+ set_current_program_space (parent_inf->pspace);
+ }
else
{
child_inf->aspace = new_address_space ();
child_inf->pspace = new program_space (child_inf->aspace);
child_inf->removable = 1;
child_inf->symfile_flags = SYMFILE_NO_READ;
- clone_program_space (child_inf->pspace, parent_pspace);
+ clone_program_space (child_inf->pspace, parent_inf->pspace);
}
}
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 3a0cc2fe456..7f9e1cf87c6 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -181,6 +181,24 @@ proc_with_prefix test_follow_fork { follow-fork-mode detach-on-fork cmd } {
".* set breakpoint here.*"
}
}
+
+ # If we end up with two inferiors, verify that they each end up with their
+ # own program space. Do this by setting a breakpoint, if we see two
+ # locations it means there are two program spaces.
+ if {${detach-on-fork} == "off" || ${follow-fork-mode} == "child"} {
+ set bpnum "<unset>"
+ gdb_test_multiple "break callee" "break callee" {
+ -re -wrap "Breakpoint ($::decimal) at $::hex: callee\\. \\(2 locations\\)" {
+ set bpnum $expect_out(1,string)
+ pass $gdb_test_name
+ }
+ }
+ gdb_test "info breakpoints $bpnum" \
+ [multi_line \
+ "$bpnum\\.1 .* inf 1" \
+ "$bpnum\\.2 .* inf 2"] \
+ "info breakpoints"
+ }
}
set reading_in_symbols_re {(?:\r\nReading in symbols for [^\r\n]*)?}
--
2.33.0
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-10 20:54 ` [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child" Simon Marchi
@ 2021-09-10 23:33 ` John Baldwin
2021-09-11 3:16 ` Simon Marchi
0 siblings, 1 reply; 18+ messages in thread
From: John Baldwin @ 2021-09-10 23:33 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 9/10/21 1:54 PM, Simon Marchi via Gdb-patches wrote:
> We found that when handling forks, two inferiors can unexpectedly share
> their program space and address space. To reproduce:
>
> 1. Using a test program that forks...
> 2. "set follow-fork-mode child"
> 3. "set detach-on-fork on" (the default)
> 4. run to a breakpoint somewhere after the fork
>
> Step 4 should have created a new inferior:
>
> (gdb) info inferiors
> Num Description Connection Executable
> 1 <null> /home/smarchi/build/wt/amd/gdb/fork
> * 2 process 251425 1 (native) /home/smarchi/build/wt/amd/gdb/fork
>
> By inspecting the state of GDB, we can see that the two inferiors now
> share one program space and one address space:
>
> Inferior 1:
>
> (top-gdb) p inferior_list.m_front.num
> $2 = 1
> (top-gdb) p inferior_list.m_front.aspace
> $3 = (struct address_space *) 0x5595e2520400
> (top-gdb) p inferior_list.m_front.pspace
> $4 = (struct program_space *) 0x5595e2520440
>
> Inferior 2:
>
> (top-gdb) p inferior_list.m_front.next.num
> $5 = 2
> (top-gdb) p inferior_list.m_front.next.aspace
> $6 = (struct address_space *) 0x5595e2520400
> (top-gdb) p inferior_list.m_front.next.pspace
> $7 = (struct program_space *) 0x5595e2520440
>
> You can then run inferior 1 again and the two inferiors will still
> erroneously share their spaces, but already at this point this is wrong.
>
> The cause of the bad {a,p}space sharing is in follow_fork_inferior.
> When following the child and detaching from the parent, we just re-use
> the parent's spaces, rather than cloning them. When we switch back to
> inferior 1 and run again, we find ourselves with two unrelated inferiors
> sharing spaces.
>
> Fix that by creating new spaces for the parent after having moved them
> to the child. My initial implementation created new spaces for the
> child instead. Doing this breaks doing "next" over fork(). When "next"
> start, we record the symtab of the starting location. When the program
> stops, we compare that symtab with the symtab the program has stopped
> at. If the symtab or the line number has changed, we conclude the
> "next" is done. If we create a new program space for the child and copy
> the parent's program space to it with clone_program_space, it creates
> new symtabs for the child as well. When the child stop, but still on
> the fork() line, GDB thinks the "next" is done because the symtab
> pointers no longer match. In reality they are two symtab instances that
> represent the same file. But moving the spaces to the child and
> creating new spaces for the parent, we avoid this problem.
>
> Note that the problem described above happens today with "detach-on-fork
> off" and "follow-fork-mode child", because we create new spaces for the
> child. This will have to be addressed later.
>
> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
> expected to have a location in each inferiors. Without the fix, when
> the two inferiors erroneously share a program space, GDB reports a
> single location.
So I wonder about the case where follow-fork-mode is parent and
detach-on-fork is off? In that case, should the existing aspace/pspace
stay with the parent and the child get clones? That is, using the
follow-fork-mode setting to determine which inferior gets the existing
aspace/pspace and assigning the cloned copies to the !follow-fork-mode
inferior?
--
John Baldwin
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-10 23:33 ` John Baldwin
@ 2021-09-11 3:16 ` Simon Marchi
2021-09-11 13:02 ` Simon Marchi
2021-09-27 19:32 ` Simon Marchi
0 siblings, 2 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-11 3:16 UTC (permalink / raw)
To: John Baldwin, Simon Marchi, gdb-patches
>> Note that the problem described above happens today with "detach-on-fork
>> off" and "follow-fork-mode child", because we create new spaces for the
>> child. This will have to be addressed later.
>>
>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>> expected to have a location in each inferiors. Without the fix, when
>> the two inferiors erroneously share a program space, GDB reports a
>> single location.
>
> So I wonder about the case where follow-fork-mode is parent and
> detach-on-fork is off? In that case, should the existing aspace/pspace
> stay with the parent and the child get clones? That is, using the
> follow-fork-mode setting to determine which inferior gets the existing
> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
> inferior?
I think that would work, to address the problem described above.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-11 3:16 ` Simon Marchi
@ 2021-09-11 13:02 ` Simon Marchi
2021-09-11 13:03 ` Simon Marchi
2021-09-27 19:32 ` Simon Marchi
1 sibling, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-09-11 13:02 UTC (permalink / raw)
To: John Baldwin, Simon Marchi, gdb-patches
On 2021-09-10 11:16 p.m., Simon Marchi wrote:
>>> Note that the problem described above happens today with "detach-on-fork
>>> off" and "follow-fork-mode child", because we create new spaces for the
>>> child. This will have to be addressed later.
>>>
>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>>> expected to have a location in each inferiors. Without the fix, when
>>> the two inferiors erroneously share a program space, GDB reports a
>>> single location.
>>
>> So I wonder about the case where follow-fork-mode is parent and
>> detach-on-fork is off? In that case, should the existing aspace/pspace
>> stay with the parent and the child get clones? That is, using the
>> follow-fork-mode setting to determine which inferior gets the existing
>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
>> inferior?
>
> I think that would work, to address the problem described above.
>
> Simon
>
Btw, I'm off until the 20th.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-11 13:02 ` Simon Marchi
@ 2021-09-11 13:03 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-11 13:03 UTC (permalink / raw)
To: John Baldwin, Simon Marchi, gdb-patches
On 2021-09-11 9:02 a.m., Simon Marchi via Gdb-patches wrote:
>
>
> On 2021-09-10 11:16 p.m., Simon Marchi wrote:
>>>> Note that the problem described above happens today with "detach-on-fork
>>>> off" and "follow-fork-mode child", because we create new spaces for the
>>>> child. This will have to be addressed later.
>>>>
>>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>>>> expected to have a location in each inferiors. Without the fix, when
>>>> the two inferiors erroneously share a program space, GDB reports a
>>>> single location.
>>>
>>> So I wonder about the case where follow-fork-mode is parent and
>>> detach-on-fork is off? In that case, should the existing aspace/pspace
>>> stay with the parent and the child get clones? That is, using the
>>> follow-fork-mode setting to determine which inferior gets the existing
>>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
>>> inferior?
>>
>> I think that would work, to address the problem described above.
>>
>> Simon
>>
>
> Btw, I'm off until the 20th.
>
> Simon
>
Haha oops, replied to the wrong message. Now everybody knows I guess :).
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-11 3:16 ` Simon Marchi
2021-09-11 13:02 ` Simon Marchi
@ 2021-09-27 19:32 ` Simon Marchi
2021-09-28 15:10 ` Tom de Vries
1 sibling, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2021-09-27 19:32 UTC (permalink / raw)
To: John Baldwin, Simon Marchi, gdb-patches
On 2021-09-10 23:16, Simon Marchi via Gdb-patches wrote:
>>> Note that the problem described above happens today with "detach-on-fork
>>> off" and "follow-fork-mode child", because we create new spaces for the
>>> child. This will have to be addressed later.
>>>
>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>>> expected to have a location in each inferiors. Without the fix, when
>>> the two inferiors erroneously share a program space, GDB reports a
>>> single location.
>>
>> So I wonder about the case where follow-fork-mode is parent and
>> detach-on-fork is off? In that case, should the existing aspace/pspace
>> stay with the parent and the child get clones? That is, using the
>> follow-fork-mode setting to determine which inferior gets the existing
>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
>> inferior?
>
> I think that would work, to address the problem described above.
FYI, I tried doing the above (giving the original program space to the
child with "follow-fork-mode child" and "detach-on-fork off") and I hit
some problems. Attached to the program space is the list of shared
libraries. So GDB would now think the parent has no shared library. I
added a solib_create_inferior_hook call to force re-discovering the
shared libraries in the parent. That seemed to work, but then in MI
there were spurious =library-loaded events that made it look like
inferior 1 re-loaded shared libraries. I'm also worried about what
other per-program space is kept, that could cause some kind of trouble
for the parent. So I will put this off for now, as it's not something
that should be done in a rush, I think.
In the mean time, I'll re-test this series and push it if all seems
good.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-27 19:32 ` Simon Marchi
@ 2021-09-28 15:10 ` Tom de Vries
2021-09-28 19:12 ` Simon Marchi
0 siblings, 1 reply; 18+ messages in thread
From: Tom de Vries @ 2021-09-28 15:10 UTC (permalink / raw)
To: Simon Marchi, John Baldwin, Simon Marchi, gdb-patches
On 9/27/21 9:32 PM, Simon Marchi via Gdb-patches wrote:
> On 2021-09-10 23:16, Simon Marchi via Gdb-patches wrote:
>>>> Note that the problem described above happens today with "detach-on-fork
>>>> off" and "follow-fork-mode child", because we create new spaces for the
>>>> child. This will have to be addressed later.
>>>>
>>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>>>> expected to have a location in each inferiors. Without the fix, when
>>>> the two inferiors erroneously share a program space, GDB reports a
>>>> single location.
>>>
>>> So I wonder about the case where follow-fork-mode is parent and
>>> detach-on-fork is off? In that case, should the existing aspace/pspace
>>> stay with the parent and the child get clones? That is, using the
>>> follow-fork-mode setting to determine which inferior gets the existing
>>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
>>> inferior?
>>
>> I think that would work, to address the problem described above.
>
> FYI, I tried doing the above (giving the original program space to the
> child with "follow-fork-mode child" and "detach-on-fork off") and I hit
> some problems. Attached to the program space is the list of shared
> libraries. So GDB would now think the parent has no shared library. I
> added a solib_create_inferior_hook call to force re-discovering the
> shared libraries in the parent. That seemed to work, but then in MI
> there were spurious =library-loaded events that made it look like
> inferior 1 re-loaded shared libraries. I'm also worried about what
> other per-program space is kept, that could cause some kind of trouble
> for the parent. So I will put this off for now, as it's not something
> that should be done in a rush, I think.
>
> In the mean time, I'll re-test this series and push it if all seems
> good.
I'm seeing:
...
(gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=child:
detach-on-fork=on: cmd=next 2: test_follow_fork: break callee
info breakpoints 2^M
Num Type Disp Enb Address What^M
2 breakpoint keep y <MULTIPLE> ^M
2.1 y 0x00000000004005ae in callee at
/home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9
inf 2^M
2.2 y 0x00000000004005ae in callee at
/home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9
inf 1^M
(gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=child:
detach-on-fork=on: cmd=next 2: test_follow_fork: info breakpoints
...
Looks like the test-case expects first inf 1, then inf 2, and this is
the other way around.
Should the test-case regexp be updated?
Thanks,
- Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-28 15:10 ` Tom de Vries
@ 2021-09-28 19:12 ` Simon Marchi
2021-09-28 19:31 ` Pedro Alves
2021-09-28 22:38 ` Tom de Vries
0 siblings, 2 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-28 19:12 UTC (permalink / raw)
To: Tom de Vries, Simon Marchi, John Baldwin, gdb-patches
On 2021-09-28 11:10, Tom de Vries wrote:
> On 9/27/21 9:32 PM, Simon Marchi via Gdb-patches wrote:
>> On 2021-09-10 23:16, Simon Marchi via Gdb-patches wrote:
>>>>> Note that the problem described above happens today with "detach-on-fork
>>>>> off" and "follow-fork-mode child", because we create new spaces for the
>>>>> child. This will have to be addressed later.
>>>>>
>>>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>>>>> expected to have a location in each inferiors. Without the fix, when
>>>>> the two inferiors erroneously share a program space, GDB reports a
>>>>> single location.
>>>>
>>>> So I wonder about the case where follow-fork-mode is parent and
>>>> detach-on-fork is off? In that case, should the existing aspace/pspace
>>>> stay with the parent and the child get clones? That is, using the
>>>> follow-fork-mode setting to determine which inferior gets the existing
>>>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
>>>> inferior?
>>>
>>> I think that would work, to address the problem described above.
>>
>> FYI, I tried doing the above (giving the original program space to the
>> child with "follow-fork-mode child" and "detach-on-fork off") and I hit
>> some problems. Attached to the program space is the list of shared
>> libraries. So GDB would now think the parent has no shared library. I
>> added a solib_create_inferior_hook call to force re-discovering the
>> shared libraries in the parent. That seemed to work, but then in MI
>> there were spurious =library-loaded events that made it look like
>> inferior 1 re-loaded shared libraries. I'm also worried about what
>> other per-program space is kept, that could cause some kind of trouble
>> for the parent. So I will put this off for now, as it's not something
>> that should be done in a rush, I think.
>>
>> In the mean time, I'll re-test this series and push it if all seems
>> good.
>
> I'm seeing:
> ...
> (gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=child:
> detach-on-fork=on: cmd=next 2: test_follow_fork: break callee
> info breakpoints 2^M
> Num Type Disp Enb Address What^M
> 2 breakpoint keep y <MULTIPLE> ^M
> 2.1 y 0x00000000004005ae in callee at
> /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9
> inf 2^M
> 2.2 y 0x00000000004005ae in callee at
> /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9
> inf 1^M
> (gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=child:
> detach-on-fork=on: cmd=next 2: test_follow_fork: info breakpoints
> ...
>
> Looks like the test-case expects first inf 1, then inf 2, and this is
> the other way around.
>
> Should the test-case regexp be updated?
Huh, I remember that have encountered this, and thought I had changed
the regexp to allow both orders. I must have lost that change. But it
still bugged me, why did I see one order sometimes and the other at
other times.
On my home computer (Arch Linux) I see "2, 1" and on my work computer
(Ubuntu 20.04) I see "1, 2". So that lets me compare both cases. The
order comes from the SALs passed to init_breakpoint_sal. It comes from
the compare_symbols function here:
https://gitlab.com/gnutools/binutils-gdb/-/blob/3a6a0158ee07ba2f960ae4a898897460382dc5ec/gdb/linespec.c#L3568-3592
It compares the value of program_space pointers to determine the order
of symbols, so that can explain the difference between two machines.
That then decides the order of the locations in the breakpoint::loc
list, since add_location_to_breakpoint only sorts by address. So
depending on which location is added first, it will give different
orders.
I think that the order of locations should be as stable as possible. I
started writing a patch for that, but then realized that even then, the
order in the test could vary, based on the architecture, whether you use
PIE or not, etc. And in the end the order doesn't really matter, what
is important is that we have two locations.
Having a more stable breakpoint location order would be nice, but not as
a way to fix this test failure.
So, what do you think of the patch below? The regexp accepts (1|2) for
both lines. If you have a simple way to say "either 1 then 2 or 2 then
1" (so that it rejects 1 then 1 or 2 then 2), I would take it, but
otherwise I think this is sufficient.
From 38f41f929f46926fede29c70064871fc23e0e215 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Tue, 28 Sep 2021 13:22:53 -0400
Subject: [PATCH] gdb.base/foll-fork.exp: accept "info breakpoints" output in
any order
The test currently requires the "inf 1" breakpoint to be before the "inf
2" breakpoint. This is not always the case:
info breakpoints 2
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE>
2.1 y 0x0000555555554730 in callee at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:9 inf 2
2.2 y 0x0000555555554730 in callee at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:9 inf 1
(gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=parent: detach-on-fork=off: cmd=next 2: test_follow_fork: info breakpoints
Since add_location_to_breakpoint uses only the address as a criterion to
sort locations, the order of locations at the same address is not
stable: it will depend on the insertion order. Here, the insertion
order comes from the order of SALs when creating the breakpoint, which
can vary from machine to machine. While it would be more user-friendly
to have a more stable order for printed breakpoint locations, it doesn't
really matter for this test, and it would be hard to define an order
that will be the same everywhere, all the time.
So, loosen the regexp to not check the "inf 1" vs "inf 2" order.
Change-Id: I5ada2e0c6ad0669e0d161bfb6b767229c0970d16
---
gdb/testsuite/gdb.base/foll-fork.exp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 7f9e1cf87c6a..c0beb089d451 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -195,8 +195,8 @@ proc_with_prefix test_follow_fork { follow-fork-mode detach-on-fork cmd } {
}
gdb_test "info breakpoints $bpnum" \
[multi_line \
- "$bpnum\\.1 .* inf 1" \
- "$bpnum\\.2 .* inf 2"] \
+ "$bpnum\\.1 .* inf (1|2)" \
+ "$bpnum\\.2 .* inf (1|2)"] \
"info breakpoints"
}
}
--
2.33.0
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-28 19:12 ` Simon Marchi
@ 2021-09-28 19:31 ` Pedro Alves
2021-09-28 19:35 ` Pedro Alves
2021-09-28 22:38 ` Tom de Vries
1 sibling, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2021-09-28 19:31 UTC (permalink / raw)
To: Simon Marchi, Tom de Vries, Simon Marchi, John Baldwin, gdb-patches
On 2021-09-28 8:12 p.m., Simon Marchi via Gdb-patches wrote:
>
> So, what do you think of the patch below? The regexp accepts (1|2) for
> both lines. If you have a simple way to say "either 1 then 2 or 2 then
> 1" (so that it rejects 1 then 1 or 2 then 2), I would take it, but
> otherwise I think this is sufficient.
There's always plain alternatives. Something like:
set any "\[^\r\n\]*"
set loc1_inf1 "$bpnum\\.1 $any inf 1\r\n$bpnum\\.2 $any inf 1"
set loc1_inf2 "$bpnum\\.1 $any inf 1\r\n$bpnum\\.2 $any inf 2"
set loc2_inf1 "$bpnum\\.2 $any inf 1\r\n$bpnum\\.2 $any inf 1"
set loc2_inf2 "$bpnum\\.2 $any inf 1\r\n$bpnum\\.2 $any inf 2"
gdb_test "info breakpoints $bpnum" \
"($loc1_inf1\r\n$loc2_inf2|$loc1_inf2\r\n$loc2_inf1)" \
"info breakpoints"
Alternatively, something like:
gdb_test_multiple "info breakpoints $bpnum" "info breakpoints" {
-wrap -re "$bpnum\\.1 $any inf ($::decimal)\r\n$bpnum\\.2 $any inf ($::decimal)" {
set inf_a $expect_out(1,string)
set inf_b $expect_out(2,string)
gdb_assert {($inf_a == 1 && $inf_b == 2) \
|| ($inf_a == 2 && $inf_b == 1)} \
$gdb_test_name
}
}
Completely untested, written in email client...
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-28 19:31 ` Pedro Alves
@ 2021-09-28 19:35 ` Pedro Alves
2021-09-28 23:32 ` Simon Marchi
0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2021-09-28 19:35 UTC (permalink / raw)
To: Simon Marchi, Tom de Vries, Simon Marchi, John Baldwin, gdb-patches
On 2021-09-28 8:31 p.m., Pedro Alves wrote:
> set loc1_inf1 "$bpnum\\.1 $any inf 1\r\n$bpnum\\.2 $any inf 1"
> set loc1_inf2 "$bpnum\\.1 $any inf 1\r\n$bpnum\\.2 $any inf 2"
>
> set loc2_inf1 "$bpnum\\.2 $any inf 1\r\n$bpnum\\.2 $any inf 1"
> set loc2_inf2 "$bpnum\\.2 $any inf 1\r\n$bpnum\\.2 $any inf 2"
>
> gdb_test "info breakpoints $bpnum" \
> "($loc1_inf1\r\n$loc2_inf2|$loc1_inf2\r\n$loc2_inf1)" \
> "info breakpoints"
I meant instead:
set loc1_inf1 "$bpnum\\.1 $any inf 1"
set loc1_inf2 "$bpnum\\.1 $any inf 2"
set loc2_inf1 "$bpnum\\.2 $any inf 1"
set loc2_inf2 "$bpnum\\.2 $any inf 2"
gdb_test "info breakpoints $bpnum" \
"($loc1_inf1\r\n$loc2_inf2|$loc1_inf2\r\n$loc2_inf1)" \
"info breakpoints"
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-28 19:35 ` Pedro Alves
@ 2021-09-28 23:32 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2021-09-28 23:32 UTC (permalink / raw)
To: Pedro Alves, Simon Marchi, Tom de Vries, John Baldwin, gdb-patches
On 2021-09-28 15:35, Pedro Alves wrote:
> On 2021-09-28 8:31 p.m., Pedro Alves wrote:
>> set loc1_inf1 "$bpnum\\.1 $any inf 1\r\n$bpnum\\.2 $any inf 1"
>> set loc1_inf2 "$bpnum\\.1 $any inf 1\r\n$bpnum\\.2 $any inf 2"
>>
>> set loc2_inf1 "$bpnum\\.2 $any inf 1\r\n$bpnum\\.2 $any inf 1"
>> set loc2_inf2 "$bpnum\\.2 $any inf 1\r\n$bpnum\\.2 $any inf 2"
>>
>> gdb_test "info breakpoints $bpnum" \
>> "($loc1_inf1\r\n$loc2_inf2|$loc1_inf2\r\n$loc2_inf1)" \
>> "info breakpoints"
>
> I meant instead:
>
> set loc1_inf1 "$bpnum\\.1 $any inf 1"
> set loc1_inf2 "$bpnum\\.1 $any inf 2"
>
> set loc2_inf1 "$bpnum\\.2 $any inf 1"
> set loc2_inf2 "$bpnum\\.2 $any inf 2"
>
> gdb_test "info breakpoints $bpnum" \
> "($loc1_inf1\r\n$loc2_inf2|$loc1_inf2\r\n$loc2_inf1)" \
> "info breakpoints"
>
Thanks a lot, I pushed a version using that.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child"
2021-09-28 19:12 ` Simon Marchi
2021-09-28 19:31 ` Pedro Alves
@ 2021-09-28 22:38 ` Tom de Vries
1 sibling, 0 replies; 18+ messages in thread
From: Tom de Vries @ 2021-09-28 22:38 UTC (permalink / raw)
To: Simon Marchi, Simon Marchi, John Baldwin, gdb-patches
On 9/28/21 9:12 PM, Simon Marchi wrote:
>
>
> On 2021-09-28 11:10, Tom de Vries wrote:
>> On 9/27/21 9:32 PM, Simon Marchi via Gdb-patches wrote:
>>> On 2021-09-10 23:16, Simon Marchi via Gdb-patches wrote:
>>>>>> Note that the problem described above happens today with "detach-on-fork
>>>>>> off" and "follow-fork-mode child", because we create new spaces for the
>>>>>> child. This will have to be addressed later.
>>>>>>
>>>>>> Test-wise, improve gdb.base/foll-fork.exp to set a breakpoint that is
>>>>>> expected to have a location in each inferiors. Without the fix, when
>>>>>> the two inferiors erroneously share a program space, GDB reports a
>>>>>> single location.
>>>>>
>>>>> So I wonder about the case where follow-fork-mode is parent and
>>>>> detach-on-fork is off? In that case, should the existing aspace/pspace
>>>>> stay with the parent and the child get clones? That is, using the
>>>>> follow-fork-mode setting to determine which inferior gets the existing
>>>>> aspace/pspace and assigning the cloned copies to the !follow-fork-mode
>>>>> inferior?
>>>>
>>>> I think that would work, to address the problem described above.
>>>
>>> FYI, I tried doing the above (giving the original program space to the
>>> child with "follow-fork-mode child" and "detach-on-fork off") and I hit
>>> some problems. Attached to the program space is the list of shared
>>> libraries. So GDB would now think the parent has no shared library. I
>>> added a solib_create_inferior_hook call to force re-discovering the
>>> shared libraries in the parent. That seemed to work, but then in MI
>>> there were spurious =library-loaded events that made it look like
>>> inferior 1 re-loaded shared libraries. I'm also worried about what
>>> other per-program space is kept, that could cause some kind of trouble
>>> for the parent. So I will put this off for now, as it's not something
>>> that should be done in a rush, I think.
>>>
>>> In the mean time, I'll re-test this series and push it if all seems
>>> good.
>>
>> I'm seeing:
>> ...
>> (gdb) PASS: gdb.base/foll-fork.exp: follow-fork-mode=child:
>> detach-on-fork=on: cmd=next 2: test_follow_fork: break callee
>> info breakpoints 2^M
>> Num Type Disp Enb Address What^M
>> 2 breakpoint keep y <MULTIPLE> ^M
>> 2.1 y 0x00000000004005ae in callee at
>> /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9
>> inf 2^M
>> 2.2 y 0x00000000004005ae in callee at
>> /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/foll-fork.c:9
>> inf 1^M
>> (gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=child:
>> detach-on-fork=on: cmd=next 2: test_follow_fork: info breakpoints
>> ...
>>
>> Looks like the test-case expects first inf 1, then inf 2, and this is
>> the other way around.
>>
>> Should the test-case regexp be updated?
>
> Huh, I remember that have encountered this, and thought I had changed
> the regexp to allow both orders. I must have lost that change. But it
> still bugged me, why did I see one order sometimes and the other at
> other times.
>
> On my home computer (Arch Linux) I see "2, 1" and on my work computer
> (Ubuntu 20.04) I see "1, 2". So that lets me compare both cases. The
> order comes from the SALs passed to init_breakpoint_sal. It comes from
> the compare_symbols function here:
>
> https://gitlab.com/gnutools/binutils-gdb/-/blob/3a6a0158ee07ba2f960ae4a898897460382dc5ec/gdb/linespec.c#L3568-3592
>
> It compares the value of program_space pointers to determine the order
> of symbols, so that can explain the difference between two machines.
>
> That then decides the order of the locations in the breakpoint::loc
> list, since add_location_to_breakpoint only sorts by address. So
> depending on which location is added first, it will give different
> orders.
>
Ack.
> I think that the order of locations should be as stable as possible. > I
> started writing a patch for that, but then realized that even then, the
> order in the test could vary, based on the architecture, whether you use
> PIE or not, etc. And in the end the order doesn't really matter, what
> is important is that we have two locations.
>
> Having a more stable breakpoint location order would be nice, but not as
> a way to fix this test failure.
>
Agreed.
> So, what do you think of the patch below? The regexp accepts (1|2) for
> both lines. If you have a simple way to say "either 1 then 2 or 2 then
> 1" (so that it rejects 1 then 1 or 2 then 2), I would take it, but
> otherwise I think this is sufficient.
>
FWIW, I prefer Pedro's gdb_assert variant, which is a bit more strict
thank this one, and takes the or-ing out the regexp domain.
Thanks,
- Tom
> From 38f41f929f46926fede29c70064871fc23e0e215 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Tue, 28 Sep 2021 13:22:53 -0400
> Subject: [PATCH] gdb.base/foll-fork.exp: accept "info breakpoints" output in
> any order
>
> The test currently requires the "inf 1" breakpoint to be before the "inf
> 2" breakpoint. This is not always the case:
>
> info breakpoints 2
> Num Type Disp Enb Address What
> 2 breakpoint keep y <MULTIPLE>
> 2.1 y 0x0000555555554730 in callee at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:9 inf 2
> 2.2 y 0x0000555555554730 in callee at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/foll-fork.c:9 inf 1
> (gdb) FAIL: gdb.base/foll-fork.exp: follow-fork-mode=parent: detach-on-fork=off: cmd=next 2: test_follow_fork: info breakpoints
>
> Since add_location_to_breakpoint uses only the address as a criterion to
> sort locations, the order of locations at the same address is not
> stable: it will depend on the insertion order. Here, the insertion
> order comes from the order of SALs when creating the breakpoint, which
> can vary from machine to machine. While it would be more user-friendly
> to have a more stable order for printed breakpoint locations, it doesn't
> really matter for this test, and it would be hard to define an order
> that will be the same everywhere, all the time.
>
> So, loosen the regexp to not check the "inf 1" vs "inf 2" order.
>
> Change-Id: I5ada2e0c6ad0669e0d161bfb6b767229c0970d16
> ---
> gdb/testsuite/gdb.base/foll-fork.exp | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
> index 7f9e1cf87c6a..c0beb089d451 100644
> --- a/gdb/testsuite/gdb.base/foll-fork.exp
> +++ b/gdb/testsuite/gdb.base/foll-fork.exp
> @@ -195,8 +195,8 @@ proc_with_prefix test_follow_fork { follow-fork-mode detach-on-fork cmd } {
> }
> gdb_test "info breakpoints $bpnum" \
> [multi_line \
> - "$bpnum\\.1 .* inf 1" \
> - "$bpnum\\.2 .* inf 2"] \
> + "$bpnum\\.1 .* inf (1|2)" \
> + "$bpnum\\.2 .* inf (1|2)"] \
> "info breakpoints"
> }
> }
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs
2021-09-10 20:53 [PATCH 1/6] gdb.base/foll-fork.exp: remove DUPLICATEs Simon Marchi
` (4 preceding siblings ...)
2021-09-10 20:54 ` [PATCH 6/6] gdb: don't share aspace/pspace on fork with "detach-on-fork on" and "follow-fork-mode child" Simon Marchi
@ 2021-09-23 19:23 ` Pedro Alves
5 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2021-09-23 19:23 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
FWIW, I had already discussed & iterated over this with Simon offlist a while ago,
so no further comments here. I gave it another skim just in case, and it still
looks good.
The series is OK.
Simon is looking at the issue John pointed at as a follow up patch.
^ permalink raw reply [flat|nested] 18+ messages in thread