On Feb 21, 2014, at 11:53 AM, Ryan Harkin <[email protected]> wrote:
>
>
>
> On 21 February 2014 18:44, Andrew Fish <[email protected]> wrote:
>
> On Feb 21, 2014, at 10:17 AM, Laszlo Ersek <[email protected]> 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.
>
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 <[email protected]>
~/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&iu=/4140/ostg.clktrk
>> _______________________________________________
>> edk2-devel mailing list
>> [email protected]
>> 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
> [email protected]
> 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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
