From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7954 invoked by alias); 27 Mar 2012 08:15:01 -0000 Received: (qmail 7913 invoked by uid 22791); 27 Mar 2012 08:14:59 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_GJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 27 Mar 2012 08:14:45 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2R8EiuO012239 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 27 Mar 2012 04:14:44 -0400 Received: from host2.jankratochvil.net (ovpn-116-28.ams2.redhat.com [10.36.116.28]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q2R8Edu4031651 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 27 Mar 2012 04:14:42 -0400 Date: Tue, 27 Mar 2012 08:15:00 -0000 From: Jan Kratochvil To: Mark Kettenis Cc: gdb-patches@sourceware.org Subject: Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5 Message-ID: <20120327081439.GA8387@host2.jankratochvil.net> References: <20120309210045.GA30432@host2.jankratochvil.net> <20120326190355.GA11001@host2.jankratochvil.net> <201203261953.q2QJrXX4023325@glazunov.sibelius.xs4all.nl> <20120326203151.GA18085@host2.jankratochvil.net> <201203262145.q2QLjRIJ024024@glazunov.sibelius.xs4all.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201203262145.q2QLjRIJ024024@glazunov.sibelius.xs4all.nl> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-03/txt/msg00901.txt.bz2 On Mon, 26 Mar 2012 23:45:27 +0200, Mark Kettenis wrote: > Well, I'd prefer not to have to re-read the entire discussion to > review your diff. But are you saying that you now do think ON_STACK > is the right way to go and I can simply forget about the history? We need some place without any installed FDE or a place with our own (unwind-terminating) FDE. _start was proven is no longer the right place (on ppc*), PLT stubs are also no longer the right place (with recent ld). So remains stack or maybe some malloc-allocated memory but from those ON_STACK seems to be easier. I had some concerns about security frameworks limiting stack executability but I have no real world countercase, it seems to be all handled correctly. Still despite [patch 1/2] now correctly removes stale breakpoints from stack in some cases the breakpoints may remain there such as if inferior longjmps out of the inferior called function. With ON_STACK inferior functions it is a real problem, stale inferior call breakpoints were not a problem before. breakpoint.h: /* The breakpoint at the end of a call dummy. */ /* FIXME: What if the function we are calling longjmp()s out of the call, or the user gets out with the "return" command? We currently have no way of cleaning up the breakpoint in these (obscure) situations. (Probably can solve this by noticing longjmp, "return", etc., it's similar to noticing when a watchpoint on a local variable goes out of scope (with hardware support for watchpoints)). */ bp_call_dummy, The "return" command is implemented in [patch 1/2] (and the comment is not updated) but retrospectively I see we may need to also handle the out-of-scope breakpoints. OTOH I did not implement it because in some cases backtrace fails temporarily due to some missing CFIs and it would immediately remove such inferior return breakpoints. It happens with Watchpoint %d deleted because the program has left the block\n\ in which its expression is valid.\n"), but a deleted watchpoint is not so serious like deleted inferior call return breakpoint. There may be rather some logic to consider a frame X stale if we successfully unwind next frame, X is not found and X is not inner to the next frame. But (a) There were some plans to remove frame_id_inner completely as it breaks with -fsplit-stack for example. (b) bp_watchpoint_scope should be also reworked the same way that time. Another way is to really handle such stale breakpoints in the longjmp breakpoint caught by GDB. I did not investigate more this way. But TBH longjmp is a hack, one should use proper C++ exceptions instead and those get caught properly for inferior calls. These were some of the reasons I did not go so far to try deal more with longjmps out of inferior calls, which are bizarre enough on their own. Thanks, Jan gdb/ 2012-03-27 Jan Kratochvil Remove momentary breakpoints for completed inferior calls. * breakpoint.h (bp_call_dummy): Update the comment. * dummy-frame.c: Include gdbthread.h. (pop_dummy_frame_bpt): New function. (pop_dummy_frame): Initialie DUMMY earlier. Call pop_dummy_frame_bpt. gdb/testsuite/ 2012-03-09 Jan Kratochvil Remove momentary breakpoints for completed inferior calls. * gdb.base/call-signal-resume.exp (maintenance print dummy-frames) (maintenance info breakpoints): New tests. --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -95,10 +95,10 @@ enum bptype /* The breakpoint at the end of a call dummy. */ /* FIXME: What if the function we are calling longjmp()s out of - the call, or the user gets out with the "return" command? We - currently have no way of cleaning up the breakpoint in these - (obscure) situations. (Probably can solve this by noticing - longjmp, "return", etc., it's similar to noticing when a + the call? "return" command is handled by pop_dummy_frame_bpt. + We currently have no way of cleaning up the breakpoint in such + (obscure) situation. (Probably can solve this by noticing + longjmp, etc., it's similar to noticing when a watchpoint on a local variable goes out of scope (with hardware support for watchpoints)). */ bp_call_dummy, --- a/gdb/dummy-frame.c +++ b/gdb/dummy-frame.c @@ -29,6 +29,7 @@ #include "gdbcmd.h" #include "gdb_string.h" #include "observer.h" +#include "gdbthread.h" /* Dummy frame. This saves the processor state just prior to setting up the inferior function call. Older targets save the registers @@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr) xfree (dummy); } +/* Delete any breakpoint B which is a momentary breakpoint for return from + inferior call matching DUMMY_VOIDP. */ + +static int +pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp) +{ + struct dummy_frame *dummy = dummy_voidp; + + if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id) + && b->thread == pid_to_thread_id (inferior_ptid)) + delete_breakpoint (b); + + /* Continue the traversal. */ + return 0; +} + /* Pop *DUMMY_PTR, restoring program state to that before the frame was created. */ static void pop_dummy_frame (struct dummy_frame **dummy_ptr) { - struct dummy_frame *dummy; + struct dummy_frame *dummy = *dummy_ptr; + + restore_infcall_suspend_state (dummy->caller_state); - restore_infcall_suspend_state ((*dummy_ptr)->caller_state); + iterate_over_breakpoints (pop_dummy_frame_bpt, dummy); /* restore_infcall_control_state frees inf_state, all that remains is to pop *dummy_ptr. */ - dummy = *dummy_ptr; *dummy_ptr = dummy->next; xfree (dummy); --- a/gdb/testsuite/gdb.base/call-signal-resume.exp +++ b/gdb/testsuite/gdb.base/call-signal-resume.exp @@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*" gdb_test_no_output "set confirm off" gdb_test "return" "" +# Verify there are no remains of the dummy frame. +gdb_test_no_output "maintenance print dummy-frames" +set test "maintenance info breakpoints" +gdb_test_multiple $test $test { + -re "call dummy.*\r\n$gdb_prompt $" { + fail $test + } + -re "\r\n$gdb_prompt $" { + pass $test + } +} + # Resume execution, the program should continue without any signal. gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"