Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift

[Paolo Bonzini <pbonzini@redhat.com>]

On 01/10/2015 21:17, Laszlo Ersek wrote:
> - 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.

Then don't do it.  Use memcpy from uint64_t to the array.  Type punning
has other problems than aliasing---for example some architectures
require pointers to be correctly aligned when accessing objects bigger
than a byte.

> ... I don't know who on earth has brain capacity for tracking this.

If you can't understand a rule (or understanding it burns too much of
your brain cycles), just find a pattern that lets you respect it without
much thought. For strict aliasing it's just "don't cast pointer types"
with a single exception, namely casting a pointer to struct to a pointer
to the first member's type and the other way round.  Everything else can
either be expressed as container_of, or simply prohibited.

> 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!).

Integer conversions are already too much for me, in fact.

Here my pattern there is just: 1) use uint16_t as sparsely as possible
(because the result of a multiplication can overflow, unlike uint8_t);
2) never write unsigned int constants---this doesn't apply to unsigned
long long constants, which instead I use liberally; 3) rely heavily on
Coverity to detect narrow types being used as {,u}int64_t after
arithmetic has been done on int.

Never writing unsigned int constants conflicts heavily with this ubsan
rule.  And I can always use the excuse that I'm writing gnu89 code
rather than c99. :)

Paolo