[Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Jeff Cody]

On 32-bit hosts, some compilers will warn on too large integer constants
for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
on those defines.

Reported-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/vhdx.h b/block/vhdx.h
index 2acd7c2..8103d4c 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -61,7 +61,7 @@
 /* These structures are ones that are defined in the VHDX specification
  * document */
 
-#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
+#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII */
 typedef struct VHDXFileIdentifier {
     uint64_t    signature;              /* "vhdxfile" in ASCII */
     uint16_t    creator[256];           /* optional; utf-16 string to identify
@@ -238,7 +238,7 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
 /* upper 44 bits are the file offset in 1MB units lower 3 bits are the state
    other bits are reserved */
 #define VHDX_BAT_STATE_BIT_MASK 0x07
-#define VHDX_BAT_FILE_OFF_MASK  0xFFFFFFFFFFF00000 /* upper 44 bits */
+#define VHDX_BAT_FILE_OFF_MASK  0xFFFFFFFFFFF00000ULL /* upper 44 bits */
 typedef uint64_t VHDXBatEntry;
 
 /* ---- METADATA REGION STRUCTURES ---- */
@@ -247,7 +247,7 @@ typedef uint64_t VHDXBatEntry;
 #define VHDX_METADATA_MAX_ENTRIES 2047  /* not including the header */
 #define VHDX_METADATA_TABLE_MAX_SIZE \
     (VHDX_METADATA_ENTRY_SIZE * (VHDX_METADATA_MAX_ENTRIES+1))
-#define VHDX_METADATA_SIGNATURE 0x617461646174656D  /* "metadata" in ASCII */
+#define VHDX_METADATA_SIGNATURE 0x617461646174656DULL  /* "metadata" in ASCII */
 typedef struct QEMU_PACKED VHDXMetadataTableHeader {
     uint64_t    signature;              /* "metadata" in ASCII */
     uint16_t    reserved;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Stefan Hajnoczi]

On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
> On 32-bit hosts, some compilers will warn on too large integer constants
> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
> on those defines.
> 
> Reported-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vhdx.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Richard W.M. Jones]

On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
> On 32-bit hosts, some compilers will warn on too large integer constants
> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
> on those defines.
> -#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
> +#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII */

I think it's better to use this C99-defined feature (from <stdint.h>):

#define VHDX_FILE_SIGNATURE UINT64_C(0x656C696678646876)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Peter Maydell]

On 14 March 2014 15:36, Richard W.M. Jones <rjones@redhat.com> wrote:
> On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
>> On 32-bit hosts, some compilers will warn on too large integer constants
>> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
>> on those defines.
>> -#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
>> +#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII */
>
> I think it's better to use this C99-defined feature (from <stdint.h>):
>
> #define VHDX_FILE_SIGNATURE UINT64_C(0x656C696678646876)

Why? It's longer and we barely use it anywhere else
in the codebase, whereas we use the ULL suffix all
over the place...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Richard W.M. Jones]

On Fri, Mar 14, 2014 at 03:38:55PM +0000, Peter Maydell wrote:
> On 14 March 2014 15:36, Richard W.M. Jones <rjones@redhat.com> wrote:
> > On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
> >> On 32-bit hosts, some compilers will warn on too large integer constants
> >> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
> >> on those defines.
> >> -#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
> >> +#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII */
> >
> > I think it's better to use this C99-defined feature (from <stdint.h>):
> >
> > #define VHDX_FILE_SIGNATURE UINT64_C(0x656C696678646876)
> 
> Why? It's longer and we barely use it anywhere else
> in the codebase, whereas we use the ULL suffix all
> over the place...

It's permitted for unsigned long long to be longer than 64 bits.  The
UINT64_C() macro will always return a 64 bit int constant.  Yup, it's
not likely and for this particular macro it wouldn't matter.  (And I'm
not going to get into language-lawyering ...)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 16:57, Richard W.M. Jones wrote:
> On Fri, Mar 14, 2014 at 03:38:55PM +0000, Peter Maydell wrote:
>> On 14 March 2014 15:36, Richard W.M. Jones <rjones@redhat.com> wrote:
>>> On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
>>>> On 32-bit hosts, some compilers will warn on too large integer constants
>>>> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
>>>> on those defines.
>>>> -#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
>>>> +#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII */
>>>
>>> I think it's better to use this C99-defined feature (from <stdint.h>):
>>>
>>> #define VHDX_FILE_SIGNATURE UINT64_C(0x656C696678646876)
>>
>> Why? It's longer and we barely use it anywhere else
>> in the codebase, whereas we use the ULL suffix all
>> over the place...
> 
> It's permitted for unsigned long long to be longer than 64 bits.

Yes.

> The
> UINT64_C() macro will always return a 64 bit int constant.

That's incorrect, for two reasons:
(a) uint64_t (exact-width integer types in general) are optional in C99.
See 7.18.1.1 "Exact-width integer types" p3:

    These types are optional. However, if an implementation provides
    integer types with widths of 8, 16, 32, or 64 bits, it shall define
    the corresponding typedef names.

I general we can't state that the "UINT64_C() macro will always return a
64 bit int constant", because the C implementation might not even
support such a type.

(b) UINT64_C() is for "uint_least64_t" (7.18.4.1 Macros for
minimum-width integer constants). "uint_least64_t" is a required type
(7.18.1.2 Minimum-width integer types).

In practice I'd say it doesn't matter which one we use:
- ULL suffix is gnu89,
- UINT64_C() macro is gnu89,
- "unsigned long long" could be wider in general than 64 bits,
- "uint_least64_t" too could be wider in general than 64 bits,
- for us both results in uint64_t exactly.

So the above is a tie, but the ULL suffix is just nicer. (IMHO :))

Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Richard W.M. Jones]

On Fri, Mar 14, 2014 at 05:26:06PM +0100, Laszlo Ersek wrote:
> (b) UINT64_C() is for "uint_least64_t" (7.18.4.1 Macros for
> minimum-width integer constants). "uint_least64_t" is a required type
> (7.18.1.2 Minimum-width integer types).
> 
> In practice I'd say it doesn't matter which one we use:
> - ULL suffix is gnu89,
> - UINT64_C() macro is gnu89,
> - "unsigned long long" could be wider in general than 64 bits,
> - "uint_least64_t" too could be wider in general than 64 bits,
> - for us both results in uint64_t exactly.
> 
> So the above is a tie, but the ULL suffix is just nicer. (IMHO :))

Interesting discussion here:

https://stackoverflow.com/questions/16360828/what-is-the-purpose-of-macros-for-minimum-width-integer-constants

suggesting that these macros aren't well-specified.  Ho hum.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

 On 03/14/14 17:51, Richard W.M. Jones wrote:
> On Fri, Mar 14, 2014 at 05:26:06PM +0100, Laszlo Ersek wrote:
>> (b) UINT64_C() is for "uint_least64_t" (7.18.4.1 Macros for
>> minimum-width integer constants). "uint_least64_t" is a required type
>> (7.18.1.2 Minimum-width integer types).
>>
>> In practice I'd say it doesn't matter which one we use:
>> - ULL suffix is gnu89,
>> - UINT64_C() macro is gnu89,
>> - "unsigned long long" could be wider in general than 64 bits,
>> - "uint_least64_t" too could be wider in general than 64 bits,
>> - for us both results in uint64_t exactly.
>>
>> So the above is a tie, but the ULL suffix is just nicer. (IMHO :))
> 
> Interesting discussion here:
> 
> https://stackoverflow.com/questions/16360828/what-is-the-purpose-of-macros-for-minimum-width-integer-constants
> 
> suggesting that these macros aren't well-specified.  Ho hum.

I think they are well specified. UINT64_C(x) is the same as
((uint_least64_t)(x)), only the macro replacement text in the latter
uses the appropriate suffix rather than a cast. It produces one integer
constant. (Specific syntax in 6.4.4.1 Integer constants.)

(Note that when I say "the same", the domain of "x" is restricted to "a
decimal, octal, or hexadecimal constant [...] with a value that does not
exceed the limits for the corresponding type.)


*Why* someone would want to use an integer constant with type
"uint_least64_t" is a separate matter. One example follows -- assume all
of the below:
- suppose you write portable C99 source code,
- hence you can't take uint64_t for granted,
- you want a constant that's otherwise small enough to be represented as
"int",
- but you want that constant to trigger the "usual arithmetic
conversions" (see 6.3.1.8) to evaluate expressions that the constant
participates in in at least 64 bits,
- you want the narrowest type that allows you to do this.

Consider

  #define MY_CONSTANT UINT64_C(123)

  /* ... */
  {
    int i = ...;

    ... (MY_CONSTANT * i) ...
  }

If the mathematical result of the multiplication (ie. the math product)
fits in 64 value bits, then the result of the above C multiplication
will be able to *represent* it. If you just write

  #define MY_CONSTANT2 123


  /* ... */
  {
    int i = ...;

    ... (MY_CONSTANT * i) ...
  }

then this may not be the case.

You ensure this "safer" representation (without explicit casts in the
client expressions) by encoding the wider type in the constant itself.
You could use the explicit cast there:

  #define MY_CONSTANT ((uint_least64_t)123)

but UINT64_C() promises not to produce a cast, just an integer constant.
You might care about that for whatever reason.

(You could of course use

  #define MY_CONSTANT 123ULL

too, but that wouldn't select a minimal type for your purpose, portably
speaking (when you don't know much about your target platform).)

I think the mechanics of UINT64_C() are well specified, just the purpose
is obscure :) 7.18.4 "Macros for integer constants" itself states (about
all the macros together):

    The following function-like macros [...] expand to integer constants
    suitable for initializing objects that have integer types
    corresponding to types defined in <stdint.h>. [...]

This question could be asked in the comp.lang.c Usenet group (and then
obsessed over for a few days! :))

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Richard W.M. Jones]

On Fri, Mar 14, 2014 at 06:27:07PM +0100, Laszlo Ersek wrote:
> *Why* someone would want to use an integer constant with type
> "uint_least64_t" is a separate matter. One example follows -- assume all
> of the below:
> - suppose you write portable C99 source code,
> - hence you can't take uint64_t for granted,
> - you want a constant that's otherwise small enough to be represented as
> "int",
> - but you want that constant to trigger the "usual arithmetic
> conversions" (see 6.3.1.8) to evaluate expressions that the constant
> participates in in at least 64 bits,
> - you want the narrowest type that allows you to do this.

- You want the code to self-document its intentions.

I don't think ULL does that because it requires people to know that
ULL is at least 64 bits.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 18:49, Richard W.M. Jones wrote:
> On Fri, Mar 14, 2014 at 06:27:07PM +0100, Laszlo Ersek wrote:
>> *Why* someone would want to use an integer constant with type
>> "uint_least64_t" is a separate matter. One example follows -- assume all
>> of the below:
>> - suppose you write portable C99 source code,
>> - hence you can't take uint64_t for granted,
>> - you want a constant that's otherwise small enough to be represented as
>> "int",
>> - but you want that constant to trigger the "usual arithmetic
>> conversions" (see 6.3.1.8) to evaluate expressions that the constant
>> participates in in at least 64 bits,
>> - you want the narrowest type that allows you to do this.
> 
> - You want the code to self-document its intentions.

I do.

> I don't think ULL does that because it requires people to know that
> ULL is at least 64 bits.

Oh. I see what you mean. This never crossed my mind (= people missing
the fact that unsigned long long can represent at least up to 2^64-1).

Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 16:36, Richard W.M. Jones wrote:
> On Fri, Mar 14, 2014 at 06:50:37AM -0400, Jeff Cody wrote:
>> On 32-bit hosts, some compilers will warn on too large integer constants
>> for constants that are 64-bit in length.  Explicitly put a 'ULL' suffix
>> on those defines.
>> -#define VHDX_FILE_SIGNATURE 0x656C696678646876  /* "vhdxfile" in ASCII */
>> +#define VHDX_FILE_SIGNATURE 0x656C696678646876ULL  /* "vhdxfile" in ASCII */
> 
> I think it's better to use this C99-defined feature (from <stdint.h>):
> 
> #define VHDX_FILE_SIGNATURE UINT64_C(0x656C696678646876)

Here's some pointing to the standard(s)...

"Unsigned long long" is a gnu-ism for C89. It's a standard part of C99.
Last time I checked, qemu used the gnu89 dialect on all build hosts
except SunOS.

So, when you use an integer constant like 0x656C696678646876, that
constant is undefined in C89. Namely,
- 6.1.3.2 "integer constants" (in C89) describes the "ladder" only up to
"unsigned long int" (unsuffixed hexadecimal),
- ULONG_MAX need not be higher than (2^32-1) (5.2.4.2.1 "sizes of
integral types <limits.h>"), which in fact happens to be the case in an
-m32 build,
- 6.1.3.2 "integer constants" (in C89) doesn't say anything about
"extended integer types" (C99 does mention those),
- in gnu89 dialect, gcc decides not to pick "long long unsigned" on its
own for this constant (IOW gcc doesn't elect to "fill in" this undefined
behavior for you even in gnu89 dialect).

So, in gnu89 you really have no other choice than saying "long long
unsigned" yourself (in this instance with the ULL suffix).

In C99 this is not an issue, because the ladder goes up to "long long
unsigned" (see 6.4.4.1 Integer constants), and that type must be able to
represent (2^64-1) minimally (5.2.4.2.1 Sizes of integer types <limits.h>).

The "problem" with the proposed UINT64_C() is that it's C99 only. If we
used C99 rather than gnu89, there would be no need for using UINT64_C(),
the constant would simply work.

(UINT64_C() might be part of gnu89 too, but then the ULL suffix, which
is also gnu89, is just shorter.)

In one sentence, the 0x656C696678646876 constant falls in the

  gnu89 \ c89

relative complement set for -m32, and gcc simply opts against
automatically picking the "unsigned long long" type (which otherwise
belongs to the same relative complement set of language features too),
and leaves the behavior undefined.

(That's my thinking at least.)

Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Peter Maydell]

On 14 March 2014 16:17, Laszlo Ersek <lersek@redhat.com> wrote:
> "Unsigned long long" is a gnu-ism for C89. It's a standard part of C99.
> Last time I checked, qemu used the gnu89 dialect on all build hosts
> except SunOS.

HACKING says we use C99...

-- PMM

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 17:26, Peter Maydell wrote:
> On 14 March 2014 16:17, Laszlo Ersek <lersek@redhat.com> wrote:
>> "Unsigned long long" is a gnu-ism for C89. It's a standard part of C99.
>> Last time I checked, qemu used the gnu89 dialect on all build hosts
>> except SunOS.
> 
> HACKING says we use C99...

HACKING lies then :)

grep for "std=c99" or "std=gnu99". You will find no hits for the former,
and one hit for the latter, when the build host is SunOS. (Or just grep
for '-std='.)

In gcc-4.8.2, -std still defaults to gnu89.

Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 17:35, Laszlo Ersek wrote:
> On 03/14/14 17:26, Peter Maydell wrote:
>> On 14 March 2014 16:17, Laszlo Ersek <lersek@redhat.com> wrote:
>>> "Unsigned long long" is a gnu-ism for C89. It's a standard part of C99.
>>> Last time I checked, qemu used the gnu89 dialect on all build hosts
>>> except SunOS.
>>
>> HACKING says we use C99...
> 
> HACKING lies then :)

Apologies, I didn't impy "lie" as in "lie". I meant "mistake".

Sorry!
Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Peter Maydell]

On 14 March 2014 16:35, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/14/14 17:26, Peter Maydell wrote:
>> On 14 March 2014 16:17, Laszlo Ersek <lersek@redhat.com> wrote:
>>> "Unsigned long long" is a gnu-ism for C89. It's a standard part of C99.
>>> Last time I checked, qemu used the gnu89 dialect on all build hosts
>>> except SunOS.
>>
>> HACKING says we use C99...
>
> HACKING lies then :)
>
> grep for "std=c99" or "std=gnu99". You will find no hits for the former,
> and one hit for the latter, when the build host is SunOS. (Or just grep
> for '-std='.)
>
> In gcc-4.8.2, -std still defaults to gnu89.

HACKING says what we intend. If we need to pass an argument
to gcc to get it to accept C99 constructs we should fix
configure.

-- PMM

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 18:08, Peter Maydell wrote:
> On 14 March 2014 16:35, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 03/14/14 17:26, Peter Maydell wrote:
>>> On 14 March 2014 16:17, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> "Unsigned long long" is a gnu-ism for C89. It's a standard part of C99.
>>>> Last time I checked, qemu used the gnu89 dialect on all build hosts
>>>> except SunOS.
>>>
>>> HACKING says we use C99...
>>
>> HACKING lies then :)
>>
>> grep for "std=c99" or "std=gnu99". You will find no hits for the former,
>> and one hit for the latter, when the build host is SunOS. (Or just grep
>> for '-std='.)
>>
>> In gcc-4.8.2, -std still defaults to gnu89.
> 
> HACKING says what we intend. If we need to pass an argument
> to gcc to get it to accept C99 constructs we should fix
> configure.

I agree 100%.

However, it wouldn't be an immediate, transparent change. For example,
out-of-range left-shifting for a signed int is explicitly undefined
behavior in C99 (6.5.7p4) -- equally for shifting left a negative value
-- and the argument has been made before that C89 does *not* say this.

(Actually I think that it's undefined just the same in C89 -- undefined
by omission. See 3.16, "... by the omission of any explicit definition
of behavior...")

IOW I welcome your proposal, but such a step will make patches like your own

  [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit

very necessary, not just a convenience to sanitize warnings that only
clang emits today.

In any case, I'd propose gnu99 rather than c99, because c99 might not be
a superset of gnu89. In some aspects it would be a step forward, but in
others it could be a step back. Gnu99 only goes forward. (Gnu99 is
promised as the next gcc default.)

Personal note: if qemu moves to c99 or gnu99, as per -std=, I want a
personal license to use constants like 0u and 1u in the code wherever I
want; no style complaints. Deal? :)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Peter Maydell]

On 14 March 2014 17:42, Laszlo Ersek <lersek@redhat.com> wrote:
> However, it wouldn't be an immediate, transparent change. For example,
> out-of-range left-shifting for a signed int is explicitly undefined
> behavior in C99 (6.5.7p4) -- equally for shifting left a negative value
> -- and the argument has been made before that C89 does *not* say this.

Does gcc *actually* change its behaviour in this area depending
on the stanadrd specified?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 19:22, Peter Maydell wrote:
> On 14 March 2014 17:42, Laszlo Ersek <lersek@redhat.com> wrote:
>> However, it wouldn't be an immediate, transparent change. For example,
>> out-of-range left-shifting for a signed int is explicitly undefined
>> behavior in C99 (6.5.7p4) -- equally for shifting left a negative value
>> -- and the argument has been made before that C89 does *not* say this.
> 
> Does gcc *actually* change its behaviour in this area depending
> on the stanadrd specified?

There are at least two aspects to this question.

- The first aspect is: assume that there is a silent change between C89
 and C99, and gcc does implement both versions of the standard
correctly. Then the silent change will affect the qemu code base.

One step in the direction of auditing this is downloading
<http://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10.pdf>, and
searching it for the string QUIET CHANGE.

One good example is for 6.4.4.1 "Integer constants":

                      QUIET CHANGE IN C99

    Unsuffixed integer constants may have different types in C99 than
    in C89. Such constants greater than LONG_MAX are of type unsigned
    long in C89, but are of type long long in C99 (if long long has
    more range than long).

I have no clue what gnu89 does.

- The 2nd aspect is level of C99 support in gcc. We could be using a
non-C89 language feature that has worked well in gnu89 dialect since
forever. The same language feature could be broken in C99 mode on an old
gcc version.

Quickly skimming <http://gcc.gnu.org/c99status.html>, there are
C99-related fixes that are as recent as

* extended identifiers: GCC 4.1 -- we shouldn't be using those,
* integer promotion rules: GCC 4.0 -- no idea about the specifics, but
  this is very important,
* inline functions: GCC 4.3 -- "Inline function support present since
  at least GCC 1.21, but with major differences from C99 semantics
  until 4.3."

For example, RHEL-5 ships gcc-4.1.2-55.el5 (and I gather that people
still build upstream qemu on RHEL-5). Using a c99 dialect, inline
functions could work differently between gcc-4.1 and say gcc-4.8, while
using a gnu89 dialect, there's probably no difference for inline
functions between gcc-4.1 and gcc-4.8.

I think we should ask gcc people... and go forward to gnu99, and fix
resultant bugs gradually.

Laszlo

Re: [Qemu-devel] [PATCH] block: Explicitly specify 'unsigned long long' for VHDX 64-bit constants

[Laszlo Ersek]

On 03/14/14 19:49, Laszlo Ersek wrote:

> One good example is for 6.4.4.1 "Integer constants":
> 
>                       QUIET CHANGE IN C99
> 
>     Unsuffixed integer constants may have different types in C99 than
>     in C89. Such constants greater than LONG_MAX are of type unsigned
>     long in C89, but are of type long long in C99 (if long long has
>     more range than long).
> 
> I have no clue what gnu89 does.

x.c:

  #include <stdio.h>

  int
  main(void)
  {
    fprintf(stdout, "%u\n", (unsigned)sizeof 2147483648);
    return 0;
  }

The following script:

  for I in c89 gnu89 c99 gnu99; do
    echo "==== $I ===="
    gcc -m32 -o x -std=$I x.c
    ./x
  done

outputs:

==== c89 ====
x.c: In function 'main':
x.c:6:3: warning: this decimal constant is unsigned only in ISO C90
[enabled by default]
   fprintf(stdout, "%u\n", (unsigned)sizeof 2147483648);
   ^
4
==== gnu89 ====
x.c: In function 'main':
x.c:6:3: warning: this decimal constant is unsigned only in ISO C90
[enabled by default]
   fprintf(stdout, "%u\n", (unsigned)sizeof 2147483648);
   ^
4
==== c99 ====
8
==== gnu99 ====
8

Laszlo