From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104555 invoked by alias); 3 Oct 2016 20:25:24 -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 104529 invoked by uid 89); 3 Oct 2016 20:25:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=unavailable version=3.3.2 spammy=succeeds, disassembly, print_insn_*, ppch X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Oct 2016 20:25:21 +0000 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u93KN7ZO141261 for ; Mon, 3 Oct 2016 16:25:20 -0400 Received: from e19.ny.us.ibm.com (e19.ny.us.ibm.com [129.33.205.209]) by mx0a-001b2d01.pphosted.com with ESMTP id 25ur8bcxd1-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 03 Oct 2016 16:25:20 -0400 Received: from localhost by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 3 Oct 2016 16:25:19 -0400 Received: from d01dlp02.pok.ibm.com (9.56.250.167) by e19.ny.us.ibm.com (146.89.104.206) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 3 Oct 2016 16:25:17 -0400 Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 1E6E36E8040; Mon, 3 Oct 2016 16:24:53 -0400 (EDT) Received: from b01ledav004.gho.pok.ibm.com (b01ledav004.gho.pok.ibm.com [9.57.199.109]) by b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u93KPLDA14352794; Mon, 3 Oct 2016 20:25:21 GMT Received: from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3093311204B; Mon, 3 Oct 2016 16:25:16 -0400 (EDT) Received: from otta.local (unknown [9.85.173.241]) by b01ledav004.gho.pok.ibm.com (Postfix) with ESMTP id CF58C112054; Mon, 3 Oct 2016 16:25:15 -0400 (EDT) Subject: Re: [PATCH, RFC] Add support for choosing disassembler cpu in GDB for POWER. To: Ulrich Weigand References: <20160930161908.6A43511C24D@oc8523832656.ibm.com> Cc: Alan Modra , gdb-patches@sourceware.org, binutils From: Peter Bergner Date: Mon, 03 Oct 2016 20:25:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20160930161908.6A43511C24D@oc8523832656.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16100320-0056-0000-0000-00000186DC2C X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005849; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000186; SDB=6.00763964; UDB=6.00364721; IPR=6.00539617; BA=6.00004779; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012861; XFM=3.00000011; UTC=2016-10-03 20:25:18 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16100320-0057-0000-0000-000005B9F2F0 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-03_11:,, 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-1609280000 definitions=main-1610030348 X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00022.txt.bz2 On 9/30/16 11:19 AM, Ulrich Weigand wrote: > The implementation in the patch does appear to be a bit ad-hoc, however :-) > Why would we want to pass that information via a new global variable, if > there is already an element "disassembler_options" in the struct > disassemble_info that GDB passes to bfd? See e.g. i386_print_insn. Yes, I saw that code. The problem is that same solution won't work for us, since print_insn_*() doesn't look at info->disassembler_options at all. Instead, it uses the dialect value which we initialize from info->disassembler_options in powerpc_init_dialect(), but that happens only once and well before gdb_print_insn_powerpc() is called. That means changing info->disassembler_options in gdb_print_insn_powerpc() won't affect anything. Now we could recompute the dialect each time we call gdb_print_insn_powerpc(), but that is done for each insn we emit and scanning through the ppc_opts table every time would be very expensive, since our table is pretty big. The only other method would be for set_disassembler_cpu() to somehow convert its arg into a dialect value that gdb_print_insn_powerpc() could stuff into the struct disassemble_info each time, which would be fairly inexpensive. However, the dialect value is computed by powerpc_init_dialect which takes a struct disassemble_info pointer and at this point in the disassembly process, it hasn't been created yet. I suppose we could try and create a fake one to pass in. So given that gdb_print_insn_powerpc() can't affect the disassembly cpu by changing info->disassembler_options and set_disassembler_cpu() isn't passed a struct disassemble_info pointer,that's why I resorted to the global variable. Given your reaction to the patch as is, how about if I try and create a temporary struct disassemble_info to pass to powerpc_init_dialect() and stash the resulting dialect in a global var that is private to gdb so that gdb_print_insn_powerpc() can stuff it into into the struct disassemble_info? Would that satisfy your reservations? Note to self, we should probably sort the ppc_opts table with the more commonly used (ie, newer) cpus listed first rather than adding them at the end of the table like we have. > I'm also not really happy about the tight integration of opcode/ppc.h > and the ppc_opts struct into GDB code ... Can't we ask a BFD routine > whether a particular CPU option is valid? E.g. by just making a "test" > call to print_insn_* and see if it succeeds? As I said above, print_insn_*() doesn't look at info->disassembler_options, so we can't use that method, but I could add a new BFD routine to do what you ask. I did it this way, so I could emit the valid cpus values in the "show disassembler-cpu" output. I suppose I could also move that to BFD and just call it from GDB. > Apart from those implementation details, I'm wondering whether we might > want to generalize the feature to allow setting any disassembler option, > not just CPU levels. Also, this could really be useful on any platform, > not just Power :-) But I see that some other architectures already use > info->disassembler_options to pass some special options, which might > make the generic solution more complex. Therefore I'd be OK with just > doing the Power implementation for now. I'm not against it, but as it is today, the POWER port only expects info->disassembler_options to hold cpu values. If we were to allow other options, we'd have to parse them more carefully. I'm also not sure whether like the disassembly dialect, if we might have problems changing the values due to when they get parsed versus when they are used. Peter