[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift |
Date: |
Thu, 1 Oct 2015 21:17:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 10/01/15 19:38, Peter Maydell wrote:
> On 1 October 2015 at 18:30, Paolo Bonzini <address@hidden> 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
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, (continued)
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Richard Henderson, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Paolo Bonzini, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Peter Maydell, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Paolo Bonzini, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Laszlo Ersek, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Paolo Bonzini, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Laszlo Ersek, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Paolo Bonzini, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Peter Maydell, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift,
Laszlo Ersek <=
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Paolo Bonzini, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Laszlo Ersek, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Paolo Bonzini, 2015/10/08
- Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift, Kevin O'Connor, 2015/10/08
[Qemu-devel] [PATCH 2/2] target-i386: Don't left shift negative constant, Eduardo Habkost, 2015/10/08