From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 113801 invoked by alias); 15 Feb 2017 23:14:26 -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 113742 invoked by uid 89); 15 Feb 2017 23:14:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=char_ptr, stxvd2x, regnames, reg-names X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 15 Feb 2017 23:14:23 +0000 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v1FN8wpr145175 for ; Wed, 15 Feb 2017 18:14:21 -0500 Received: from e34.co.us.ibm.com (e34.co.us.ibm.com [32.97.110.152]) by mx0b-001b2d01.pphosted.com with ESMTP id 28mswt8da6-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 15 Feb 2017 18:14:21 -0500 Received: from localhost by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 15 Feb 2017 16:14:20 -0700 Received: from d03dlp03.boulder.ibm.com (9.17.202.179) by e34.co.us.ibm.com (192.168.1.134) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 15 Feb 2017 16:14:18 -0700 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id C55D819D8047; Wed, 15 Feb 2017 16:13:30 -0700 (MST) Received: from b01ledav03.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v1FNEHjE55771222; Wed, 15 Feb 2017 23:14:17 GMT Received: from b01ledav03.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EFB46B2050; Wed, 15 Feb 2017 18:14:16 -0500 (EST) Received: from otta.local (unknown [9.85.164.60]) by b01ledav03.gho.pok.ibm.com (Postfix) with ESMTP id 4E941B204D; Wed, 15 Feb 2017 18:14:16 -0500 (EST) Subject: Re: [PATCH, updated] Add support for setting disassembler-options in GDB for POWER, ARM and S390 To: Pedro Alves References: <867f4uccky.fsf@gmail.com> <65f2f8ce-450b-297a-dcab-7a8bc0ebc256@vnet.ibm.com> <77996338-961a-5a69-e41a-f1adbb3b23da@redhat.com> Cc: Yao Qi , gdb-patches@sourceware.org, Alan Modra , Ulrich Weigand , Eli Zaretskii , Nick Clifton , binutils From: Peter Bergner Date: Wed, 15 Feb 2017 23:14:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <77996338-961a-5a69-e41a-f1adbb3b23da@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17021523-0016-0000-0000-0000062F3E8E X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006623; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000203; SDB=6.00822493; UDB=6.00402382; IPR=6.00599945; BA=6.00005143; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014297; XFM=3.00000011; UTC=2017-02-15 23:14:20 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17021523-0017-0000-0000-000037707D01 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-02-15_12:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702150209 X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00428.txt.bz2 On 2/14/17 2:01 PM, Pedro Alves wrote: > - Maintain the intended git commit log as an integral part > of the patch, and include it in patch re-posts, so that > any revision of the patch can be reviewed as a > self-contained entity. There's probably some rationale for > some changes to the tests that is written down in some > earlier intro, but was lost meanwhile. I'm not a git expert, as I'm paid to work on gcc and we still use subversion. I am willing to learn though, so can you explain what you mean by the above? > - For each new revision of the patch, bump a v2, v3, etc. > revision number in the email subject, so that's easier > to find specific revisions, and to identify whic > email contains the latest version. Easily done, as I've been doing just that internally. I'm frightened to say that I'm at v25 and counting. :-( >> +manual and/or the output of @kbd{objdump --help}. The default value is ''. > > '' renders as a single double-quote, not two single-quotes. In any > case, I'd suggest saying instead "The default is the empty string", or > "The default is not options". Likewise in NEWS. Done. >> @@ -888,6 +896,8 @@ gdb_buffered_insn_length_init_dis (struct gdbarch *gdbarch, >> di->endian = gdbarch_byte_order (gdbarch); >> di->endian_code = gdbarch_byte_order_for_code (gdbarch); >> >> + di->application_data = gdbarch; > > Is this above used anywhere? I think this might be left over from something I was trying and forgot to remove. I'll remove it. >> + char *options = remove_whitespace_and_extra_commas (prospective_options); >> + char *iter, opt[256]; > > Can we get rid of the hardcoded (and not enforced) limit? > Maybe just use strtok_r instead of FOR_EACH_DISASSEMBLER_OPTION? > >> + /* Verify we have valid disassembler options. */ >> + FOR_EACH_DISASSEMBLER_OPTION (opt, iter, options) The problem with strtok{,_r} is that it is destructive to the option string, so we'd have to dup the string before scanning it, which doesn't seem very elegant either. We would also need to remember to free the dup'd string as well when we were done with it. The reason I copied the parsed option into a char array was that I needed a null terminated string that I could use with strcmp. Unfortunately, the POWER port has several options that have a common prefix: Eg: "e500" & "e500mc", "ppc" & "ppc32" and "ppc64", etc. ...which strncmp cannot disambiguate, because it doesn't enforce the two strings have the same length. I have two ideas, one is to write our own strcmp that treats option delimiters like ',' just like '\0'. The other idea would be to modify the disassembler_options string so that we use '\0' as the delimiter between the different options. We'd need an extra '\0' at the end to know when we've run out of options though. If we did this, then we could just use standard strcmp on the options. >> +/* A completion function for "set disassembler". */ >> + >> +static VEC (char_ptr) * >> +disassembler_options_completer (struct cmd_list_element *ignore, >> + const char *text, const char *word) >> +{ >> + struct gdbarch *gdbarch = get_current_arch (); >> + const disasm_options_t *opts = gdbarch_valid_disassembler_options (gdbarch); >> + >> + if (opts != NULL) >> + { >> + /* Only attempt to complete on the last option text. */ >> + const char *separator = strrchr (text, ','); >> + if (separator != NULL) >> + text = separator + 1; > > I believe 'word' points past the comma already? 'word' does point past the last comma, but sometimes, it points well past the last comma. For example, if I type: set disassembler-options force-thumb, reg-name-g ...then 'text' will equal "force-thumb, reg-name-g" and 'word' will equal "g". To get the completer to match "reg-names-gcc", I have to modify 'text' to be "reg-names-g". >> + while (ISSPACE (*text)) >> + text++; > > skip_spaces_const. Done. >> + /* Add the command that controls the disassembler options. */ >> + cmd = add_setshow_string_noescape_cmd ("disassembler-options", no_class, >> + &prospective_options, _("\ >> +Set the disassembler options.\n\ >> +Usage: set disassembler