From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 1/5] symbols: prefix static symbols with their source file names Date: Wed, 28 Oct 2015 13:42:49 +0000 Message-ID: <5630D0D9.9060405@citrix.com> References: <562E12E802000078000AE785@prv-mh.provo.novell.com> <562E217102000078000AE89F@prv-mh.provo.novell.com> <5630C5B9.1090807@citrix.com> <5630DADA02000078000AF87F@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZrR0A-0001Fq-AE for xen-devel@lists.xenproject.org; Wed, 28 Oct 2015 13:42:54 +0000 In-Reply-To: <5630DADA02000078000AF87F@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , Stefano Stabellini , Ian Jackson , Tim Deegan , Ian Campbell , xen-devel List-Id: xen-devel@lists.xenproject.org On 28/10/15 13:25, Jan Beulich wrote: >>>> On 28.10.15 at 13:55, wrote: >> On 26/10/15 11:49, Jan Beulich wrote: >>> This requires adjustments to the tool generating the symbol table and >>> its as well as nm's invocation. >>> >>> Note: Not warning about duplicate symbols in the EFI case for now, as >>> a binutils bug causes misnamed file name entries to appear in EFI >>> binaries' symbol tables when the file name is longer than 18 chars. >>> (Not doing so also avoids other duplicates getting printed twice.) >>> >>> Signed-off-by: Jan Beulich >> Should warn_dups become fatal once the patches to fix these... >> >> Duplicate symbol 'memory.c#get_reserved_device_memory' (ffff82d080140183 >> != ffff82d080118b5b) >> Duplicate symbol 'platform_hypercall.c#__maddr_to_virt' >> (ffff82d08023a3a2 != ffff82d080167e66) >> Duplicate symbol 'platform_hypercall.c#__virt_to_maddr' >> (ffff82d08023a401 != ffff82d080167ec5) >> Duplicate symbol 'platform_hypercall.c#cpumask_check' (ffff82d08023a489 >> != ffff82d080167f4d) >> >> have been committed, to avoid accidental reintroduction? > They all went in already. Or are you saying you saw these on top > of what is in staging right now? Current staging right now andrewcoop@andrewcoop:/local/xen.git/xen$ git log --oneline staging^.. 69bdd7f symbols: prefix static symbols with their source file names bf0d492 x86/mm: don't call HVM-only function for PV guests However, those duplicates are from the compat code, which I didn't specifically take your patch 2 for. > > However - no to the question here. For one, there's nothing fatal > about it the absence of xSplice. And even with xSplice I'm not sure > this really should be fatal at the build stage. For the non-xSplice case, the worst which can happen is indeed just a harder-to-read stack trace. However, for the xSplice case, we really should take reasonable steps to make patching easier, and that includes avoiding duplicate symbols. As a future user of xSplice, I would definitely like an option to fail the hypervisor build if it will result in a hard-to-patch binary. > What should force > people to at least look closely would be a runtime patch using such > a symbol. And second, ... > >> I note that even sysv format doesn't appear to provide directory >> information, so we still cant distinguish duplicate static symbols in >> identically named source files in difference directories, but hopefully >> that should be very rare. > ... this. I actually see one with some gcc versions (an inline function > not expanded inline in two different cpufreq.c). An inline function with an ASSERT/BUG or alternative by any chance? GCC appears to aggressively out-of-line these, which is why had to sprinkle always_inline to helpers such as stac()/clac() ~Andrew