On 21 February 2014 18:44, Andrew Fish afish@apple.com wrote:
On Feb 21, 2014, at 10:17 AM, Laszlo Ersek lersek@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.cl... _______________________________________________ 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.cl... _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel