From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 57A4D3858CDB for ; Mon, 3 Apr 2023 13:51:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 57A4D3858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680529860; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=iJ0ygnYsbsKYbI3lTsF5fgnQsCF6fgyLk2ow1iH2vco=; b=chDqPSNe1S1QyXrFs3RClGt7oBi8hAacxZWsbH++xR65lDHktyL5ghQCggqUf3142XOi3W aPTKoVhriL9uPPk9FmYOO5MV4AYsacuoYuHFuE3VFYmrkGawSqCUhNi6C00CmysbW1uavF JHnAPo624YCQ9JKAlVxdy+UNrEa/Wf0= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-68-r2HZEzi7PLO4-jCCaJFb6w-1; Mon, 03 Apr 2023 09:50:59 -0400 X-MC-Unique: r2HZEzi7PLO4-jCCaJFb6w-1 Received: by mail-qt1-f199.google.com with SMTP id m7-20020a05622a118700b003e4e203bc30so17507059qtk.7 for ; Mon, 03 Apr 2023 06:50:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680529858; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=iJ0ygnYsbsKYbI3lTsF5fgnQsCF6fgyLk2ow1iH2vco=; b=nYgAgSyuWBtk0Jygd+ecfwtEpvwPOam9wxFFWPvdwvT7ccPYe3vRfr1XsgdG0DupWC 4eTBRhnBE8D5g6MwH2xWZ2oVrZZmX+0YdweXAQ1r0VJMadPa6rpb1P28i9PFspAat9c0 WNfG1oZcqMQM0pkJg4+G+tTpcAkMy+tZiirnuFwwSZSMi+0wvc79jE2oHoAUTDmqtn93 poOIvYrSjSpaFMmA150KANqe0uy5FcohYaW2+5jrRtpQdimHHBWgSQFMFfGxcquiiUIr YCAieenjg0uM8d3cCkZgnc5flV/ymxcIxjUQebSCgCyfbAi0StOlzM6YfpjNASdv8cR2 TENg== X-Gm-Message-State: AAQBX9fVhYBxC2BJrn1LRLboSsqkThbCyt05b3+YnXjOhfFSdRTK/vnz yzOi7fbqMP8NRrugBm1jHgGB/pyXgDQFpwX6wsMFz9WAI9dlmQMH1kNo64tMWt0n2cFMeakAvAv 34zvQ7E2LyAy+fzjQmwrAKW7PIx0jDaZKis2JzQ6v3XLuZNP5vrlIQ900O2JHXGrTig991cSfI5 334OXRmg== X-Received: by 2002:ad4:5e8f:0:b0:5dd:cbdc:85f6 with SMTP id jl15-20020ad45e8f000000b005ddcbdc85f6mr58770650qvb.26.1680529858270; Mon, 03 Apr 2023 06:50:58 -0700 (PDT) X-Google-Smtp-Source: AKy350Y55OhutJzhDzK/mIyMtTdqFx6PGvE7jML0ZUkbF/hOPlMUASPNSMMD+zQZ1seSRlxqiZcqgQ== X-Received: by 2002:ad4:5e8f:0:b0:5dd:cbdc:85f6 with SMTP id jl15-20020ad45e8f000000b005ddcbdc85f6mr58770588qvb.26.1680529857640; Mon, 03 Apr 2023 06:50:57 -0700 (PDT) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id b25-20020ac87559000000b003e45a39ed74sm2538944qtr.81.2023.04.03.06.50.57 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Apr 2023 06:50:57 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: Re: [PATCHv5 04/11] gdb: avoid repeated signal reporting during failed conditional breakpoint In-Reply-To: <1409cdc89437b485fd464dc99005d0457c88693f.1678987897.git.aburgess@redhat.com> References: <1409cdc89437b485fd464dc99005d0457c88693f.1678987897.git.aburgess@redhat.com> Date: Mon, 03 Apr 2023 14:50:56 +0100 Message-ID: <87iledcbwf.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Andrew Burgess writes: > Consider the following case: > > (gdb) list some_func > 1 int > 2 some_func () > 3 { > 4 int *p = 0; > 5 return *p; > 6 } > 7 > 8 void > 9 foo () > 10 { > (gdb) break foo if (some_func ()) > Breakpoint 1 at 0x40111e: file bpcond.c, line 11. > (gdb) r > Starting program: /tmp/bpcond > > Program received signal SIGSEGV, Segmentation fault. > 0x0000000000401116 in some_func () at bpcond.c:5 > 5 return *p; > Error in testing breakpoint condition: > The program being debugged was signaled while in a function called from GDB. > GDB remains in the frame where the signal was received. > To change this behavior use "set unwindonsignal on". > Evaluation of the expression containing the function > (some_func) will be abandoned. > When the function is done executing, GDB will silently stop. > > Program received signal SIGSEGV, Segmentation fault. > > Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5 > 5 return *p; > (gdb) > > Notice that this line: > > Program received signal SIGSEGV, Segmentation fault. > > Appears twice in the output. The first time is followed by the > current location. The second time is a little odd, why do we print > that? > > Printing that line is controlled, in part, by a global variable, > stopped_by_random_signal. This variable is reset to zero in > handle_signal_stop, and is set if/when GDB figures out that the > inferior stopped due to some random signal. > > The problem is, in our case, GDB first stops at the breakpoint for > foo, and enters handle_signal_stop and the stopped_by_random_signal > global is reset to 0. > > Later within handle_signal_stop GDB calls bpstat_stop_status, it is > within this function (via bpstat_check_breakpoint_conditions) that the > breakpoint condition is checked, and, we end up calling the inferior > function (some_func in our example above). > > In our case above the thread performing the inferior function call > segfaults in some_func. GDB catches the SIGSEGV and handles the stop, > this causes us to reenter handle_signal_stop. The global variable > stopped_by_random_signal is updated, this time it is set to true > because the thread stopped due to SIGSEGV. As a result of this we > print the first instance of the line (as seen above in the example). > > Finally we unwind GDB's call stack, the inferior function call is > complete, and we return to the original handle_signal_stop. However, > the stopped_by_random_signal global is still carrying the value as > computed for the inferior function call's stop, which is why we now > print a second instance of the line, as seen in the example. > > To prevent this, I propose adding a scoped_restore before we start an > inferior function call. This will save and restore the global > stopped_by_random_signal value. > > With this done, the output from our example is now this: > > (gdb) list some_func > 1 int > 2 some_func () > 3 { > 4 int *p = 0; > 5 return *p; > 6 } > 7 > 8 void > 9 foo () > 10 { > (gdb) break foo if (some_func ()) > Breakpoint 1 at 0x40111e: file bpcond.c, line 11. > (gdb) r > Starting program: /tmp/bpcond > > Program received signal SIGSEGV, Segmentation fault. > 0x0000000000401116 in some_func () at bpcond.c:5 > 5 return *p; > Error in testing condition for breakpoint 1: > The program being debugged stopped while in a function called from GDB. > Evaluation of the expression containing the function > (some_func) will be abandoned. > When the function is done executing, GDB will silently stop. > > Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5 > 5 return *p; > (gdb) > > We now only see the 'Program received signal SIGSEGV, ...' line once, > which I think makes more sense. > > Finally, I'm aware that the last few lines, that report the stop as > being at 'Breakpoint 1', when this is not where the thread is actually > located anymore, is not great. I'll address that in the next commit. I went ahead and pushed this commit. If there are any problems, please let me know, I'm happy to address any issues. Thanks, Andrew > --- > gdb/infcall.c | 9 + > gdb/testsuite/gdb.base/infcall-failure.c | 48 ++++++ > gdb/testsuite/gdb.base/infcall-failure.exp | 185 +++++++++++++++++++++ > 3 files changed, 242 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/infcall-failure.c > create mode 100644 gdb/testsuite/gdb.base/infcall-failure.exp > > diff --git a/gdb/infcall.c b/gdb/infcall.c > index 9ed17bf4f8b..e6cc6ed1a21 100644 > --- a/gdb/infcall.c > +++ b/gdb/infcall.c > @@ -1293,6 +1293,15 @@ call_function_by_hand_dummy (struct value *function, > /* Register a clean-up for unwind_on_terminating_exception_breakpoint. */ > SCOPE_EXIT { delete_std_terminate_breakpoint (); }; > > + /* The stopped_by_random_signal variable is global. If we are here > + as part of a breakpoint condition check then the global will have > + already been setup as part of the original breakpoint stop. By > + making the inferior call the global will be changed when GDB > + handles the stop after the inferior call. Avoid confusion by > + restoring the current value after the inferior call. */ > + scoped_restore restore_stopped_by_random_signal > + = make_scoped_restore (&stopped_by_random_signal, 0); > + > /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - > If you're looking to implement asynchronous dummy-frames, then > just below is the place to chop this function in two.. */ > diff --git a/gdb/testsuite/gdb.base/infcall-failure.c b/gdb/testsuite/gdb.base/infcall-failure.c > new file mode 100644 > index 00000000000..fae012b2cda > --- /dev/null > +++ b/gdb/testsuite/gdb.base/infcall-failure.c > @@ -0,0 +1,48 @@ > +/* Copyright 2022-2023 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + 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 . */ > + > +/* A function that segfaults (assuming that reads of address zero are > + prohibited), this is used from within a breakpoint condition. */ > +int > +func_segfault () > +{ > + volatile int *p = 0; > + return *p; /* Segfault here. */ > +} > + > +/* A function in which we will place a breakpoint. This function is itself > + then used from within a breakpoint condition. */ > +int > +func_bp () > +{ > + int res = 0; /* Second breakpoint. */ > + return res; > +} > + > +int > +foo () > +{ > + return 0; /* First breakpoint. */ > +} > + > +int > +main () > +{ > + int res = foo (); > + > + return res; > +} > diff --git a/gdb/testsuite/gdb.base/infcall-failure.exp b/gdb/testsuite/gdb.base/infcall-failure.exp > new file mode 100644 > index 00000000000..214a64f8de3 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/infcall-failure.exp > @@ -0,0 +1,185 @@ > +# Copyright 2022-2023 Free Software Foundation, Inc. > + > +# 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 . > + > +# Some simple tests of inferior function calls from breakpoint > +# conditions, in a single-threaded inferior. > +# > +# Test what happens when the inferior function (from a breakpoint > +# condition) either hits a nested breakpoint, or segfaults. > + > +standard_testfile > + > +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ > + {debug}] == -1 } { > + return > +} > + > +set bp_1_line [gdb_get_line_number "First breakpoint"] > +set bp_2_line [gdb_get_line_number "Second breakpoint"] > +set segv_line [gdb_get_line_number "Segfault here"] > + > +# Start GDB based on TARGET_ASYNC and TARGET_NON_STOP, and then runto > +# main. > +proc start_gdb_and_runto_main { target_async target_non_stop } { > + save_vars { ::GDBFLAGS } { > + append ::GDBFLAGS \ > + " -ex \"maint set target-non-stop $target_non_stop\"" > + append ::GDBFLAGS \ > + " -ex \"maintenance set target-async ${target_async}\"" > + > + clean_restart ${::binfile} > + } > + > + if { ![runto_main] } { > + return -1 > + } > + > + return 0 > +} > + > +# Start GDB according to ASYNC_P and NON_STOP_P, then setup a > +# conditional breakpoint. The breakpoint condition includes an > +# inferior function call that will itself hit a breakpoint. Check how > +# GDB reports this to the user. > +proc_with_prefix run_cond_hits_breakpoint_test { async_p non_stop_p } { > + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { > + return > + } > + > + # Setup the conditional breakpoint and record its number. > + gdb_breakpoint "${::srcfile}:${::bp_1_line} if (func_bp ())" > + set bp_1_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ > + "get number of first breakpoint"] > + > + # Setup a breakpoint inside func_bp. > + gdb_breakpoint "${::srcfile}:${::bp_2_line}" > + set bp_2_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ > + "get number of second breakpoint"] > + > + gdb_test "continue" \ > + [multi_line \ > + "Continuing\\." \ > + "" \ > + "Breakpoint ${bp_2_num}, func_bp \\(\\) at \[^\r\n\]+:${::bp_2_line}" \ > + "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+" \ > + "Error in testing condition for breakpoint ${bp_1_num}:" \ > + "The program being debugged stopped while in a function called from GDB\\." \ > + "Evaluation of the expression containing the function" \ > + "\\(func_bp\\) will be abandoned\\." \ > + "When the function is done executing, GDB will silently stop\\." \ > + "" \ > + "Breakpoint ${bp_1_num}, \[^\r\n\]+" \ > + "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+"] > +} > + > +# Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior > +# function. The inferior function being called will itself have a > +# breakpoint within it. Check how GDB reports this to the user. > +proc_with_prefix run_call_hits_breakpoint_test { async_p non_stop_p } { > + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { > + return > + } > + > + # Setup a breakpoint inside func_bp. > + gdb_breakpoint "${::srcfile}:${::bp_2_line}" > + set bp_2_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ > + "get number of second breakpoint"] > + > + > + gdb_test "call func_bp ()" \ > + [multi_line \ > + "" \ > + "Breakpoint ${bp_2_num}, func_bp \\(\\) at \[^\r\n\]+:${::bp_2_line}" \ > + "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+" \ > + "The program being debugged stopped while in a function called from GDB\\." \ > + "Evaluation of the expression containing the function" \ > + "\\(func_bp\\) will be abandoned\\." \ > + "When the function is done executing, GDB will silently stop\\."] > +} > + > +# Start GDB according to ASYNC_P and NON_STOP_P, then setup a > +# conditional breakpoint. The breakpoint condition includes an > +# inferior function call that segfaults. Check how GDB reports this > +# to the user. > +proc_with_prefix run_cond_hits_segfault_test { async_p non_stop_p } { > + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { > + return > + } > + > + # This test relies on the inferior segfaulting when trying to > + # access address zero. > + if { [is_address_zero_readable] } { > + unsupported "address zero is readable" > + return > + } > + > + # Setup the conditional breakpoint and record its number. > + gdb_breakpoint "${::srcfile}:${::bp_1_line} if (func_segfault ())" > + set bp_1_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ > + "get number of first breakpoint"] > + > + gdb_test "continue" \ > + [multi_line \ > + "Continuing\\." \ > + "" \ > + "Program received signal SIGSEGV, Segmentation fault\\." \ > + "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \ > + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \ > + "Error in testing condition for breakpoint ${bp_1_num}:" \ > + "The program being debugged stopped while in a function called from GDB\\." \ > + "Evaluation of the expression containing the function" \ > + "\\(func_segfault\\) will be abandoned\\." \ > + "When the function is done executing, GDB will silently stop\\." \ > + "" \ > + "Breakpoint ${bp_1_num}, \[^\r\n\]+" \ > + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] > +} > + > +# Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior > +# function. The inferior function will segfault. Check how GDB > +# reports this to the user. > +proc_with_prefix run_call_hits_segfault_test { async_p non_stop_p } { > + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { > + return > + } > + > + # This test relies on the inferior segfaulting when trying to > + # access address zero. > + if { [is_address_zero_readable] } { > + unsupported "address zero is readable" > + return > + } > + > + gdb_test "call func_segfault ()" \ > + [multi_line \ > + "" \ > + "Program received signal SIGSEGV, Segmentation fault\\." \ > + "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \ > + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \ > + "The program being debugged stopped while in a function called from GDB\\." \ > + "Evaluation of the expression containing the function" \ > + "\\(func_segfault\\) will be abandoned\\." \ > + "When the function is done executing, GDB will silently stop\\."] > +} > + > +foreach_with_prefix target_async { "on" "off" } { > + foreach_with_prefix target_non_stop { "on" "off" } { > + run_cond_hits_breakpoint_test $target_async $target_non_stop > + run_call_hits_breakpoint_test $target_async $target_non_stop > + > + run_cond_hits_segfault_test $target_async $target_non_stop > + run_call_hits_segfault_test $target_async $target_non_stop > + } > +} > -- > 2.25.4