From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 65127 invoked by alias); 16 Feb 2017 01:59:04 -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 65047 invoked by uid 89); 16 Feb 2017 01:59:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=incredibly, Easily, H*i:sk:b6cac06, H*f:sk:b6cac06 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; Thu, 16 Feb 2017 01:59:01 +0000 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v1G1ww1t147016 for ; Wed, 15 Feb 2017 20:58:59 -0500 Received: from e38.co.us.ibm.com (e38.co.us.ibm.com [32.97.110.159]) by mx0b-001b2d01.pphosted.com with ESMTP id 28mygwxmdu-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 15 Feb 2017 20:58:59 -0500 Received: from localhost by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 15 Feb 2017 18:58:58 -0700 Received: from d03dlp03.boulder.ibm.com (9.17.202.179) by e38.co.us.ibm.com (192.168.1.138) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 15 Feb 2017 18:58:55 -0700 Received: from b03cxnp08026.gho.boulder.ibm.com (b03cxnp08026.gho.boulder.ibm.com [9.17.130.18]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 173B819D8042; Wed, 15 Feb 2017 18:58:08 -0700 (MST) Received: from b03ledav006.gho.boulder.ibm.com (b03ledav006.gho.boulder.ibm.com [9.17.130.237]) by b03cxnp08026.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v1G1wtuX10682644; Wed, 15 Feb 2017 18:58:55 -0700 Received: from b03ledav006.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2DE7FC6037; Wed, 15 Feb 2017 18:58:55 -0700 (MST) Received: from otta.local (unknown [9.85.164.60]) by b03ledav006.gho.boulder.ibm.com (Postfix) with ESMTP id E25CFC6043; Wed, 15 Feb 2017 18:58:53 -0700 (MST) 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: Thu, 16 Feb 2017 01:59: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: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17021601-0028-0000-0000-0000070F6396 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006624; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000203; SDB=6.00822547; UDB=6.00402415; IPR=6.00599999; 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.00014299; XFM=3.00000011; UTC=2017-02-16 01:58:57 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17021601-0029-0000-0000-0000339CE939 Message-Id: <7458110b-366a-ee02-9668-3ebef0dfd8cf@vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-02-15_14:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702160018 X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00433.txt.bz2 On 2/15/17 6:21 PM, Pedro Alves wrote: > In practice, what this means is just that you use > "git commit --ammend", and edit the commit message to include the > rationale for the patch, and update it / maintain it whenever the patch > changes in a way that might change the description/rationale for > the patch. Ah, the --ammend is the part I was missing. Thanks. >> Easily done, as I've been doing just that internally. >> I'm frightened to say that I'm at v25 and counting. :-( > > Internal revisions don't count, only public submissions. :-) Well the number of public submissions isn't too far off my internal patches. :-) >> Eg: "e500" & "e500mc", "ppc" & "ppc32" and "ppc64", etc. >> >> ...which strncmp cannot disambiguate, because it doesn't enforce the >> two strings have the same length. > > You could handle that with: > > if (optlen == strlen (valid_options->name[i]) > && strncmp (opt, optlen, valid_options->name[i]) == 0) Yes, but that involves two scans over the string, so... >> I have two ideas, one is to write our own strcmp that treats option >> delimiters like ',' just like '\0'. > > Or something like that. ... I will attempt to try this first, since it seems like the easiest solution. Is there a preferred location for a function like this to go into? Looking around, it seems maybe common/common-utils.c? >> 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. > > Can't see how this would work without an interning step, > given the delimiters come from user input. To be honest, I'd prefer an option string like this, since it would allow for an easy FOR_EACH_DISASSEMBLER_OPTION macro and we could use standard strcmp, but I too was wondering how I could easily add 1 extra char to the string to hold the extra null byte which is needed to identify the end of the option strings. That's why I was going to try my first suggestion first. > Please just check that > > (gdb) complete set disassembler-options force-thumb, reg-name-g > > does the right thing. It does: (gdb) complete set disassembler-options force-thumb, reg-names-g set disassembler-options force-thumb, reg-names-gcc Ditto for the ARM specific set arm disassembler option (which uses short option names): (gdb) complete set arm disassembler g set arm disassembler gcc > This mention of "breaking the test cases up in many smaller > test cases" is the sort of rationale that would be nice to put in the > commit log. :-) I wasn't actually sure that that's what you're > doing, and why. Seemed like you've changed the tests to avoid > hardcoding offsets too? (it would probabably be clearer to do that > with a separate preparatory patch, with the added advantage that > that part could probably be merged to master quickly, but fine > to keep it together if you prefer.) Well that was the whole rational for this patch. I had made some changes to GAS to modify the support for our POWER9 cpu and thought I'd be a good citizen and update the GDB testsuite while I was at it. Unfortunately, the file was incredibly large and hard to update due to the hard coded file offsets. Also, the test case tested for power7, power8 and power9 all in the same test case. I thought it best if we test each cpu separately and that we should be able to enforce the disassembler options when doing it (in case we deprecate some instruction(s) on newer cpus). And this is what led to the patch we have now. :-) Peter