On 10/01/15 19:38, Peter Maydell wrote: > On 1 October 2015 at 18:30, Paolo Bonzini <pbonz...@redhat.com> wrote: >> >> >> On 01/10/2015 19:07, Laszlo Ersek wrote: >>>> In addition, C89 didn't say at all what the result was for signed data >>>> types, so technically we could compile QEMU with -std=gnu89 (the default >>>> until GCC5) and call it a day. >>>> >>>> Really the C standard should make this implementation-defined. >>> >>> Obligatory link: http://blog.regehr.org/archives/1180 >> >> Many ideas in there are good (e.g. mem*() being defined for invalid >> argument and zero lengths, and of course item 7 which is the issue at >> hand). In many cases it's also good to change undefined behavior to >> unspecified values, however I think that goes too far. >> >> For example I'm okay with signed integer overflow being undefined >> behavior, and I also disagree with "It is permissible to compute >> out-of-bounds pointer values including performing pointer arithmetic on >> the null pointer". Using uintptr_t is just fine. > > I bet you QEMU breaks the 'out of bounds pointer arithmetic' > rule all over the place. (set_prop_arraylen(), for a concrete > example off the top of my head.) > > Signed integer overflow being UB is a really terrible idea which > is one of the core cases for nailing down the UB -- everybody > expects signed integers to behave as 2s-complement, when in > fact what the compiler can and will do currently is just do totally > unpredictable things... > >> Also strict aliasing improves performance noticeably at least on some >> kind of code. The relaxation of strict aliasing that GCC does with >> unions would be a useful addition to the C standard, though. > > QEMU currently turns off strict-aliasing entirely, which I think > is entirely sensible of us.
Hm, I didn't know that. Indeed it is part of QEMU_CFLAGS. Another example: the kernel. In the top Makefile, KBUILD_CFLAGS gets -fno-strict-aliasing. And according to "Documentation/kbuild/makefiles.txt", "... the top Makefile owns the variable $(KBUILD_CFLAGS) and uses it for compilation flags for the entire tree". Yet another example: edk2. (See "BaseTools/Conf/tools_def.template", GCC_ALL_CC_FLAGS.) > A lot of the underlying intention behind the proposal (as I > interpret it) is "consistency and predictability of behaviour > for the programmer trumps pure performance". That sounds like > a good idea to me. I once spent an afternoon interpreting the "effective type" paragraphs in the C standard ("6.5 Expressions", paragraphs 6 and 7). They make sense, and it is possible to write conformant code. Here's an example: - In the firmware, allocate an array of bytes, dynamically. This array will have no declared type. - Populate the array byte-wise, from fw_cfg. Because the stores happen through character-typed lvalues, they do not "imbue" the target object with any effective type, for further accesses that do not modify the value. (I.e., for further reads.) - Get a (uint8_t*) into the array somewhere, and cast it to (struct acpi_table_hdr *). Read fields through the cast pointer. Assuming no out-of-bounds situation (considering the entire pointed to acpi_table_hdr struct), and assuming no alignment violations for the fields (which is implementation-defined), these accesses will be fine. *However*. If in point 2 you populate the array with uint64_t accesses, that *does* imbue the array elements with an effective type that is binding for further read accesses. And, in step 3, because the ACPI table header struct does not include uint64_t fields, those accesses will be undefined behavior. ... I don't know who on earth has brain capacity for tracking this. Effective type *does* propagate in a trackable manner, but it is one order of magnitude harder to follow for humans than integer conversions -- and resultant ranges -- are (and those are hard enough already!). So, it would be nice and prudent to comply with the effective type / strict aliasing rules, and allow the compiler to optimize "more", but personally I think I wouldn't be able to track effective type *realiably* (despite being fully conscious of integer promotions and conversions, for example). Therefore, I embrace -fno-strict-aliasing. Thanks Laszlo