From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47626 invoked by alias); 15 Aug 2016 09:22:10 -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 47615 invoked by uid 89); 15 Aug 2016 09:22:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:1951, sorted, H*F:U*nickc 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; Mon, 15 Aug 2016 09:22:07 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A92DE7F368; Mon, 15 Aug 2016 09:22:06 +0000 (UTC) Received: from [10.36.5.19] (vpn1-5-19.ams2.redhat.com [10.36.5.19]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u7F9M53I007200 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 15 Aug 2016 05:22:06 -0400 Subject: Re: [PATCH] sim: unify symbol table handling To: Mike Frysinger , gdb-patches@sourceware.org References: <20160813195500.25598-1-vapier@gentoo.org> From: Nick Clifton Message-ID: <9c6fd370-885a-4290-160c-e9898a87e051@redhat.com> Date: Mon, 15 Aug 2016 09:22:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160813195500.25598-1-vapier@gentoo.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-08/txt/msg00156.txt.bz2 Hi Mike, > Nick: can you double check the aarch64 & msp430 changes ? > And see if in general this makes sense to you ? Sure. > diff --git a/sim/aarch64/interp.c b/sim/aarch64/interp.c > [...] > /* Filter out (in place) symbols that are useless for disassembly. > COUNT is the number of elements in SYMBOLS. > Return the number of useful symbols. */ > > -static unsigned long > -remove_useless_symbols (asymbol **symbols, unsigned long count) > +static long > +remove_useless_symbols (asymbol **symbols, long count) I understand the change to a signed long, but personally I consider it a mistake. The number of symbols is always going to be a positive value, and the need for an error value could easily be handled using -1U instead of -1L. But the problem is endemic to the BFD library's symbol handling code, so I guess that it will have to stay. *sigh* > diff --git a/sim/aarch64/memory.h b/sim/aarch64/memory.h > -extern void mem_add_blk (sim_cpu *, uint64_t, char *, uint64_t, bfd_boolean); (I have no problem with this part of the patch, but just to note that it is nothing to do with unifying symbol table handling...) > +int > +trace_load_symbols (SIM_DESC sd) > +{ Wouldn't it make sense for trace_load_symbols to also remove useless symbols and then sort the table ? Isn't this something that all sims will want ? [The code for remove_useless_symbols in aarch64/interp.c is basically generic, not aarch64 specific]. > +bfd_vma > +trace_sym_value (SIM_DESC sd, const char *name) > +{ > [...] > + for (i = 0; i < STATE_PROG_SYMS_COUNT (sd); ++i) > + if (strcmp (asymbols[i]->name, name) == 0) > + return bfd_asymbol_value (asymbols[i]); If there was a flag to say that the symbol table was sorted, then this lookup could be done using bsearch(). So basically as far as I can see there is nothing wrong with the patch. Certainly not from an AArch64 of MSP430 point of view. Cheers Nick