From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94139 invoked by alias); 14 Jun 2017 16:37:54 -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 94039 invoked by uid 89); 14 Jun 2017 16:37:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=axis, respected, noticeable, H*M:f315 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Jun 2017 16:37:52 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 33EFC33458B; Wed, 14 Jun 2017 16:37:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 33EFC33458B Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 33EFC33458B Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 62C417F566; Wed, 14 Jun 2017 16:37:54 +0000 (UTC) Subject: Re: [PATCH v2] arc: Select CPU model properly before disassembling To: Simon Marchi , Anton Kolesov References: <39A54937CC95F24AA2F794E2D2B66B135874F238@DE02WEMBXB.internal.synopsys.com> <20170613134951.29359-1-Anton.Kolesov@synopsys.com> <887411aaab641fb92f809eadce4d9011@polymtl.ca> <39A54937CC95F24AA2F794E2D2B66B1358756E48@DE02WEMBXB.internal.synopsys.com> Cc: gdb-patches@sourceware.org, Francois Bedard From: Pedro Alves Message-ID: <02d18307-f315-bc64-d9f3-550feaf01b41@redhat.com> Date: Wed, 14 Jun 2017 16:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00430.txt.bz2 On 06/14/2017 05:27 PM, Simon Marchi wrote: > On 2017-06-14 16:02, Anton Kolesov wrote: >> This is the same thing as in {arm,rs6000,s390-linux}-tdep.c - they all >> use >> global static variable, which would be shared across multiple gdbarch >> instances. So when options are changed that would change that for any >> gdbarch >> of that architecture. RS6000 and S390 don't even assign this variable >> any value >> - it completely managed by the disasm.c. Although in ARC case there is >> a memory >> leak, because arc_disassembler_options is only assigned by arc-tdep.c and >> never freed - that's because I didn't properly understood what other >> arches >> do - they don't free options as well, but they also don't allocate >> them in >> each gdbarch_init, because there usecase is somewhat different. > > Indeed, set_disassembler_options takes care of the > allocation/deallocation when the user changes the options. > >> I'm not sure >> how big of a problem this leak is, because outside of a GDB test suite >> I don't >> think there are a lot of practical cases where same GDB instance is >> reused to >> connect/disconnect, so leak wouldn't be noticeable. I think, I can make >> things better for ARC, by moving arc_disassembler_options into the >> gdbarch_tdep, then each gdbarch for ARC will have its own options, but >> that >> would mean behavior different from other arches which support >> disassembler_options - there options are shared across gdbarches of >> architecture. Should I do that in V3? > > Ah, I did not understand previously that sharing the disassembler > options between all gdbarches of an architecture is the intended > behavior. Having different behavior across architectures for this > wouldn't be a good idea, I think. Also, as of today, the disassembler > options are very much user driven. The architecture family can set some > default at startup (ARM, for example, sets "reg-names-std"), but after > that it's all up to the user. Right, in a given debug session you can have different gdbarch instances at play even for the same cpu architecture. E.g., the gdbarch determined for the binary vs the gdbarch created for the target description. > > In your case, it's the opposite along those two axis: you want GDB to > set disassembler options in the back of the user, and you want > disassembler options (at least some of them) to be gdbarch-specific. To > support that correctly, I think we should first define what behavior we > want from GDB. For example: > > - if the user doesn't specify any disassembler-options, it's probably > right to automatically set it for them. > - if the user provides a disassembler option other than cpu= and then > connects to a gdbserver, we probably want to add the cpu= to what they > have specified. > - if they connect first and then set another, unrelated option, do we > keep the cpu= option or does it get lost? > - if they set a particular cpu= option and then connect to a gdbserver > that reports something else, which one do we choose? > - if they connect to a gdbserver that reports an architecture and we > add a cpu= for it, then they connect to a gdbserver that doesn't report > an architecture, do we keep the previous cpu= or not? > - what happens with the various options when we switch gdbarch, do we > keep the user-provided ones but flush the GDB-inferred ones? I'd think that the "set/show disassembler-options" setting wouldn't ever be changed by connecting to a target and that whatever the user explicitly set should be respected. I.e., gdb / opcodes would choose which cpu to use for disassembly based on this logical sequence: - if user specified cpu, use that, - otherwise, if tdesc specified cpu, use that, - otherwise, infer cpu from bfd/elf section, - otherwise, use default cpu Thanks, Pedro Alves