From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22753 invoked by alias); 25 Nov 2010 07:59:12 -0000 Received: (qmail 22504 invoked by uid 22791); 25 Nov 2010 07:59:07 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,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; Thu, 25 Nov 2010 07:58:59 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oAP7wuxF030807 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 25 Nov 2010 02:58:56 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oAP7wpCP007622 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 25 Nov 2010 02:58:55 -0500 Received: from host0.dyn.jankratochvil.net (localhost.localdomain [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id oAP7woxe025765; Thu, 25 Nov 2010 08:58:50 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id oAP7wmsY025764; Thu, 25 Nov 2010 08:58:48 +0100 Date: Thu, 25 Nov 2010 07:59:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: RFC: next/finish/etc -vs- exceptions Message-ID: <20101125075847.GA19270@host0.dyn.jankratochvil.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: 2010-11/txt/msg00424.txt.bz2 On Thu, 07 Oct 2010 03:37:38 +0200, Tom Tromey wrote: > * breakpoint.h (enum bptype) bp_exception_master>: New constants. Therefore it is a precent new `bptype's are permitted instead of using breakpoint_ops (which may need some extensions, not sure). It may be explained also by the Joel's difficulties to do so: http://sourceware.org/ml/gdb-patches/2009-06/msg00298.html BTW the testcase does not work on neither ppc32 nor on ppc64. ppc32 due to some unexpected next/step stop lines. I see two places in the code using CORE_ADDR where is probably gdbarch_convert_from_func_ptr_addr appropriate for the ppc64 case. > @@ -8524,6 +8588,10 @@ until_break_command (char *arg, int from_tty, int anywhere) > frame_unwind_caller_id (frame), > bp_until); > make_cleanup_delete_breakpoint (breakpoint2); > + > + set_longjmp_breakpoint (thread); > + tp->initiating_frame = frame_unwind_caller_id (frame); tp->initiating_frame should be initialized from set_longjmp_breakpoint, as it is required for that operation. It probably should not be placed in TP. If we are stepping/until-ing/etc. some code and execute some breakpoint's command list trying to step/next/etc. again already from a different frame it won't work. But this is a problem for most of the TP variables already so that's OK for this patch. It should be carried over from the set-breakpoint to resume-breakpoint otherwise. > +static void > +until_next_continuation (void *arg) > +{ > + struct thread_info *tp = arg; Missing empty line. :-) > + delete_longjmp_breakpoint (tp->num); > +} > @@ -1270,7 +1284,19 @@ until_next_command (int from_tty) > > tp->step_multi = 0; /* Only one call to proceed */ > > + set_longjmp_breakpoint (thread); > + tp->initiating_frame = get_frame_id (frame); > + old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread); > + > proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1); > + > + if (target_can_async_p () && is_running (inferior_ptid)) > + { > + discard_cleanups (old_chain); > + add_continuation (tp, until_next_continuation, tp, NULL); continuation_free_args is NULL here but I think the breakpoint should get deleted even if there is some premature thread deletion. But maybe just all the breakpoints specific for that thread (clear_thread_inferior_resources) should be deleted which would also solve this problem? > +static void > +insert_exception_resume_breakpoint (struct thread_info *tp, > + struct block *b, > + struct frame_info *frame, > + struct symbol *sym) > +{ > + struct gdb_exception e; > + > + /* We want to ignore errors here. */ > + TRY_CATCH (e, RETURN_MASK_ALL) Shouldn't it be RETURN_MASK_ERROR then? Otherwise it should rethrow QUIT (I hope it works that way) but no such cleanups are probably needed here. > +static void > +check_exception_resume (struct execution_control_state *ecs, > + struct frame_info *frame, struct symbol *func) > +{ > + struct gdb_exception e; > + > + TRY_CATCH (e, RETURN_MASK_ALL) again RETURN_MASK_ERROR probably. > diff --git a/gdb/testsuite/gdb.cp/gdb9593.exp b/gdb/testsuite/gdb.cp/gdb9593.exp > new file mode 100644 > index 0000000..04e9c82 > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/gdb9593.exp Please do not use numeric names for testcases. When dealing with them during regression testing / rebasing etc. it makes more difficult to keep track of all the numbers and what they were testing. > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } { > + untested gdb9593.exp > + return -1 > +} maybe prepare_for_testing? (nitpick) > +gdb_exit > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir > +gdb_load ${binfile} prepare_for_testing / clean_restart (nitpick) > +if {!$ok} { > + untested gdb9593.exp rather unsupported? Maybe not, just a hint. > + return -1 > +} > + > +# See http://sourceware.org/bugzilla/show_bug.cgi?id=9593 > + > +gdb_test "next" \ > + ".*catch (...).*" \ > + "next over a throw 1" These all have redundant leading `.*'. `(...)' is not backslashed. > --- /dev/null > +++ b/gdb/testsuite/gdb.java/jnpe.exp [...] > +set testfile "jnpe" > +set srcfile ${testfile}.java > +set binfile ${objdir}/${subdir}/${testfile} > +if { [compile_java_from_source ${srcdir}/$subdir/${srcfile} ${binfile} "-g"] != "" } { > + untested "Couldn't compile ${srcdir}/$subdir/${srcfile}" > + return -1 > +} maybe prepare_for_testing? (nitpick) > +# The line where we stop differ according to gcj; check just we did not already > +# execute the catch point. > + > +gdb_test "next" \ > + "" \ Here should be ".*" (such sanity checking is missing now in lib/gdb.exp). > --- /dev/null > +++ b/gdb/testsuite/gdb.java/jnpe.java > @@ -0,0 +1,38 @@ > +// Test next-over-NPE. > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2009 Free Software Foundation, Inc. +2010 > + public static void main (String[] args) > + { > + try > + { > + System.out.println (npe ()); // break here > + } > + catch (NullPointerException n) > + { > + System.out.println ("success"); // catch point BTW the testcase fives on Fedora 14 x86_64 and i686: +FAIL: gdb.java/jnpe.exp: continue to breakpoint: catch point but this is due to -O0 -g .debug_line wrong in this case. Breakpoint for line 35 is at 0x400cb3: 0x0000000000400cb3 <+112>: mov $0x601560,%edi 0x0000000000400cb8 <+117>: callq 0x400a18 <_Jv_InitClass@plt> 0x0000000000400cbd <+122>: mov $0x1,%ebx But the execution goes: (gdb) stepi 0x0000000000400d0d 33 catch (NullPointerException n) 2: x/i $pc => 0x400d0d : mov (%rax),%rax (gdb) stepi 35 System.out.println ("success"); // catch point 2: x/i $pc => 0x400d10 : test %bl,%bl (gdb) stepi 0x0000000000400d12 35 System.out.println ("success"); // catch point 2: x/i $pc => 0x400d12 : jne 0x400cbd (gdb) stepi 0x0000000000400cbd 35 System.out.println ("success"); // catch point 2: x/i $pc => 0x400cbd : mov $0x1,%ebx > + } > + } > +} Thanks, Jan