From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6639321B02839 for ; Thu, 7 Dec 2017 03:14:18 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1929968689; Thu, 7 Dec 2017 11:18:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3E0F57843E; Thu, 7 Dec 2017 11:18:49 +0000 (UTC) To: Ard Biesheuvel , Liming Gao Cc: "edk2-devel@lists.01.org" , Michael Kinney , Hao Wu , Andrew Fish , Jeff Fan References: <1512632891-5236-1-git-send-email-liming.gao@intel.com> From: Laszlo Ersek Message-ID: <5b49394f-cb68-c00a-6174-a14a619e02c5@redhat.com> Date: Thu, 7 Dec 2017 12:18:48 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 07 Dec 2017 11:18:51 +0000 (UTC) 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: Thu, 07 Dec 2017 11:14:18 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/07/17 09:46, Ard Biesheuvel wrote: > On 7 December 2017 at 07:48, Liming Gao wrote: >> From: Michael Kinney >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=573 >> https://bugzilla.tianocore.org/show_bug.cgi?id=796 >> >> The same issue is reported again by GCC. Resend this patch again. >> This patch renames the duplicated function name to fix it. >> >> The SecPeiDebugAgentLib uses the global variable >> mMemoryDiscoveredNotifyList for a PPI notification on >> the Memory Discovered PPI. This same variable name is >> used in the DxeIplPeim for the same PPI notification. >> >> The XCODE5 tool chain detects this duplicate symbol >> when the OVMF platform is built with the flag >> -D SOURCE_DEBUG_ENABLE. >> >> The fix is to rename this global variable in the >> SecPeiDebugAgentLib library. >> > > No, the fix is to make it STATIC unless it *needs* to be a global. > Is that the case here? I agree with you (of course), but Mike explained earlier (if I recall correctly -- and perhaps you remember too) that giving internal linkage to global variables (i.e., making them STATIC) messes either with debuggability under VS, or else defeats "GLOBAL_REMOVE_IF_UNREFERENCED". (I'm not sure which one of the two.) So, I've settled on considering "extern by default" just another peculiarity of edk2. *shrug* I'm just glad -fno-common catches bugs like this! Reviewed-by: Laszlo Ersek (Obviously I'm not trying to dismiss your objection at all! Just stating my view. If the patch is changed to STATIC, I'll R-b that version *more happily* than this one.) Thanks! Laszlo > > >> Cc: Andrew Fish >> Cc: Jeff Fan >> Cc: Hao Wu >> Cc: Laszlo Ersek >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Michael D Kinney >> Reviewed-by: Jeff Fan >> --- >> .../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> index b717e33..9f5223a 100644 >> --- a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> +++ b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c >> @@ -32,7 +32,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mVectorHandoffInf >> } >> }; >> >> -GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_NOTIFY_DESCRIPTOR mMemoryDiscoveredNotifyList[1] = { >> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_NOTIFY_DESCRIPTOR mDebugAgentMemoryDiscoveredNotifyList[1] = { >> { >> (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), >> &gEfiPeiMemoryDiscoveredPpiGuid, >> @@ -554,7 +554,7 @@ InitializeDebugAgent ( >> // Register for a callback once memory has been initialized. >> // If memery has been ready, the callback funtion will be invoked immediately >> // >> - Status = PeiServicesNotifyPpi (&mMemoryDiscoveredNotifyList[0]); >> + Status = PeiServicesNotifyPpi (&mDebugAgentMemoryDiscoveredNotifyList[0]); >> if (EFI_ERROR (Status)) { >> DEBUG ((EFI_D_ERROR, "DebugAgent: Failed to register memory discovered callback function!\n")); >> CpuDeadLoop (); >> -- >> 2.6.3.windows.1 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> GitPatchExtractor 1.1 >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel