From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B54A021B03841 for ; Wed, 24 May 2017 14:44:05 -0700 (PDT) Received: by mail-it0-x231.google.com with SMTP id c15so46771544ith.0 for ; Wed, 24 May 2017 14:44:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=8g/6x4LvAIfNPW0oQzY6isxSHfK2HndpQN3pnniPhXA=; b=bDPgbuZR/HGg0OPqIJoF8hMojmZgBnmrAf3EAHFV56REkaI5yPw+kZbwLo19zFOMuF AEkIzXm0pn/dAVwRn7pKgAKyqqUpHtxhHxv1JAFWrrUaHGNf6MRtCLTvHkLBSDbnCqpk K9ApBUaq0cilr0Jnpvaejtnh5NiX03PDt1pN4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=8g/6x4LvAIfNPW0oQzY6isxSHfK2HndpQN3pnniPhXA=; b=OVD6uBGEaV56JfaS1C17zpjx+5XTKts6cUCFTzJR3669825AUTE3PejP1BUvAdfCy7 Yu/zVDCaPnbkateiWSu3zQF4+1NC6rK0KEFbhEkkp53ca4NtDx7Duh+l2ryoj4zQiRiI JJ67Dh4gwSNbNAs6gOVYpSL78ikfxDWfPLyMwBR6xGg3Ru4yswD/Mv7u9IXTThkkJ+XY 7fWfEt4NimiU3xJTNgp2m/+PZPx5IRNNSveAYzbyDcziDJ4oWHwlipJiZoxNeQuX5SKi jc3gmZLHoFwNsj/DWuu9zZmd5XRHANoWHxRGPDEqDXSUxezpVS/ynIL4BgEzO5bkTmqT 2z5w== X-Gm-Message-State: AODbwcBsXIAF2eAO76p9pc3Q1TXp4GiqCtIyCdpGBNZNV4ca2mvODTZz zCvTcrJ2Nt5dtZnOf3AWQaWWSi/50z+a X-Received: by 10.36.224.133 with SMTP id c127mr11142338ith.73.1495662244945; Wed, 24 May 2017 14:44:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.164.24 with HTTP; Wed, 24 May 2017 14:44:04 -0700 (PDT) In-Reply-To: References: <1495581673-10788-1-git-send-email-michael.d.kinney@intel.com> From: Ard Biesheuvel Date: Wed, 24 May 2017 14:44:04 -0700 Message-ID: To: "Kinney, Michael D" Cc: Laszlo Ersek , "Wu, Hao A" , "edk2-devel@lists.01.org" , "afish@apple.com" , "Fan, Jeff" Subject: Re: [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 May 2017 21:44:06 -0000 Content-Type: text/plain; charset="UTF-8" On 24 May 2017 at 13:18, Kinney, Michael D wrote: > Laszlo, > > I agree with the request to add 'static' to the variable declaration > in the SecPeiDebugAgentLib. The variable name change will be retained > because the same symbol name can still be confusing when debugging. > > The part that is more concerning is why this duplicate symbol reference > was not triggered by MSFT or GCC tool chain families, so I did some > more investigation from the MSFT side this morning. > > My first thought was that the optimizer in the compiler/linker was > optimizing away the duplicate symbol before the final link stage, > so I tried -b NOOPT builds. That also did not trigger a duplicate > symbol error and the MAP file contains only the single reference to > _mMemoryDiscoveredNotifyList from the DxeIpl. > > 0002:000001b8 _mMemoryDiscoveredNotifyList 000164f8 DxeIpl:DxeLoad.obj > > I then evaluated what functions in the DebugAgentLib the DxeIpl > is using. It turns out that it is only using a single function > SaveAndSetDebugTimerInterrupt() that is implemented in > > SourceLevelDebugPkg\Library\DebugAgent\DebugAgentCommon\DebugAgent.c > > The mMemoryDiscoveredNotifyList duplicate symbol is implemented in > > SourceLevelDebugPkg\Library\DebugAgent\SecPeiDebugAgent\SecPeiDebugAgentLib.c > > Even with a MSFT -b NOOPT build, when I look at the MAP file, the > linker only pulls in symbols from the obj in the DebugAgentLib that > contains the one SaveAndSetDebugTimerInterrupt() function. The symbols > from the other objs in the DebugAgentLib are not included in the final > DxeIpl PE/COFF image. > > 0001:000081b0 _InitializeDebugTimer 00008410 f SecPeiDebugAgentLib:DebugTimer.obj > 0001:00008330 _IsDebugTimerTimeout 00008590 f SecPeiDebugAgentLib:DebugTimer.obj > 0001:000083d0 _SaveAndSetDebugTimerInterrupt 00008630 f SecPeiDebugAgentLib:DebugTimer.obj > > So it appears that even with -b NOOPT builds, the linker is > filtering out unreferenced objs which explains why the duplicate > symbol error is not triggered. > It does not need to filter out what it never considered for inclusion in the first place. That is why it is called a library: only objects that satisfy some as yet unresolved symbol reference will get pulled in. > The XCODE5 tool chain appears to be evaluating symbols > across all objs in a lib and detects a duplicate symbol. > > The question is what is the required linker behavior for EDK II > builds? Require library filtering at obj level, or require no > duplicates across all objs in all libs? > Require no *externally visible* duplicates at all. That is really the only scaleable solution. Libraries declare their externally visible symbols in their .h file, and ideally, nothing else should be exposed. I know this is infeasible in the general case, due to the fact that libraries typically consist of several objects, but it should be the goal not to export anything if we can avoid it. Note that this applies to functions as well as variables