[edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol

Liming Gao posted 1 patch 6 years, 4 months ago
.../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c         | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Liming Gao 6 years, 4 months ago
From: Michael Kinney <michael.d.kinney@intel.com>

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.

Cc: Andrew Fish <afish@apple.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>
---
 .../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
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Ni, Ruiyu 6 years, 4 months ago
On 12/7/2017 3:48 PM, Liming Gao wrote:
> From: Michael Kinney <michael.d.kinney@intel.com>
> 
> 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.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
> ---
>   .../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 ();
> 

It's a good code practice to use library name as prefix for library
global variables.

-- 
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Ni, Ruiyu 6 years, 4 months ago
On 12/7/2017 3:48 PM, Liming Gao wrote:
> From: Michael Kinney <michael.d.kinney@intel.com>
> 
> 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.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
> ---
>   .../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 ();
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Ard Biesheuvel 6 years, 4 months ago
On 7 December 2017 at 07:48, Liming Gao <liming.gao@intel.com> wrote:
> From: Michael Kinney <michael.d.kinney@intel.com>
>
> 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?


> Cc: Andrew Fish <afish@apple.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
> ---
>  .../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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Kinney, Michael D 6 years, 4 months ago
Ard,

If the variable was not prepended with GLOBAL_REMOVE_IF_UNREFERENCED, I would agree.

GLOBAL_REMOVE_IF_UNREFERENCED for some compilers is not compatible with static.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> On Behalf Of Ard Biesheuvel
> Sent: Thursday, December 7, 2017 12:47 AM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; edk2-
> devel@lists.01.org; Andrew Fish <afish@apple.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Jeff Fan <jeff.fan@intel.com>
> Subject: Re: [edk2] [Patch]
> SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate
> symbol
> 
> On 7 December 2017 at 07:48, Liming Gao
> <liming.gao@intel.com> wrote:
> > From: Michael Kinney <michael.d.kinney@intel.com>
> >
> > 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?
> 
> 
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Jeff Fan <jeff.fan@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > Reviewed-by: Jeff Fan <jeff.fan@intel.com>
> > ---
> >
> .../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentL
> ib.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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Laszlo Ersek 6 years, 4 months ago
On 12/07/17 09:46, Ard Biesheuvel wrote:
> On 7 December 2017 at 07:48, Liming Gao <liming.gao@intel.com> wrote:
>> From: Michael Kinney <michael.d.kinney@intel.com>
>>
>> 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 <lersek@redhat.com>

(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 <afish@apple.com>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
>> ---
>>  .../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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Ard Biesheuvel 6 years, 4 months ago
On 7 December 2017 at 11:18, Laszlo Ersek <lersek@redhat.com> wrote:
> On 12/07/17 09:46, Ard Biesheuvel wrote:
>> On 7 December 2017 at 07:48, Liming Gao <liming.gao@intel.com> wrote:
>>> From: Michael Kinney <michael.d.kinney@intel.com>
>>>
>>> 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.)
>

That doesn't quite ring a bell, but if that is the case, it deserves a mention.

Note that STATIC variables are also removed when unreferenced (but may
require a warning override like we have for GCC if it is only used
from DEBUG () code). In any case, polluting the global namespace in a
heterogeneous project like EDK2 is something that should only be done
with good reason IMO.

> 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!
>

Well, the thing is, external linkage defeats optimizations in the
compiler, and also prevents it from emitting the variable into a
read-only section even if it would otherwise be able to infer from the
usage that the variable is never modified.

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> (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 <afish@apple.com>
>>> Cc: Jeff Fan <jeff.fan@intel.com>
>>> Cc: Hao Wu <hao.a.wu@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>>> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
>>> ---
>>>  .../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
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Gao, Liming 6 years, 4 months ago
Ard and Ersek:
  On VS, static may make debug become hard. And, STATIC + GLOBAL_REMOVE_IF_UNREFERENCED may failure on old VS before VS supports /Gw option. So, I don't add static here. 
  I would like to separate static usage topic. We could discuss more and summary the rule on how to use static in code. But for this build failure issue, I prefer to fix it with this solution first. Is it OK to you?

Thanks
Liming
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, December 7, 2017 7:41 PM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Andrew Fish <afish@apple.com>; Jeff Fan <jeff.fan@intel.com>
> Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
> 
> On 7 December 2017 at 11:18, Laszlo Ersek <lersek@redhat.com> wrote:
> > On 12/07/17 09:46, Ard Biesheuvel wrote:
> >> On 7 December 2017 at 07:48, Liming Gao <liming.gao@intel.com> wrote:
> >>> From: Michael Kinney <michael.d.kinney@intel.com>
> >>>
> >>> 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.)
> >
> 
> That doesn't quite ring a bell, but if that is the case, it deserves a mention.
> 
> Note that STATIC variables are also removed when unreferenced (but may
> require a warning override like we have for GCC if it is only used
> from DEBUG () code). In any case, polluting the global namespace in a
> heterogeneous project like EDK2 is something that should only be done
> with good reason IMO.
> 
> > 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!
> >
> 
> Well, the thing is, external linkage defeats optimizations in the
> compiler, and also prevents it from emitting the variable into a
> read-only section even if it would otherwise be able to infer from the
> usage that the variable is never modified.
> 
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >
> > (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 <afish@apple.com>
> >>> Cc: Jeff Fan <jeff.fan@intel.com>
> >>> Cc: Hao Wu <hao.a.wu@intel.com>
> >>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> >>> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
> >>> ---
> >>>  .../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
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Ard Biesheuvel 6 years, 4 months ago
On 7 December 2017 at 14:19, Gao, Liming <liming.gao@intel.com> wrote:
> Ard and Ersek:
>   On VS, static may make debug become hard. And, STATIC + GLOBAL_REMOVE_IF_UNREFERENCED may failure on old VS before VS supports /Gw option. So, I don't add static here.
>   I would like to separate static usage topic. We could discuss more and summary the rule on how to use static in code. But for this build failure issue, I prefer to fix it with this solution first. Is it OK to you?
>

Yes that is fine.

-- 
Ard.

>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Thursday, December 7, 2017 7:41 PM
>> To: Laszlo Ersek <lersek@redhat.com>
>> Cc: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; Wu, Hao A
>> <hao.a.wu@intel.com>; Andrew Fish <afish@apple.com>; Jeff Fan <jeff.fan@intel.com>
>> Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
>>
>> On 7 December 2017 at 11:18, Laszlo Ersek <lersek@redhat.com> wrote:
>> > On 12/07/17 09:46, Ard Biesheuvel wrote:
>> >> On 7 December 2017 at 07:48, Liming Gao <liming.gao@intel.com> wrote:
>> >>> From: Michael Kinney <michael.d.kinney@intel.com>
>> >>>
>> >>> 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.)
>> >
>>
>> That doesn't quite ring a bell, but if that is the case, it deserves a mention.
>>
>> Note that STATIC variables are also removed when unreferenced (but may
>> require a warning override like we have for GCC if it is only used
>> from DEBUG () code). In any case, polluting the global namespace in a
>> heterogeneous project like EDK2 is something that should only be done
>> with good reason IMO.
>>
>> > 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!
>> >
>>
>> Well, the thing is, external linkage defeats optimizations in the
>> compiler, and also prevents it from emitting the variable into a
>> read-only section even if it would otherwise be able to infer from the
>> usage that the variable is never modified.
>>
>> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> >
>> > (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 <afish@apple.com>
>> >>> Cc: Jeff Fan <jeff.fan@intel.com>
>> >>> Cc: Hao Wu <hao.a.wu@intel.com>
>> >>> Cc: Laszlo Ersek <lersek@redhat.com>
>> >>> Contributed-under: TianoCore Contribution Agreement 1.0
>> >>> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>> >>> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
>> >>> ---
>> >>>  .../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
>> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Posted by Wu, Hao A 6 years, 4 months ago
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Liming Gao
> Sent: Thursday, December 07, 2017 3:48 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D; Wu, Hao A; Laszlo Ersek; Andrew Fish; Jeff Fan
> Subject: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix
> duplicate symbol
> 
> From: Michael Kinney <michael.d.kinney@intel.com>
> 
> 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.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
> ---
>  .../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c         | 4
> ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebug
> AgentLib.c
> b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebug
> AgentLib.c
> index b717e33..9f5223a 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebug
> AgentLib.c
> +++
> b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebug
> AgentLib.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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel