From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24692 invoked by alias); 6 Jan 2012 22:38:00 -0000 Received: (qmail 24484 invoked by uid 22791); 6 Jan 2012 22:37:58 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 06 Jan 2012 22:37:43 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1RjIPu-0001d4-7s from Luis_Gustavo@mentor.com ; Fri, 06 Jan 2012 14:37:42 -0800 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 6 Jan 2012 14:37:41 -0800 Received: from [0.0.0.0] ([172.16.63.104]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 6 Jan 2012 14:37:41 -0800 Message-ID: <4F07779A.10808@mentor.com> Date: Fri, 06 Jan 2012 22:38:00 -0000 From: Luis Machado Reply-To: "Gustavo, Luis" User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15 MIME-Version: 1.0 To: Eli Zaretskii CC: gdb-patches@sourceware.org Subject: Re: [RFC stub-side break conditions 3/5] GDB-side changes References: <4F05BA10.3090107@mentor.com> <83y5tlnrsx.fsf@gnu.org> In-Reply-To: <83y5tlnrsx.fsf@gnu.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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-01/txt/msg00252.txt.bz2 On 01/06/2012 06:44 AM, Eli Zaretskii wrote: >> Date: Thu, 05 Jan 2012 12:56:16 -0200 >> From: Luis Machado >> >> This machinery works by marking locations modified through the >> "condition_changed" field. Conditions change whenever we create a new >> location, use the "condition" command or delete an old duplicate >> location. Furthermore, if a location at addr 0xdeadbeef had its >> condition modified, all duplicate locations at 0xdeadbeef are marked >> modified as well. >> >> The modification detection takes place inside >> update_global_location_list (...). By calling >> force_breakpoint_reinsertion (...), we mark every location at address >> 0xdeadbeef as modified. When consolidating the final updated list of >> locations, we detect these locations and mark the non-duplicate location >> at 0xdeafbeef as "needs_update". >> >> The "needs_update" field is used during breakpoint insertion. It forces >> insertion of breakpoints even if they are marked as inserted already. >> >> Now that we have information about locations that we should reinsert in >> the target, we proceed to build a list of hex-encoded agent expressions. >> >> At this point, if any conditional expressions fail to parse to a valid >> agent expression, we assume that the conditions will be evaluated on >> GDB's side. Thus we don't send the conditions to the stub. Otherwise, we >> proceed to insert the breakpoints and the remote code now attaches the >> hex-encoded expressions to the z0/z1 packet. >> >> Regarding always-inserted mode, if it is "on" we must send condition >> changes right away, or else the target will potentially miss breakpoint >> hits. If always-inserted is "off", this isn't too critical, so we just >> wait for the insertion call to send all the conditions to the stub. We >> will remove them when stopping anyway. > WIBNI these details were somewhere in the source comments or (gasp!) > in gdbint.texinfo? I could probably improve source documentation if it is not enough. I did point out in some places that conditions need to be synched with the target. Regarding the internal documentation, i can include such descriptions there, no problem. >> +/* Shows the current mode of breakpoint condition evaluation. Explicitly shows >> + what "auto" is translating to. */ >> + >> +static void >> +show_condition_evaluation_mode (struct ui_file *file, int from_tty, >> + struct cmd_list_element *c, const char *value) >> +{ >> + if (condition_evaluation_mode == condition_evaluation_auto) >> + fprintf_filtered (file, >> + _("Breakpoint condition evaluation " >> + "mode is %s (currently %s).\n"), >> + value, >> + breakpoint_condition_evaluation_mode ()); >> + else >> + fprintf_filtered (file, _("Breakpoint condition evaluation mode is %s.\n"), >> + value); >> +} >> + > Is it a good idea to show "gdb" or "stub" rather than "auto"? After > all, as you explained elsewhere, the translation is not 100% accurate, > depending on the specifics of each individual condition. This will only show what the global setting is. If "auto", it will show "auto", but will also show what GDB is currently using given the available features. If the user decides to switch to either "gdb" or "stub" (or "target"), this is what will be displayed by this function. This one does not relate directly to what is shown in "info break". What is displayed in "info break" depends on the global setting and on the successful parsing of an expression into an agent expression, so it displays exactly where each location condition will be evaluated. If "gdb" is set, every condition will be evaluated by GDB. If "stub" (or "target") is set, there is no guarantee everything will be handled by the target (given the documented limitations), so the fallback is to evaluate conditions on GDB's side. I'm regretting the "info break" change a little, it seems to obscure rather than clarify. :-) >> + /* Print whether the remote stub is doing the breakpoint's condition >> + evaluation. If GDB is doing the evaluation, don't print anything. */ >> + if (loc&& is_breakpoint (b)&& loc->cond_bytecode >> + && breakpoint_condition_evaluation_mode () >> + != condition_evaluation_gdb) >> + { >> + ui_out_text (uiout, " ("); >> + ui_out_field_string (uiout, "condeval", >> + breakpoint_condition_evaluation_mode ()); > I suggest "cond.eval." instead of "condeval". Better yet, how about > "evaluated by"? Sounds good. I'll make that change. Fixed all the other comments. > Btw, what happens if I set the mode to "stub" and the sub does not > support this? Do I get any feedback, and if so, at what time? If the target (stub) does not advertise support for condition evaluation, nothing will happen. We will fallback to "gdb" mode automatically. Target-side condition evaluation only happens if: 1 - Target supports it (via ConditionalBreakpoints feature). 2 - breakpoint condition-evaluation is "auto" or "stub". There isn't a warning message though. I'll send an updated version of the patch with the fixes. Thanks -- Luis lgustavo@codesourcery.com