On 21 February 2014 18:44, Andrew Fish <af...@apple.com> wrote:
>
> On Feb 21, 2014, at 10:17 AM, Laszlo Ersek <ler...@redhat.com> wrote:
>
> On 02/21/14 16:12, Ryan Harkin wrote:
>
> Hi Olivier,
>
> I've just noticed that the upstream EDK2 repository for the FVP AEMv8
> model is broken when built with Linaro GCC 13.12 onwards.
>
> The error I see is:
>
> UEFI firmware (version built at 14:54:24 on Feb 21 2014)
> add-symbol-file
>
> /linaro/uefi/master/upstream/edk2.git/Build/ArmVExpress-FVP-AArch64/DEBUG_ARMGCC/AARCH64/ArmPlatformPkg/PrePi/PeiMPCore/DEBUG/ArmPlatformPrePiMPCore.dll
> 0x88000780
> Decompress Failed - Not Found
>
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT
> /linaro/uefi/master/upstream/edk2.git/ArmPlatformPkg/PrePi/PrePi.c(194):
> !EFI_ERROR (Status)
>
> I've tracked the bug as far as function "FfsProcessSection" [1] where
> at line 373, it calls into function "ExtractGuidedSectionDecode" [2]
> which then calls into "SavedData->ExtractDecodeHandlerTable [Index]".
> At that point, I can't work out where it goes.
>
> I can "fix" the problem if I re-org the variables at the top of
> FfsProcessSection so that DstBuffer is at the start of the
> declarations. That is obviously not a fix. But it will probably hint
> at why the subsequent code is broken.
>
> Cheers,
> Ryan
>
> [1] EmbeddedPkg/Library/PrePiLib/FwVol.c, line 285
> [2]
> EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c:166
>
>
> Here's a random shot (could be completely unrelated):
>
>
> EFI_STATUS
> FfsProcessSection (
> IN EFI_SECTION_TYPE SectionType,
> IN EFI_COMMON_SECTION_HEADER *Section,
> IN UINTN SectionSize,
> OUT VOID **OutputBuffer
> )
> {
> /* ... */
> UINTN DstBufferSize;
> /* ... */
>
> You're on AArch64, so UINTN means UINT64.
>
> Note that this "auto" variable is not initialized, hence its contents are
> indeterminate. Fast forward to the next use:
>
> Status = UefiDecompressGetInfo (
> (UINT8 *) ((EFI_COMPRESSION_SECTION *) Section + 1),
> (UINT32) SectionLength - sizeof
> (EFI_COMPRESSION_SECTION),
> (UINT32 *) &DstBufferSize,
> &ScratchBufferSize
> );
> } else if (Section->Type == EFI_SECTION_GUID_DEFINED) {
> Status = ExtractGuidedSectionGetInfo (
> Section,
> (UINT32 *) &DstBufferSize,
> &ScratchBufferSize,
> &SectionAttribute
> );
>
> Whichever of these functions is invoked, it fills in only four bytes
> (UINT32). Then,
>
> DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (DstBufferSize) + 1);
>
> etc etc etc.
>
> By reordering the local variables, you could be limiting the nonzero
> garbage in "DstBufferSize" to those four bytes that *are* ultimately
> overwritten.
>
> I guess initing DstBufferSize to zero is easy enough to try... :)
>
>
As you guessed, it was easy to try and it does indeed fix the problem.
However....
>
>
> Why not just make it UINT32 and remove the casts. A real fix!
>
That looks like the correct solution to me. And yes, it works also.
Andrew, would you like to submit a patch as it's your change? I'm very
happy to do it, so whichever you'd prefer.
Thanks for your help, both!
Cheers,
Ryan.
>
> Thanks,
>
> Andrew Fish
>
> Thanks
> Laszlo
>
>
> ------------------------------------------------------------------------------
> Managing the Performance of Cloud-Based Applications
> Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
> Read the Whitepaper.
>
> http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
>
>
>
> ------------------------------------------------------------------------------
> Managing the Performance of Cloud-Based Applications
> Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
> Read the Whitepaper.
>
> http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
>
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
