From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102914 invoked by alias); 17 Jul 2019 01:21:31 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 102903 invoked by uid 89); 17 Jul 2019 01:21:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-21.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 spammy= X-Spam-Status: No, score=-21.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: smtp2200-217.mail.aliyun.com Received: from smtp2200-217.mail.aliyun.com (HELO smtp2200-217.mail.aliyun.com) (121.197.200.217) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 17 Jul 2019 01:21:29 +0000 X-Alimail-AntiSpam:AC=CONTINUE;BC=0.07436326|-1;CH=green;DM=CONTINUE|CONTINUE|true|0.0457982-0.00241046-0.951791;FP=0|0|0|0|0|-1|-1|-1;HT=e02c03310;MF=han_mao@c-sky.com;NM=1;PH=DS;RN=2;RT=2;SR=0;TI=SMTPD_---.EyjGI9A_1563326486; Received: from localhost(mailfrom:han_mao@c-sky.com fp:SMTPD_---.EyjGI9A_1563326486) by smtp.aliyun-inc.com(10.147.41.187); Wed, 17 Jul 2019 09:21:26 +0800 Date: Wed, 17 Jul 2019 01:21:00 -0000 From: Mao Han To: Mark Wielaard Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH V4 0/1] Add C-SKY support Message-ID: <20190717011951.GA3584@vmh-VirtualBox> References: <28258bf60492011cde0aaaded27ca6f4b6914de1.camel@klomp.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28258bf60492011cde0aaaded27ca6f4b6914de1.camel@klomp.org> User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2019-q3/txt/msg00060.txt.bz2 On Tue, Jul 16, 2019 at 03:48:22PM +0200, Mark Wielaard wrote: > > > > These attributes including cpu name and some other ISA related > > > > descriptions. > > > > Some thing like: > > > > CSKY_ARCH_NAME: "ck810" > > > > CSKY_CPU_NAME: "ck810f" > > > > CSKY_ISA_FLAG: 0x12345678 > > > > CSKY_ISA_EXT_FLAG: 5 > > > > They are not documented yet. > > > > I'v ask the person who is responsible for these to update the ABI > > > > documents, but I think it will take a quite long time for them to > > > > do that. They are quite busy at present. > > > > > > OK. If you can add that tweak to src/readelf.c and add an > > > check_object_attribute hook that handles the above attributes that > > > would be good. > > > > > > Ideally you also add a testcase for tests/readelf-A.sh > > > Some of those tests cheat and create the attributes by hand. > > > But it would be nice if you could generate a small .o file with the > > > latest toolchain to be used as testcase in some other tests. > > > > I'm not sure about how to handle different data type here. It seems > > only tag_name is required when data type is string, I could not > > found how to handle int here. > > The binary with csky.attribute currently can not be generate with > > public > > released toolchain, so I don't know how to add the testcase. > > OK, lets add a testcase once this has gone upstream in the rest of the > toolchain. Good point about the tag representing a string or number. > But I think this is (accidentially) handled correctly already. See this > comment in readelf.c (print_attributes): > > /* GNU style tags have either a uleb128 value, > when lowest bit is not set, or a string > when the lowest bit is set. > "compatibility" (32) is special. It has > both a string and a uleb128 value. For > non-gnu we assume 6 till 31 only take ints. > XXX see arm backend, do we need a separate > hook? */ > > We probably need another hook one day, but it looks like csky follows > this assumption (4 and 5 are strings, 6 and 7 are numbers). Yes, csky follows this assumption, so it is handled correctly already. > There is one bug in the implementation though. The vendor check is > wrong, checks for "gnu", should obviously be "csky": > > diff --git a/backends/csky_attrs.c b/backends/csky_attrs.c > index 9b236f1c..177f0ba2 100644 > --- a/backends/csky_attrs.c > +++ b/backends/csky_attrs.c > @@ -43,7 +43,7 @@ csky_check_object_attribute (Ebl *ebl __attribute__ ((unused)), > const char **tag_name, > const char **value_name __attribute__ ((unused))) > { > - if (!strcmp (vendor, "gnu")) > + if (!strcmp (vendor, "csky")) > switch (tag) > { > case 4: > > > Tested on x86 > > ============================================================================ > > Testsuite summary for elfutils 0.176 > > ============================================================================ > > # TOTAL: 209 > > # PASS: 204 > > # SKIP: 5 > > # XFAIL: 0 > > # FAIL: 0 > > # XPASS: 0 > > # ERROR: 0 > > ============================================================================ > > > > Changes since v1: > > - Add the Signed-off-by line and the copyright > > > > Changes since v2: > > - move changelog to corresponding entries > > - correct core dump registers size > > - remove unused fpu DWARF register > > > > Changes since v3: > > - add testfilecsky.bz2 and hello_csky.ko.bz2 > > - add csky_check_object_attribute > > > > Mao Han (1): > > Add backend support for C-SKY > > The new patch looks really good. Thanks. The addition of the testcases > really helps showing things look good. I can make that one small fix > s/gnu/csky/ in csky_attrs.c if you agree that is what was intended. > Then I'll push it to master. Yes, please. Thanks for your review and help improveing the patch. > And after the next release we can add some more testcases and handle to > register mappings more correctly. OK. Thanks, Mao Han