Thanks for your contribution, I submitted Andrew's fix in revision 15252.
From: Andrew Fish [mailto:afish@apple.com] Sent: 22 February 2014 18:47 To: edk2-devel@lists.sourceforge.net Cc: linaro-uefi Subject: Re: [edk2] Upstream EDK2 broken with latest Linaro toolchain (13.12 onwards)
On Feb 21, 2014, at 11:53 AM, Ryan Harkin ryan.harkin@linaro.org wrote:
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_AR MGCC/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/PrePiExtractGuidedSectionLi b.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.
I got an error message trying to commit. Could you commit this change.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Andrew Fish afish@apple.com
~/work/edk2>svn diff EmbeddedPkg/Library/PrePiLib/FwVol.c
Index: EmbeddedPkg/Library/PrePiLib/FwVol.c
===================================================================
--- EmbeddedPkg/Library/PrePiLib/FwVol.c (revision 15251)
+++ EmbeddedPkg/Library/PrePiLib/FwVol.c (working copy)
@@ -293,7 +293,7 @@
UINT32 SectionLength;
UINT32 ParsedLength;
EFI_COMPRESSION_SECTION *CompressionSection;
- UINTN DstBufferSize;
+ UINT32 DstBufferSize;
VOID *ScratchBuffer;
UINT32 ScratchBufferSize;
VOID *DstBuffer;
@@ -322,13 +322,13 @@
Status = UefiDecompressGetInfo (
(UINT8 *) ((EFI_COMPRESSION_SECTION *) Section + 1),
(UINT32) SectionLength - sizeof (EFI_COMPRESSION_SECTION),
- (UINT32 *) &DstBufferSize,
+ &DstBufferSize,
&ScratchBufferSize
);
} else if (Section->Type == EFI_SECTION_GUID_DEFINED) {
Status = ExtractGuidedSectionGetInfo (
Section,
- (UINT32 *) &DstBufferSize,
+ &DstBufferSize,
&ScratchBufferSize,
&SectionAttribute
);
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 http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktr k &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 http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktr k &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 http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktr k_______________________________________________ &iu=/4140/ostg.clktrk_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel