From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109010 invoked by alias); 24 Aug 2018 15:09:51 -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 108984 invoked by uid 89); 24 Aug 2018 15:09:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 Aug 2018 15:09:48 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w7OF9gLK009773 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 24 Aug 2018 11:09:47 -0400 Received: by simark.ca (Postfix, from userid 112) id 69D181EB43; Fri, 24 Aug 2018 11:09:42 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 1BB5B1EB37; Fri, 24 Aug 2018 11:09:40 -0400 (EDT) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=_96c893c9983e7fbe3bbb7580aea3ffd2" Date: Fri, 24 Aug 2018 15:09:00 -0000 From: Simon Marchi To: John Darrington Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] gdb: Added builtin types for 24 bit integers. In-Reply-To: <20180824061125.7vjvlopdx4bstg4l@jocasta.intra> References: <20180823173526.26144-1-john@darrington.wattle.id.au> <7b7853c6462d8806bc4a2a743330a382@polymtl.ca> <20180823200349.gxeuad3ms3c2apei@jocasta.intra> <603c98bc68bec04acb84d809c838abb0@polymtl.ca> <20180824061125.7vjvlopdx4bstg4l@jocasta.intra> Message-ID: <0f42f2a6bd5c76d40242043ca8b65cf3@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00593.txt.bz2 --=_96c893c9983e7fbe3bbb7580aea3ffd2 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII; format=flowed Content-length: 2804 On 2018-08-24 02:11, John Darrington wrote: > On Thu, Aug 23, 2018 at 04:35:25PM -0400, Simon Marchi wrote: > > It is clear now, but somebody doing a git blame to know why 24-bit > integer types were added would only find the patch that adds them > by > itself and wonder who uses that. A little message like > > This patch adds 24-bit integer types, used when debugging on the > S12Z > architecture (added by a later patch in this series). > > clears that up. That might looks a bit silly, but I think it > helps in > the long run. > > I fully agree with you. I've worked on other projects however had a > different opinion - they insisted that the checkin comment NOT contain > any rationale for the change, instead it should just summarize what > changed. I find that rather pointless but anyway .... Well, if you look at our commit history, you'll see we like to be verbose :). > > It seems that up till now there has been no 24 bit targets, so > the > > other > > two patches as some necessary things to make that possible. > > Thanks. Coming back to the code of the patch, I was wondering if > these > 24-bit types are useful or even relevant for any other > architecture. > > There most certainly are plenty of 24 bit architectures especially in > the > embedded world - just apparently none that gdb currently supports :( > > Would it work if you only defined the types for s12z > architectures, > storing the reference in the gdbarch_tdep object? > > My first reaction is that it probably *could* be made to work, but not > in an elegant fashion. Somehow I'd have to avoid that gdb ever calls > the > read_encoded_value function. I'm not sure I understand. I was only talking about the definition of the int24_t and uint24_t types, not the handling of DW_EH_PE_udata3. From what I read, the C99 standard mandates that the 8, 16, 32 and 64 variants of the intX_t/uintX_t types exist. Other types (with other values of X) would be extensions. That's why I thought it would make sense to define that in the s12z-specific gdbarches only. In the end I don't really mind, but it just looks like the "clean" way to do it and doesn't seem really more difficult. Can you see if the attached diff (applied on top of your series) work for you? And as far as I understand, this is disconnected from the handling of DW_EH_PE_udata3. > I do concede that adding DW_EH_PE_udata3 might be problematic since > it's not part of the dwarf standard. An alternative might be to rework > the read_encoded_value function to not rely on the dwarf enums (all it > really cares about is the size of the target's address space. I'll take a look at that patch (2/3) separately and reply to it. Simon --=_96c893c9983e7fbe3bbb7580aea3ffd2 Content-Transfer-Encoding: base64 Content-Type: text/x-diff; name=0001-Move-builtin_uint24_t-type-to-s12z-tdep.c.patch Content-Disposition: attachment; filename=0001-Move-builtin_uint24_t-type-to-s12z-tdep.c.patch; size=2813 Content-length: 3815 RnJvbSBlMDkwMjczM2EyOTcyNmJhMTA3YzQxOTgwYmRiZDg1MDAyNjFjODUy IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBTaW1vbiBNYXJjaGkg PHNpbW9uLm1hcmNoaUBlcmljc3Nvbi5jb20+CkRhdGU6IEZyaSwgMjQgQXVn IDIwMTggMTE6MDM6NDUgLTA0MDAKU3ViamVjdDogW1BBVENIXSBNb3ZlIGJ1 aWx0aW5fdWludDI0X3QgdHlwZSB0byBzMTJ6LXRkZXAuYwoKLS0tCiBnZGIv Z2RidHlwZXMuYyAgfCAgNCAtLS0tCiBnZGIvZ2RidHlwZXMuaCAgfCAgMiAt LQogZ2RiL3MxMnotdGRlcC5jIHwgMTYgKysrKysrKysrKystLS0tLQogMyBm aWxlcyBjaGFuZ2VkLCAxMSBpbnNlcnRpb25zKCspLCAxMSBkZWxldGlvbnMo LSkKCmRpZmYgLS1naXQgYS9nZGIvZ2RidHlwZXMuYyBiL2dkYi9nZGJ0eXBl cy5jCmluZGV4IDA1YmY3YjEuLjY1YjEyMTEgMTAwNjQ0Ci0tLSBhL2dkYi9n ZGJ0eXBlcy5jCisrKyBiL2dkYi9nZGJ0eXBlcy5jCkBAIC01NDAyLDEwICs1 NDAyLDYgQEAgZ2RidHlwZXNfcG9zdF9pbml0IChzdHJ1Y3QgZ2RiYXJjaCAq Z2RiYXJjaCkKICAgICA9IGFyY2hfaW50ZWdlcl90eXBlIChnZGJhcmNoLCAx NiwgMCwgImludDE2X3QiKTsKICAgYnVpbHRpbl90eXBlLT5idWlsdGluX3Vp bnQxNgogICAgID0gYXJjaF9pbnRlZ2VyX3R5cGUgKGdkYmFyY2gsIDE2LCAx LCAidWludDE2X3QiKTsKLSAgYnVpbHRpbl90eXBlLT5idWlsdGluX2ludDI0 Ci0gICAgPSBhcmNoX2ludGVnZXJfdHlwZSAoZ2RiYXJjaCwgMjQsIDAsICJp bnQyNF90Iik7Ci0gIGJ1aWx0aW5fdHlwZS0+YnVpbHRpbl91aW50MjQKLSAg ICA9IGFyY2hfaW50ZWdlcl90eXBlIChnZGJhcmNoLCAyNCwgMSwgInVpbnQy NF90Iik7CiAgIGJ1aWx0aW5fdHlwZS0+YnVpbHRpbl9pbnQzMgogICAgID0g YXJjaF9pbnRlZ2VyX3R5cGUgKGdkYmFyY2gsIDMyLCAwLCAiaW50MzJfdCIp OwogICBidWlsdGluX3R5cGUtPmJ1aWx0aW5fdWludDMyCmRpZmYgLS1naXQg YS9nZGIvZ2RidHlwZXMuaCBiL2dkYi9nZGJ0eXBlcy5oCmluZGV4IGViN2Mz NjUuLjE0MDU5YWIgMTAwNjQ0Ci0tLSBhL2dkYi9nZGJ0eXBlcy5oCisrKyBi L2dkYi9nZGJ0eXBlcy5oCkBAIC0xNjExLDggKzE2MTEsNiBAQCBzdHJ1Y3Qg YnVpbHRpbl90eXBlCiAgIHN0cnVjdCB0eXBlICpidWlsdGluX3VpbnQ4Owog ICBzdHJ1Y3QgdHlwZSAqYnVpbHRpbl9pbnQxNjsKICAgc3RydWN0IHR5cGUg KmJ1aWx0aW5fdWludDE2OwotICBzdHJ1Y3QgdHlwZSAqYnVpbHRpbl9pbnQy NDsKLSAgc3RydWN0IHR5cGUgKmJ1aWx0aW5fdWludDI0OwogICBzdHJ1Y3Qg dHlwZSAqYnVpbHRpbl9pbnQzMjsKICAgc3RydWN0IHR5cGUgKmJ1aWx0aW5f dWludDMyOwogICBzdHJ1Y3QgdHlwZSAqYnVpbHRpbl9pbnQ2NDsKZGlmZiAt LWdpdCBhL2dkYi9zMTJ6LXRkZXAuYyBiL2dkYi9zMTJ6LXRkZXAuYwppbmRl eCA1MTY5MDI1Li40OGFmNDIyIDEwMDY0NAotLS0gYS9nZGIvczEyei10ZGVw LmMKKysrIGIvZ2RiL3MxMnotdGRlcC5jCkBAIC03Niw2ICs3NiwxMSBAQCBz dGF0aWMgY29uc3QgaW50IHJlZ19wZXJtW05fUEhZU0lDQUxfUkVHSVNURVJT XSA9CiAgICBSRUdfQ0NXCiAgIH07CiAKK3N0cnVjdCBnZGJhcmNoX3RkZXAK K3sKKyAgdHlwZSAqYnVpbHRpbl91aW50MjQ7Cit9OworCiBzdGF0aWMgY29u c3QgY2hhciAqCiBzMTJ6X3JlZ2lzdGVyX25hbWUgKHN0cnVjdCBnZGJhcmNo ICpnZGJhcmNoLCBpbnQgcmVnbnVtKQogewpAQCAtMTM4LDcgKzE0MywxMCBA QCBzMTJ6X3JlZ2lzdGVyX3R5cGUgKHN0cnVjdCBnZGJhcmNoICpnZGJhcmNo LCBpbnQgcmVnX25yKQogICAgIGNhc2UgMjoKICAgICAgIHJldHVybiBidWls dGluX3R5cGUgKGdkYmFyY2gpLT5idWlsdGluX3VpbnQxNjsKICAgICBjYXNl IDM6Ci0gICAgICByZXR1cm4gYnVpbHRpbl90eXBlIChnZGJhcmNoKS0+YnVp bHRpbl91aW50MjQ7CisgICAgICB7CisJc3RydWN0IGdkYmFyY2hfdGRlcCAq dGRlcCA9IGdkYmFyY2hfdGRlcCAoZ2RiYXJjaCk7CisJcmV0dXJuIHRkZXAt PmJ1aWx0aW5fdWludDI0OworICAgICAgfQogICAgIGNhc2UgNDoKICAgICAg IHJldHVybiBidWlsdGluX3R5cGUgKGdkYmFyY2gpLT5idWlsdGluX3VpbnQz MjsKICAgICBkZWZhdWx0OgpAQCAtMzQ3LDEwICszNTUsNiBAQCBjb25zdGV4 cHIgZ2RiX2J5dGUgczEyel9icmVha19pbnNuW10gPSB7MHgwMH07CiAKIHR5 cGVkZWYgQlBfTUFOSVBVTEFUSU9OIChzMTJ6X2JyZWFrX2luc24pIHMxMnpf YnJlYWtwb2ludDsKIAotc3RydWN0IGdkYmFyY2hfdGRlcAotewotfTsKLQog c3RhdGljIHN0cnVjdCBnZGJhcmNoICoKIHMxMnpfZ2RiYXJjaF9pbml0IChz dHJ1Y3QgZ2RiYXJjaF9pbmZvIGluZm8sCiAgICAgICAgICAgICAgICAgICAg IHN0cnVjdCBnZGJhcmNoX2xpc3QgKmFyY2hlcykKQEAgLTM1OCw2ICszNjIs OCBAQCBzMTJ6X2dkYmFyY2hfaW5pdCAoc3RydWN0IGdkYmFyY2hfaW5mbyBp bmZvLAogICBzdHJ1Y3QgZ2RiYXJjaF90ZGVwICp0ZGVwID0gKHN0cnVjdCBn ZGJhcmNoX3RkZXAgKikgeG1hbGxvYyAoc2l6ZW9mICp0ZGVwKTsKICAgc3Ry dWN0IGdkYmFyY2ggKmdkYmFyY2ggPSBnZGJhcmNoX2FsbG9jICgmaW5mbywg dGRlcCk7CiAKKyAgdGRlcC0+YnVpbHRpbl91aW50MjQgPSBhcmNoX2ludGVn ZXJfdHlwZSAoZ2RiYXJjaCwgMjQsIDEsICJ1aW50MjRfdCIpOworCiAgIC8q IFRhcmdldCBkYXRhIHR5cGVzLiAgKi8KICAgc2V0X2dkYmFyY2hfc2hvcnRf Yml0IChnZGJhcmNoLCAxNik7CiAgIHNldF9nZGJhcmNoX2ludF9iaXQgKGdk YmFyY2gsIDE2KTsKLS0gCjIuNy40Cgo= --=_96c893c9983e7fbe3bbb7580aea3ffd2--