On Wed, Jul 06, 2011, Olivier Martin wrote:
Anyway, these are the build instructions to build the Tianocore project (UEFI Open Source implementation) and to test on qEmu: http://edk2.svn.sourceforge.net/viewvc/edk2/trunk/edk2/BeagleBoardPkg/readme .txt?revision=11997
I had started following these, but ran into some issues with my setup; I will try with the alternate build-next.sh approach too, to see whether I get other issues or not.
The two classes of issues I ran into was: a) variables being written to, but never read b) conditions being always true
I indeed had to apply the patch you mention; what's the state of that patch? Are these things which will get merged soonish? Without the patch, my toolchain wouldn't be properly picked up (it'd try running c:/program files/ stuff and calling a CS toolchain) and I had some error conditions like '-(' in linker invocations due to missing '' escapes. These go away with the patch and my toolchain is picked up and works.
I'm running Ubuntu oneiric (which will become 11.10) and the toolchain I've used is our packaged Linaro GCC based cross-compiler (arm-linux-gnueabi-gcc from the gcc-arm-linux-gnueabi package).
a) variables being written to, but never read
This is stuff like:
--- a/edk2/BaseTools/Source/C/EfiLdrImage/EfiLdrImage.c +++ b/edk2/BaseTools/Source/C/EfiLdrImage/EfiLdrImage.c @@ -181,7 +181,7 @@ Returns: CHAR8* OutputFileName = NULL; CHAR8* InputFileNames[MAX_PE_IMAGES + 1]; UINT8 InputFileCount = 0; - BOOLEAN QuietFlag = FALSE; + //BOOLEAN QuietFlag = FALSE; UINT64 DebugLevel = 0; UINT64 VerboseLevel = 0; EFI_STATUS Status = EFI_SUCCESS; @@ -220,7 +220,7 @@ Returns: }
if ((stricmp (argv[0], "-q") == 0) || (stricmp (argv[0], "--quiet") == 0)) { - QuietFlag = TRUE; + //QuietFlag = TRUE; argc --; argv ++; continue;
QuietFlag is never used (read), newer gccs detect this and report it as a fatal warning by default (not sure where this warning is configured as a fatal one, whether it's in UEFI or in the toolchain config).
There are many such cases, but some are a bit more worrying, like variables holding the return values of fread of fwrite not being checked: --- a/edk2/BaseTools/Source/C/GenVtf/GenVtf.c +++ b/edk2/BaseTools/Source/C/GenVtf/GenVtf.c @@ -1141,7 +1141,7 @@ Returns: EFI_STATUS Status; UINT64 CompStartAddress; UINT64 FileSize; - UINT64 NumByteRead; + //UINT64 NumByteRead; UINT64 NumAdjustByte; UINT8 *Buffer; FILE *Fp; @@ -1189,7 +1189,7 @@ Returns: // // Read first 64 bytes of PAL header and use it to find version info // - NumByteRead = fread (Buffer, sizeof (UINT8), SIZE_OF_PAL_HEADER, Fp); + /* NumByteRead = */fread (Buffer, sizeof (UINT8), SIZE_OF_PAL_HEADER, Fp);
// // PAL header contains the version info. Currently, we will use the header @@ -1200,7 +1200,7 @@ Returns: } }
- NumByteRead = fread (Buffer, sizeof (UINT8), (UINTN) FileSize, Fp); + /* NumByteRead = */fread (Buffer, sizeof (UINT8), (UINTN) FileSize, Fp); fclose (Fp);
//
I just quickly hacked the source to get it to build, but we should fix the code to loop until all bytes have been read or to raise an error.
The last such cases is a conditional piece of code: --- a/edk2/BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c +++ b/edk2/BaseTools/Source/C/LzmaCompress/Sdk/C/LzmaEnc.c @@ -1919,11 +1919,13 @@ static SRes LzmaEnc_CodeOneBlock(CLzmaEnc *p, Bool useLimits, UInt32 maxPackSize static SRes LzmaEnc_Alloc(CLzmaEnc *p, UInt32 keepWindowSize, ISzAlloc *alloc, ISzAlloc *allocBig) { UInt32 beforeSize = kNumOpts; + #ifdef COMPRESS_MF_MT Bool btMode; + #endif if (!RangeEnc_Alloc(&p->rc, alloc)) return SZ_ERROR_MEM; - btMode = (p->matchFinderBase.btMode != 0); #ifdef COMPRESS_MF_MT + btMode = (p->matchFinderBase.btMode != 0); p->mtMode = (p->multiThread && !p->fastMode && btMode); #endif
b) conditions being always true
A bunch of asserts trigger a warning in newer GCCs, essentially the ASSERT macros in question are doing multiple things: - testing whether the object is null - testing some other condition on the object, like some state being correct but the objects are guaranteed to never be null from GCC PoV, so it raises a fatal warning; as a workaround I just commented the asserts out, but I guess what we should do is review whether any of the ASSERT_LOCKED() users has a chance of getting a NULL pointer, and either drop the NULL check or replace the macro with two.
--- a/edk2/MdeModulePkg/Core/Dxe/Event/Event.c +++ b/edk2/MdeModulePkg/Core/Dxe/Event/Event.c @@ -225,7 +225,7 @@ CoreNotifyEvent ( // // Event database must be locked // - ASSERT_LOCKED (&gEventQueueLock); + //ASSERT_LOCKED (&gEventQueueLock);
// // If the event is queued somewhere, remove it
I got this for a bunch of ASSERT_LOCKED(), ASSERT_PROTOCOL_ALREADY_INSTALLED(), and ASSERT_VOLUME_LOCKED()
These are all issues in edk2, except the ASSERT_VOLUME_LOCKED() macro issue which is the only issue in FatPkg and only there.
A cleaner workaround would be to make this warnings non-fatal for now for this toolchain, but the code should also be fixed not to trigger them. What's the best way to address this? Is there some bug tracker I should report these issues at?
To try the resulting image, I just did: qemu-system-arm -mtdblock \ ../Build/BeagleBoard/DEBUG_ARMGCC/FV/BeagleBoard_EFI_flashboot.fd \ -nographic -M beagle
which sent this on the serial port: UART Enabled Magic delay to disable watchdog timers properly.
ASSERT_EFI_ERROR (Status = Unsupported) ASSERT /home/lool/git/edk2/edk2/edk2/EmbeddedPkg/Library/PrePiLib/PrePiLib.c(73): !EFI_ERROR (Status)
I didn't actually look into this issue yet.
I tried passing a barebox based SD image I had built earlier (see other message) with -sd sd.img, but that didn't change the output.