From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7194 invoked by alias); 15 Aug 2010 23:10:10 -0000 Received: (qmail 7185 invoked by uid 22791); 15 Aug 2010 23:10:09 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_CP,TW_XF,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 15 Aug 2010 23:10:03 +0000 Received: (qmail 10106 invoked from network); 15 Aug 2010 23:10:01 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 15 Aug 2010 23:10:01 -0000 From: Pedro Alves To: Michael Snyder Subject: Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n} Date: Sun, 15 Aug 2010 23:10:00 -0000 User-Agent: KMail/1.13.2 (Linux/2.6.33-29-realtime; KDE/4.4.2; x86_64; ; ) Cc: "gdb-patches@sourceware.org" , Hui Zhu , Eli Zaretskii References: <201008151946.55330.pedro@codesourcery.com> <4C683A6C.8040501@vmware.com> In-Reply-To: <4C683A6C.8040501@vmware.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201008160009.57924.pedro@codesourcery.com> 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-08/txt/msg00216.txt.bz2 On Sunday 15 August 2010 20:05:16, Michael Snyder wrote: > Pedro Alves wrote: > > On Sunday 15 August 2010 08:16:34, Hui Zhu wrote: > >> Sending packet: $QTFrame:ffffffff#fa...Packet received: OK > > > > I think it is a bug that this is assuming 32-bit, two's complement > > on both client/server sides. (not sure it that was what you were > > referring to). > > It's not really assuming that -- it's just assuming that no > legitimate frame ID will ever be as high as ffffffff. I don't know where you're getting that from. If you believe that to be true, then you also have to believe Hui's patch is wrong, as it documents "-1", not the magic hard limit of "0xffffffff". Start here: /* tfind end */ static void trace_find_end_command (char *args, int from_tty) { trace_find_command ("-1", from_tty); ^^ } through here: /* Worker function for the various flavors of the tfind command. */ void tfind_1 (enum trace_find_type type, int num, ^^^^^^^ ULONGEST addr1, ULONGEST addr2, int from_tty) { int target_frameno = -1, target_tracept = -1; struct frame_id old_frame_id = null_frame_id; struct breakpoint *tp; /* Only try to get the current stack frame if we have a chance of succeeding. In particular, if we're trying to get a first trace frame while all threads are running, it's not going to succeed, so leave it with a default value and let the frame comparison below (correctly) decide to print out the source location of the trace frame. */ if (!(type == tfind_number && num == -1) && (has_stack_frames () || traceframe_number >= 0)) old_frame_id = get_frame_id (get_current_frame ()); target_frameno = target_trace_find (type, num, addr1, addr2, &target_tracept); if (type == tfind_number && num == -1 ^^^^^^^^^ && target_frameno == -1) { /* We told the target to get out of tfind mode, and it did. */ } else if (target_frameno == -1) { /* A request for a non-existent trace frame has failed. Our response will be different, depending on FROM_TTY: If FROM_TTY is true, meaning that this command was typed interactively by the user, then give an error and DO NOT change the state of traceframe_number etc. However if FROM_TTY is false, meaning that we're either in a script, a loop, or a user-defined command, then DON'T give an error, but DO change the state of traceframe_number etc. to invalid. The rationalle is that if you typed the command, you might just have committed a typo or something, and you'd like to NOT lose your current debugging state. However if you're in a user-defined command or especially in a loop, then you need a way to detect that the command failed WITHOUT aborting. This allows you to write scripts that search thru the trace buffer until the end, and then continue on to do something else. */ if (from_tty) error (_("Target failed to find requested trace frame.")); Notice how you get to the "else if" branch. Choosing any other "high enough" random frame id that doesn't exist errors out. Only the special case of 0xffffffff being parsed as -1 on both target and host actually gets out of tfind mode. Continuing, the target_trace_find call ends up here: static int remote_trace_find (enum trace_find_type type, int num, ULONGEST addr1, ULONGEST addr2, int *tpp) { struct remote_state *rs = get_remote_state (); char *p, *reply; int target_frameno = -1, target_tracept = -1; p = rs->buf; strcpy (p, "QTFrame:"); p = strchr (p, '\0'); switch (type) { case tfind_number: sprintf (p, "%x", num); ^^^^^^^^^ break; ... and here, NUM, is an int, and, is printed with %x. Assuming that prints 0xffffffff is actually host dependent (e.g., consider ILP64). Granted, likely not a concern on all hosts we care about. Still a needless assumption. The gdbserver side does: ... unpack_varlen_hex (packet, &frame); tfnum = (int) frame; if (tfnum == -1) { trace_debug ("Want to stop looking at traceframes"); current_traceframe = -1; write_ok (own_buf); return; } Again would break if the target is ILP64, and host is not. And below that bit of code you'll see that trying to select any other high enough frame number that does not exist does not get out of tfind mode, but instead returns an error. > That might also be iffy, but less so.... I should have mentioned that I consider this a _design_ bug we get to live with. Fixing it like I suggested would be incompatible with current targets, so, best leave this alone until it actually becomes a problem. Though, this raises the point that Hui's docs don't actually match what GDB sends. Not sure what to do. I guess I'll just pretend I didn't spot this. ;-) > IMO, negatives should have an explicit '-' encoding; in > > this case, "$QTFrame:-1". Note sure if the RSP docs mention something > > about this. We are careful in some cases (passing thread id's, > > I think, is one case), though clearly not everywhere. -- Pedro Alves