From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25164 invoked by alias); 31 Jul 2018 10:23:15 -0000 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 Received: (qmail 25154 invoked by uid 89); 31 Jul 2018 10:23:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=editor, screw X-HELO: relay.fit.cvut.cz Received: from relay.fit.cvut.cz (HELO relay.fit.cvut.cz) (147.32.232.237) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 31 Jul 2018 10:23:11 +0000 Received: from imap.fit.cvut.cz (imap.fit.cvut.cz [IPv6:2001:718:2:2901:0:0:0:238] (may be forged)) by relay.fit.cvut.cz (8.15.2/8.15.2) with ESMTPS id w6VAN6ut018366 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Tue, 31 Jul 2018 12:23:08 +0200 (CEST) (envelope-from jan.vrany@fit.cvut.cz) Received: from sao (02788439.bb.sky.com [2.120.132.57] (may be forged)) (authenticated bits=0 as user vranyj1) by imap.fit.cvut.cz (8.15.2/8.15.2) with ESMTPSA id w6VAN575027577 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT); Tue, 31 Jul 2018 12:23:06 +0200 (CEST) (envelope-from jan.vrany@fit.cvut.cz) Message-ID: <002372e4889b31236eb59b5a3414930cd0549346.camel@fit.cvut.cz> Subject: Re: [RFC] mi: add -a option to the "-data-disassemble" command From: Jan Vrany To: gdb-patches Cc: Tom Tromey Date: Tue, 31 Jul 2018 10:23:00 -0000 In-Reply-To: <87d0v469yy.fsf@tromey.com> References: <83effpubx5.fsf@gnu.org> <20180727075447.16243-1-jan.vrany@fit.cvut.cz> <87d0v469yy.fsf@tromey.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-07/txt/msg00804.txt.bz2 On Mon, 2018-07-30 at 09:04 -0600, Tom Tromey wrote: > > > > > > "Jan" == Jan Vrany writes: > > Jan> A CLI command "disassemble" allows use to specify a single > Jan> address - in that case function surrounding that address is > Jan> disassembled. > > Jan> high = parse_and_eval_address (oarg); > Jan> end_seen = 1; > Jan> break; > Jan> + case ADDR_OPT: > Jan> + addr = parse_and_eval_address (oarg); > Jan> + addr_seen = 1; > Jan> + break; > > The indentation looked slightly off here. > It could just be how the patch looked here, but could you double-check > just in case? My fault. My editor put 8 spaces instead of tab (\t) at the beginning of my lines. Will fix. > > Jan> + if (!((line_seen && file_seen && num_seen && !start_seen && !end_seen && !addr_seen) > Jan> + || (line_seen && file_seen && !num_seen && !start_seen && !end_seen && !addr_seen) > Jan> + || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen) > Jan> + || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen))) > > I suppose this ought to check for the case where either -s or -e is > given along with -a. That seems like an error. > I'm sorry I'm confused. Let me try to explain. There are (now) four forms: 1a) -f filename -l linenum 1b) -f filename -l linenum -n lines 2) -s start-addr -e end-addr 3) -a addr Command must have one of the above four forms, otherwise it's invalid. Each "line" in the code above checks one of the above form (in order as written here). Note, that there's negation at the very beginning, so the condition holds if the command has none of the four forms. Seems "good" to me. Makes sense? Perhaps it's easier to see with little reformatting (hope email won't screw it): if (! (( line_seen && file_seen && num_seen && !start_seen && !end_seen && !addr_seen) || ( line_seen && file_seen && !num_seen && !start_seen && !end_seen && !addr_seen) || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen) || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen))) error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n " "howmany]] | [-s startaddr -e endaddr] | [-a addr] ) [--] mode.")); Now, it appears to me that first two lines can be merged as forms 1a and 1b differ only in presence of lines. Also, I forgot to update the comment above. So, what about: /* Allow only filename + linenum (with how_many which is not - required) OR start_addr + end_addr. */ + required) OR start_addr + end_addr OR addr */ - if (!((line_seen && file_seen && num_seen && !start_seen && !end_seen) - || (line_seen && file_seen && !num_seen && !start_seen && !end_seen) - || (!line_seen && !file_seen && !num_seen && start_seen && end_seen))) + if (!( ( line_seen && file_seen && !start_seen && !end_seen && !addr_seen) + || (!line_seen && !file_seen && !num_seen && start_seen && end_seen && !addr_seen) + || (!line_seen && !file_seen && !num_seen && !start_seen && !end_seen && addr_seen))) this passes mi-disassemble.exp on my machine. > This "if" made me laugh. :-) I cannot say I was exactly laughing. But followed the crowed anyway. > > Jan> + mi_gdb_test "112-data-disassemble -a \$pc -- 0" \ > Jan> + "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \ > Jan> + "data-disassemble function around pc assembly only" > Jan> + > Jan> + mi_gdb_test "112-data-disassemble -a callee4 -- 0" \ > Jan> + "112\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"callee4\",offset=\"$decimal\",inst=\".*\"\}.*\]" > \ > Jan> + "data-disassemble function callee4 assembly only" > > Probably the second "112" should be "113". I don't know if you can > reuse tokens or not, but it seems simple and safe not to. Sure, will fix and send a patch once we agree on the "if". Thanks, Jan