On 06/16/14 17:54, Andrew Fish wrote: > > On Jun 16, 2014, at 5:11 AM, Laszlo Ersek <ler...@redhat.com > <mailto:ler...@redhat.com>> wrote: > >> On 06/16/14 08:14, Michael Casadevall wrote: >>> Hey all, >>> >>> I've been working on porting TianoCore to KVM on AArch64, and I've >>> recently run into an issue which I'm hoping someone can help me to >>> debug. Recent proposed changes to KVM place the address space for >>> BootROMs and persistent storage from 0x0-0x8000000, with the early boot >>> code for Tiano startng at 0x0. >> >> Looks like a spectacularly bad idea. The C standard guarantees that any >> pointer to a function, and any pointer to an object, will compare >> unequal to a null pointer. On edk2 platforms, null pointers have >> all-bits-zero representation. If you place an object at address zero, >> then the above guarantee is breached. >> > > Well PCs (and lots of other devices) have memory/ROM at address zero. In > this case it is an assembly branch instruction that lives at zero, so no > harm done to the C language.
I agree. > Dereferencing NULL is undefined behavior in C. While it is not > pedantically defined in C a volatile variable can be used to write to > address zero. There may be a real reason to do this, like to init the ECC. > > This is a good description of undefined behavior in C. > http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html My point was not about writing to the zero address. My point was about null pointers never aliasing an object or a function. 6.3.2.3 Pointers 3 An integer constant expression with the value 0, or such an expression cast to type void *, is called a /null pointer constant/. If a null pointer constant is converted to a pointer type, the resulting pointer, called a /null pointer/, is guaranteed to compare unequal to a pointer to any object or function. Consider the following C code (hypothetical, not sure if anything like this exists in the edk2 C code, but it could): { SOME_STRUCT *Ptr; Ptr = (VOID *)(UINTN)PcdGet32 (PcdFdBaseAddress); if (Ptr == NULL) { // // does not point to an object, bail // return EFI_WHATEVER; } SomeFunction (Ptr->Field1); return EFI_SOMETHING_ELSE; } Such code is right to think that (Ptr == NULL) implies that Ptr does not point to a SOME_STRUCT object, because the C standard guarantees that. Assuming that PcdGet32() expands to 0 (-->FixedAtBuild), the (VOID *)(UINTN)PcdGet32 (PcdFdBaseAddress) expression is an "integer constant expression" cast to void *. Hence the above citation directly applies; the expression is a null pointer constant, which is then (in the assignment to Ptr) converted to the pointer-to-SOME_STRUCT type. The resultant pointer is a null pointer. Let's see another example (assuming little endian, and 32-bit or 64-bit pointer size). { SOME_STRUCT *Ptr; UINT32 Value; ZeroMem (&Ptr, sizeof Ptr); Value = PcdGet32 (PcdFdBaseAddress); CopyMem (&Ptr, &Value, 4); if (Ptr == NULL) { // // does not point to an object, bail // return EFI_WHATEVER; } SomeFunction (Ptr->Field1); return EFI_SOMETHING_ELSE; } Even this (strongly platform-dependent) code (which lacks any pointer conversion) will break under the suggested use, because the suggested setting results in "Ptr" containing a bit pattern that corresponds to the internal representation of null pointers to structures. When the patch writes "set PcdFdBaseAddress to zero", it *intends* to say "place the image at the zero address". What it *actually* says, for C code, is "pointers converted -- or copied -- from PcdFdBaseAddress shall be NULL". Thanks, Laszlo ------------------------------------------------------------------------------ HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions Find What Matters Most in Your Big Data with HPCC Systems Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. Leverages Graph Analysis for Fast Processing & Easy Data Exploration http://p.sf.net/sfu/hpccsystems _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
