Re: [Patch V4 09/10] EmulatorPkg/Sec: Change local variable scope for XCODE5

["Jordan Justen" <jordan.l.justen@intel.com>]

On 2019-08-15 19:14:36, Michael D Kinney wrote:
> The local variable PpiArray[10] is declared in middle
> of the SEC module _ModuleEntryPoint().  When building
> for XCODE5 with optimizations enabled, this results in
> corruption and an exception.

It looks like with old code, the scope containing PpiArray was closed,
but a dangling reference to had been made to it's location on the
stack. So, I think the change is good but we should add this extra
detail to the commit message.

-Jordan

> The fix is to move the
> declaration of PpiArray[10] to the standard location
> at the beginning of the function so the storage for
> this local variable is allocated for the entire
> lifetime of the function.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Andrew Fish <afish@apple.com>
> ---
>  EmulatorPkg/Sec/Sec.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/EmulatorPkg/Sec/Sec.c b/EmulatorPkg/Sec/Sec.c
> index 701032233b..b734d2bb87 100644
> --- a/EmulatorPkg/Sec/Sec.c
> +++ b/EmulatorPkg/Sec/Sec.c
> @@ -75,6 +75,7 @@ _ModuleEntryPoint (
>    EFI_PEI_PPI_DESCRIPTOR    *SecPpiList;
>    UINTN                     SecReseveredMemorySize;
>    UINTN                     Index;
> +  EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];
>  
>    EMU_MAGIC_PAGE()->PpiList = PpiList;
>    ProcessLibraryConstructorList ();
> @@ -104,16 +105,13 @@ _ModuleEntryPoint (
>    SecCoreData->PeiTemporaryRamBase = (VOID *)((UINTN)SecCoreData->PeiTemporaryRamBase + SecReseveredMemorySize);
>    SecCoreData->PeiTemporaryRamSize -= SecReseveredMemorySize;
>  #else
> -  {
> -    //
> -    // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
> -    // or I don't understand temp RAM correctly?
> -    //
> -    EFI_PEI_PPI_DESCRIPTOR    PpiArray[10];
> +  //
> +  // When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? Either there is a bug
> +  // or I don't understand temp RAM correctly?
> +  //
>  
> -    SecPpiList = &PpiArray[0];
> -    ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
> -  }
> +  SecPpiList = &PpiArray[0];
> +  ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
>  #endif
>    // Copy existing list, and append our entries.
>    CopyMem (SecPpiList, PpiList, sizeof (EFI_PEI_PPI_DESCRIPTOR) * Index);
> -- 
> 2.21.0.windows.1
>