[1/2] Add dprintf and detach test (PR breakpoints/17012)

  Ping.


-------- Original Message --------
Subject: [PATCH 1/2] Add dprintf and detach test (PR breakpoints/17012)
Date: Wed, 18 Jun 2014 10:26:10 -0400
From: Simon Marchi <simon.marchi@ericsson.com>
To: <gdb-patches@sourceware.org>
CC: Simon Marchi <simon.marchi@ericsson.com>

This adds a test to demonstrate PR 17012, where adding a dprintf in a
linux native process and detaching leaves the trap instruction in the
process. I copied bits from many other existing tests.

The test fails now, but is fixed by the following commit.

gdb/testsuite/ChangeLog:

2014-06-18  Simon Marchi  simon.marchi@ericsson.com

	PR breakpoints/17012
	gdb.base/dprintf-detach.c: New file.
	gdb.base/dprintf-detach.exp: New file.

---
 gdb/testsuite/gdb.base/dprintf-detach.c   | 18 +++++++
 gdb/testsuite/gdb.base/dprintf-detach.exp | 80 +++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/dprintf-detach.c
 create mode 100644 gdb/testsuite/gdb.base/dprintf-detach.exp
  

Hello Simon,

> 2014-06-18  Simon Marchi  simon.marchi@ericsson.com
> 
> 	PR breakpoints/17012
> 	gdb.base/dprintf-detach.c: New file.
> 	gdb.base/dprintf-detach.exp: New file.

Sorry for the delay in answering this.

> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.c b/gdb/testsuite/gdb.base/dprintf-detach.c
> new file mode 100644
> index 0000000..91f49ce
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dprintf-detach.c
> @@ -0,0 +1,18 @@
> +#include <stdlib.h>

This file needs a copyright notice, please.

> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
> new file mode 100644
> index 0000000..18ba154
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
> @@ -0,0 +1,80 @@
> +#   Copyright 2003-2014 Free Software Foundation, Inc.

Can you confirm that 2003 is the  correct year for the start
of the range?

> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# This test checks that inserting a dprintf and detaching does not crash
> +# the program.
> +#
> +# Related bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17012
> +
> +# Only GNU/Linux is known to support (dprintf and detach).
> +if { ! [istarget "*-*-linux*"] } {
> +  return 0
> +}
> +
> +# Are we on a target board?
> +if [is_remote target] then {
> +    return 0
> +}
> +
> +standard_testfile
> +set escapedbinfile  [string_to_regexp ${binfile}]
> +
> +if [prepare_for_testing "failed to prepare for dprintf-detach" \
> +    ${testfile} ${srcfile} {debug}] {
> +    return -1
> +}
> +
> +# The problem was showing up in non-stop mode, since it enables
> +# "breakpoint always-inserted", so this could also be
> +# "set breakpoint always-inserted on".
> +gdb_test_no_output "set non-stop on"
> +
> +if ![runto_main] {
> +    fail "Can't run to main"
> +    return -1
> +}
> +
> +# Get PID of test program.
> +set inferior_pid -1
> +set test "get inferior process ID"
> +gdb_test_multiple "call getpid ()" $test {
> +    -re ".* = ($decimal).*$gdb_prompt $" {
> +	set inferior_pid $expect_out(1,string)
> +	pass $test
> +    }
> +}
> +
> +# Add a dprintf and detach.
> +gdb_test "dprintf function, \"hello\"" "Dprintf .*" "dprintf insertion"
> +gdb_test "detach" "Detaching from program: .*$escapedbinfile, .*" "detach program"
> +
> +# Exit gdb. Until we do that, the process will exist as a zombie.

Two spaces after the period.

> +gdb_exit
> +
> +# Give some time for the ex-inferior to run and hopefully not crash.
> +sleep 1
> +
> +# Check that the process still exists.
> +set test "detached process should continue to exist"
> +if {[catch {exec kill -0 $inferior_pid}]} {
> +	# Process does not exist.
> +	fail "$test"
> +} else {
> +	# Process exists.
> +	pass "$test"
> +}
> +
> +# Clean up.
> +catch {exec kill -9 $inferior_pid}

I am not quite sure about how well how the above would work in practice.
How about restarting GDB, and do an "attach" gdb_test? That  way, we
can  kill the inferior process from GDB as well. (it's a good thing
you used a 30-times loops as well  to make sure the process eventually
terminates on its own as well).
  

On 14-07-07 11:06 AM, Joel Brobecker wrote:
> Hello Simon,
> 
>> 2014-06-18  Simon Marchi  simon.marchi@ericsson.com
>>
>> 	PR breakpoints/17012
>> 	gdb.base/dprintf-detach.c: New file.
>> 	gdb.base/dprintf-detach.exp: New file.
> 
> Sorry for the delay in answering this.
> 
>> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.c b/gdb/testsuite/gdb.base/dprintf-detach.c
>> new file mode 100644
>> index 0000000..91f49ce
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/dprintf-detach.c
>> @@ -0,0 +1,18 @@
>> +#include <stdlib.h>
> 
> This file needs a copyright notice, please.

I checked a few other test files and they didn't have the copyright notice,
that's why I didn't add it. I have no problem adding it if it should be
there.

>> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
>> new file mode 100644
>> index 0000000..18ba154
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
>> @@ -0,0 +1,80 @@
>> +#   Copyright 2003-2014 Free Software Foundation, Inc.
> 
> Can you confirm that 2003 is the  correct year for the start
> of the range?

Oops, just 2014.

>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +# This test checks that inserting a dprintf and detaching does not crash
>> +# the program.
>> +#
>> +# Related bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17012
>> +
>> +# Only GNU/Linux is known to support (dprintf and detach).
>> +if { ! [istarget "*-*-linux*"] } {
>> +  return 0
>> +}
>> +
>> +# Are we on a target board?
>> +if [is_remote target] then {
>> +    return 0
>> +}
>> +
>> +standard_testfile
>> +set escapedbinfile  [string_to_regexp ${binfile}]
>> +
>> +if [prepare_for_testing "failed to prepare for dprintf-detach" \
>> +    ${testfile} ${srcfile} {debug}] {
>> +    return -1
>> +}
>> +
>> +# The problem was showing up in non-stop mode, since it enables
>> +# "breakpoint always-inserted", so this could also be
>> +# "set breakpoint always-inserted on".
>> +gdb_test_no_output "set non-stop on"
>> +
>> +if ![runto_main] {
>> +    fail "Can't run to main"
>> +    return -1
>> +}
>> +
>> +# Get PID of test program.
>> +set inferior_pid -1
>> +set test "get inferior process ID"
>> +gdb_test_multiple "call getpid ()" $test {
>> +    -re ".* = ($decimal).*$gdb_prompt $" {
>> +	set inferior_pid $expect_out(1,string)
>> +	pass $test
>> +    }
>> +}
>> +
>> +# Add a dprintf and detach.
>> +gdb_test "dprintf function, \"hello\"" "Dprintf .*" "dprintf insertion"
>> +gdb_test "detach" "Detaching from program: .*$escapedbinfile, .*" "detach program"
>> +
>> +# Exit gdb. Until we do that, the process will exist as a zombie.
> 
> Two spaces after the period.
> 
>> +gdb_exit
>> +
>> +# Give some time for the ex-inferior to run and hopefully not crash.
>> +sleep 1
>> +
>> +# Check that the process still exists.
>> +set test "detached process should continue to exist"
>> +if {[catch {exec kill -0 $inferior_pid}]} {
>> +	# Process does not exist.
>> +	fail "$test"
>> +} else {
>> +	# Process exists.
>> +	pass "$test"
>> +}
>> +
>> +# Clean up.
>> +catch {exec kill -9 $inferior_pid}
> 
> I am not quite sure about how well how the above would work in practice.
> How about restarting GDB, and do an "attach" gdb_test? That  way, we
> can  kill the inferior process from GDB as well. (it's a good thing
> you used a 30-times loops as well  to make sure the process eventually
> terminates on its own as well).

It "works", as in "it runs fine on my machine". Your suggestion to attach the
process with a new gdb seems more portable, as it doesn't rely on kill. I will
try to make a version based on that.
  

> I checked a few other test files and they didn't have the copyright
> notice, that's why I didn't add it. I have no problem adding it if it
> should be there.

Yeah, we've been pretty bad about this in the past, and normally
we should go back and fix all files. But it can be so much work
researching those that we tend to do it piecemeal, when we happen
to notice one of these files. In the meantime, we try to be better
with all new files being added.