Of all the gin joints in all the towns in all the world, Laszlo Ersek had to
walk into mine at 15:55:00 on Monday 03 February 2014 and say:
> On 02/04/14 00:05, Bill Paul wrote:
> > Of all the gin joints in all the towns in all the world, Laszlo Ersek had
> > to
> >
> > walk into mine at 14:36:03 on Monday 03 February 2014 and say:
> >> On 02/03/14 23:02, Bill Paul wrote:
> >>> For the most part this works fine, but I noticed that when I build
> >>> OVMF, I see the following warnings:
> >>>
> >>> edk2/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c: In
> >>> function 'FvbGetPhysicalAddress':
> >>> edk2/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c:249:
> >>> error: 'FwhInstance' may be used uninitialized in this function
> >>>
> >>> This appears to be due to lines like this:
> >>> EFI_FW_VOL_INSTANCE *FwhInstance;
> >>>
> >>> which should be this:
> >>> EFI_FW_VOL_INSTANCE *FwhInstance = NULL;
> >>>
> >>> This is easily corrected, but I'm wondering why nobody else seems to
> >>> have noticed it. Maybe newer versions of GCC ignore it?
> >>
> >> At least in FvbGetPhysicalAddress(), there's an
> >> ASSERT_EFI_ERROR(Status). Gcc's warning is incorrect (at least in DEBUG
> >> builds). I guess newer gcc versions might deduce that.
> >
> > I'm not doing a DEBUG build: I'm doing a RELEASE build. That probably
> > makes the ASSERTs no-ops, in which case the warning is valid.
>
> gcc's man page says under -Wuninitialized,
>
> [...] Because these warnings depend on optimization, the exact
> variables or elements for which there are warnings depends on the
> precise optimization options and version of GCC used.
I don't think it's a matter of compiler options. Looking at the code, I don't
think it's implemented very safely. For example:
EFI_STATUS
FvbGetPhysicalAddress (
IN UINTN Instance,
OUT EFI_PHYSICAL_ADDRESS *Address,
IN ESAL_FWB_GLOBAL *Global,
IN BOOLEAN Virtual
)
{
EFI_FW_VOL_INSTANCE *FwhInstance = NULL;
EFI_STATUS Status;
//
// Find the right instance of the FVB private data
//
Status = GetFvbInstance (Instance, Global, &FwhInstance, Virtual);
ASSERT_EFI_ERROR (Status);
*Address = FwhInstance->FvBase[Virtual];
return EFI_SUCCESS;
}
Here, you call GetFvbInstance() to populate FwhInstance. What happens if it
fails? In the DEBUG case, the assert triggers, and maybe you end up with a
hard abort, but what about the RELEASE case? If GetFvbInstance() fails, you
blow up, regardless of optimization, because FwhInstance is never set
properly. (If you didn't initialize it to NULL, it would contain stack
garbage.)
Wouldn't it be more sensible to do?
EFI_STATUS
FvbGetPhysicalAddress (
IN UINTN Instance,
OUT EFI_PHYSICAL_ADDRESS *Address,
IN ESAL_FWB_GLOBAL *Global,
IN BOOLEAN Virtual
)
{
EFI_FW_VOL_INSTANCE *FwhInstance = NULL;
EFI_STATUS Status;
//
// Find the right instance of the FVB private data
//
Status = GetFvbInstance (Instance, Global, &FwhInstance, Virtual);
ASSERT_EFI_ERROR (Status);
if (EFI_ERROR(Status))
return (Status);
*Address = FwhInstance->FvBase[Virtual];
return EFI_SUCCESS;
}
That way you test that the call succeeded before trying to dereference
FwhInstance.
>
> >>> edk2/StdLib/BsdSocketLib/ns_print.c: In function '__ns_sprintrrf':
> >>> edk2/StdLib/BsdSocketLib/ns_print.c:231: error: format '%Lu' expects
> >>> type 'long long unsigned int', but argument 3 has type 'u_long'
> >>> edk2/StdLib/BsdSocketLib/ns_print.c:519: error: format '%Lu' expects
> >>> type 'long long unsigned int', but argument 5 has type 'u_long'
> >>>
> >>> Apparently it wants the %Lu to be just %u
> >>
> >> Seems like a genuine bug (when built for Ia32).
> >
> > It triggers for me with X64 builds too. I'm pretty sure unsigned long is
> > always 32 bits.
>
> I thought that the X64 build used the LP64 model, where sizeof(long) ==
> sizeof(long long) == sizeof(void*) == sizeof(UINTN) == sizeof(UINT64). I
> could be wrong, in edk2 I only use the INTXX and UINTXX types.
>
> (Under the above LP64 assumption, I meant that the bug was a bug "in
> practice" only on Ia32. Gcc catches the format string discrepancy in any
> case though, which is a good thing.)
>
> If edk2 uses LLP64 (from Windows tradition), then I'm wrong of course.
Hm. Well, bear in mind that with the UNIXGCC case, the mingw-gcc-build.py
script is building a GCC targeted for MinGW, which is a native Windows
compiler. So it may actually be honoring the LLP64 case.
I just did a quick test:
- Building with mingw-gcc in EDKII: sizeof(long) == 4 bytes
- Building same code with GNU EFI (ELF gcc): sizeof long == 8 bytes
So I guess that's it.
> >>> It's possible that all of this may be attributable to the cross
> >>> compiler toolchain being out of date, but these seem like genuine bugs
> >>> to me. Again, I'm surprised they haven't been noticed before.
> >>
> >> You could be the first user ever to try to build these on FreeBSD, with
> >> the UNIXGCC toolset.
> >
> > Again, it doesn't matter that it's on FreeBSD. The cross compiler would
> > behave the same if I built it on Solaris instead.
>
> Possibly, but there you might be seeing the same problem, as Solaris is
> likely not among the host platforms (same as FreeBSD) where the
> cross-compiler-generator script is tested.
>
> I did understand your point about how it shouldn't matter. I'm just
> saying that the docs imply it wasn't tested there.
And I'm saying it just needs to be tested, period. :)
> >> Can you try with GCC44+ instead?
> >>
> >> (At least OvmfPkg/README has such a recommendation (from SVN r13569),
> >> albeit for completely different grounds.) The ports tree seems to have
> >> GCC46 even.
> >
> > Uhm... I can't just "try with GCC44+ instead," can I?
> >
> > I need the cross compiler environment to build EFI binaries, and the
> > mingw- gcc-build.py script is set up for gcc 4.3.0.
>
> I don't understand why your need for an environment that produces EFI
> binaries on x86_64 FreeBSD is so different from my need for an
> environment that produces EFI binaries on x86_64 Linux.
Okay, look.
I have FreeBSD/amd64 9.1-RELEASE? Right? It comes with a native gcc compiler.
(It also comes with clang, but that's another issue.)
However:
a) It's gcc 4.2.1
b) It only produces 64-bit code
c) I don't think the GNU binutils that it uses was configured to handle all of
the special sub classes of PEI files (e.g. UEFI drivers vs. UEFI applications
-- you have to set the type in the PE image header and I think out of the box
it only handles applications)
So, if I want to build an IA32 application, I'm stuck right away: the native
compiler doesn't build 32-bit code. Even if I had GCC 4.4.x and even if I
fixed up the binutils correctly, I still would only be able to build X64
binaries. And it happens I want to build both IA32 and X64 images for my
VxWorks OS loader application. Since I knew that from the outset, that's why I
picked the UNIXGCC case.
The reason the FreeBSD native compiler only produces 64-bit code is that
historically the host compiler only supports the host machine type. I can
build a 32-bit FreeBSD world from a 64-bit FreeBSD host system, but along the
way it builds a 32-bit cross compiler.
Similarly, FreeBSD/i386 comes with a gcc that only builds 32-bit binaries.
> If gcc-4.4 works
> for me out of the box, what theoretical proof exists that says it can
> never If so, then I want you to understand this very clearly:
Well.. see above.
> <http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=Getting_St
> arted_with_EDK_II>
>
> A few different build environment types are documented. If
> instructions are not available for your exact system configuration,
> you may still be able to tweak the instructions to work on your
> system.
>
> * Using EDK II with Native GCC 4.4 (Recommended when using newer
> versions of Linux)
> * Unix-like systems (For older Linux distributions, or when using
> Cygwin or Mac OS X)
> [...]
>
> It doesn't say that you *can't* work with gcc-4.4 on other operating
> systems. It simply recommends UNIXGCC for systems that are unfortunate
> enough to lack an out-of-the-box GCC44 installation. FreeBSD is not such
> a system I think.
Just to be clear, it says up there "Native GCC 4.4." So, when you say to use
the GCC44+ build, you're actually talking about using the native/host GCC
tools that come with Linux. Right? Like, /usr/bin/gcc, right?
Then you need to realize that this case will only ever work for Linux users.
You should not be surprised that it doesn't work for FreeBSD, or NetBSD, or
anything else. And before you say to me "well, can't you just upgrade your
version of gcc?" let me explain that with FreeBSD, there is a base OS unit and
the C compiler is part of it. I am not going to replace it. I could install
another GCC in a different location, but if I'm going to do that I may as well
use the cross compiler method anyway.
In my opinion it's not a good idea to rely on the host compiler for what is
really a cross-build target. I mean, you can support that if you want, but if
that's _all_ you support, you're implicitly only supporting Linux too. And
that's not fair, since the intent seems to have been to allow people to use
other UNIX or UNIX-like OSes too.
> I don't use a cross compiler, just the quirky flags that come from
> "BaseTools/Conf/tools_def.template":
>
> gcc -g -fshort-wchar -fno-stack-protector -fno-strict-aliasing -Wall
> -Werror -Wno-missing-braces -Wno-array-bounds -ffunction-sections
> -fdata-sections -c -include AutoGen.h
> -DSTRING_ARRAY_NAME=LoadLinuxLibStrings -m64
> "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS -mno-red-zone
> -Wno-address -mcmodel=large -Wno-address -Wno-unused-but-set-variable
> -mno-mmx -mno-sse ...
>
> The generated code is not for a different architecture, it's Ia32 / X64
> code just wrapped in different headers than usual. (You didn't say that
> your build host was a different architecture, you just said "I'm
> building on FreeBSD".)
The fact that my host compiler happens to generate code for one of the desired
UEFI target CPU architectures should be be construed to mean that it's
implicitly suitable for target builds. It *might* be, but only if a) I'm very
careful or b) I'm very lucky.
> > One assumes that it picks the
>
> > gcc/binutils versions that it does because these are known to work:
> (known to work on the tested platforms, which FreeBSD is not)
>
> > I'm not
> > going to spend hours building a bunch of different gccs/binutils trying
> > to find another one by trial and error.
>
> Sure. You could try installing gcc-4.6 and passing -t GCC46 though,
> because your OS makes that convenient.
>
> I regularly build X64 OvmfPkg (with native gcc-4.8), ArmVExpressPkg
> platforms (DSCs) for Aarch64 (with the ARMLINUXGCC /
> aarch64-linux-gnu-gcc cross compiler), and other ArmVExpressPkg DSCs for
> 32-bit ARM (with the ARMGCC / arm-linux-gnu-gcc cross compiler). I think
> I do understand the cross compiler concept on the user level.
Well... look. UEFI is really a cross-built target. I know that in lots of
Linux distros they've been shipping GCC with the ability to build PE32/PE32+
and that's nice. Even FreeBSD does that. And supporting that as a build target
is fine. But you should also have some robust support for cross-build tool
chains so that people who aren't on Linux can play too. Now, clearly someone
put some thought into doing that since the cross-compiler configuration
exists. What would be nice is if:
a) mingw-gcc-build.py could build ARM and Itanium cross-compilers too
(then you would have a complete development kit that could be used
anywhere for any target)
b) somebody would test it once in a while
-Bill
> Apologies that I couldn't help.
>
> Laszlo
--
=============================================================================
-Bill Paul (510) 749-2329 | Senior Member of Technical Staff,
[email protected] | Master of Unix-Fu - Wind River Systems
=============================================================================
"I put a dollar in a change machine. Nothing changed." - George Carlin
=============================================================================
------------------------------------------------------------------------------
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=121051231&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
