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_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 <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&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