[Qemu-devel] [PATCH 0/2] target-ppc: Fix clang and w32 warnings

[Qemu-devel] [PATCH 0/2] target-ppc: Fix clang and w32 warnings

[Peter Maydell]

These patches fix compiler warnings from clang and the mingw32
compiler; the aim is to get to a point where we can make the
warnings into errors for both these builds.

Peter Maydell (2):
  target-ppc: Mark SR() and gen_sync_exception() as !CONFIG_USER_ONLY
  target-ppc: Cast ssize_t to size_t before printing with %zx

 hw/ppc/spapr.c         | 2 +-
 target-ppc/translate.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/2] target-ppc: Mark SR() and gen_sync_exception() as !CONFIG_USER_ONLY

[Peter Maydell]

The functions SR() and gen_sync_exception() are only used in softmmu
configs; wrap them in #ifndef CONFIG_USER_ONLY to suppress clang warnings
on the linux-user builds.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-ppc/translate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index d381632..78b9cf3 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -346,11 +346,13 @@ static inline void gen_stop_exception(DisasContext *ctx)
     ctx->exception = POWERPC_EXCP_STOP;
 }
 
+#ifndef CONFIG_USER_ONLY
 /* No need to update nip here, as execution flow will change */
 static inline void gen_sync_exception(DisasContext *ctx)
 {
     ctx->exception = POWERPC_EXCP_SYNC;
 }
+#endif
 
 #define GEN_HANDLER(name, opc1, opc2, opc3, inval, type)                      \
 GEN_OPCODE(name, opc1, opc2, opc3, inval, type, PPC_NONE)
@@ -452,7 +454,10 @@ EXTRACT_HELPER(ME, 1, 5);
 EXTRACT_HELPER(TO, 21, 5);
 
 EXTRACT_HELPER(CRM, 12, 8);
+
+#ifndef CONFIG_USER_ONLY
 EXTRACT_HELPER(SR, 16, 4);
+#endif
 
 /* mtfsf/mtfsfi */
 EXTRACT_HELPER(FPBF, 23, 3);
-- 
1.9.1

[Qemu-devel] [PATCH 2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

[Peter Maydell]

The mingw32 compiler complains about trying to print variables of type
ssize_t with the %z format string specifier. Since we're printing it
as unsigned hex anyway, cast to size_t to silence the warning.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I suspect that this is a compiler bug, but this is the only instance
in the codebase so I'm prepared to work around it to get to a point
where we can turn on warnings-as-errors for w32, because some of the
w32-specific errors really are flagging up issues we need to fix.
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 08401e0..9aaa800 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
     }
     if (spapr->rtas_size > RTAS_MAX_SIZE) {
         hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
-                 spapr->rtas_size, RTAS_MAX_SIZE);
+                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
         exit(1);
     }
     g_free(filename);
-- 
1.9.1

Re: [Qemu-devel] [PATCH 2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

[Stefan Weil]

Am 23.12.2014 um 23:22 schrieb Peter Maydell:
> The mingw32 compiler complains about trying to print variables of type
> ssize_t with the %z format string specifier. Since we're printing it
> as unsigned hex anyway, cast to size_t to silence the warning.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I suspect that this is a compiler bug, but this is the only instance
> in the codebase so I'm prepared to work around it to get to a point
> where we can turn on warnings-as-errors for w32, because some of the
> w32-specific errors really are flagging up issues we need to fix.
> ---
>   hw/ppc/spapr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 08401e0..9aaa800 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>       }
>       if (spapr->rtas_size > RTAS_MAX_SIZE) {
>           hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
> -                 spapr->rtas_size, RTAS_MAX_SIZE);
> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>           exit(1);
>       }
>       g_free(filename);

Which compiler did you use? I get no warning with Debian's 
x86_64-w64-mingw32-gcc 4.6.3 or
native MinGW-w32 compilers.

Stefan

Re: [Qemu-devel] [PATCH 2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

[Peter Maydell]

On 23 December 2014 at 22:36, Stefan Weil <sw@weilnetz.de> wrote:
> Am 23.12.2014 um 23:22 schrieb Peter Maydell:
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>>       }
>>       if (spapr->rtas_size > RTAS_MAX_SIZE) {
>>           hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
>> -                 spapr->rtas_size, RTAS_MAX_SIZE);
>> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>>           exit(1);
>>       }
>>       g_free(filename);
>
>
> Which compiler did you use? I get no warning with Debian's
> x86_64-w64-mingw32-gcc 4.6.3 or
> native MinGW-w32 compilers.

$ i586-mingw32msvc-gcc --version
i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2)

Yes, this is ancient... it's from the Debian mingw32 package.
I just use this for compile testing, not for trying to run.
I should probably switch to the w64 compiler for build tests;
I forget now if there was a reason why I hadn't.

I suspect, as I say, that this is just a generic old-gcc bug,
but it's the only one in the codebase, so it seems easiest
just to fix it.

-- PMM

Re: [Qemu-devel] [PATCH 2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2835 bytes --]

On 12/23/2014 03:22 PM, Peter Maydell wrote:
> The mingw32 compiler complains about trying to print variables of type
> ssize_t with the %z format string specifier. Since we're printing it
> as unsigned hex anyway, cast to size_t to silence the warning.

Hmm.  I wonder if mingw headers are mixing 'long' and 'unsigned int' (or
maybe 'unsigned long' and 'int') for the underlying definitions of
'ssize_t' vs. 'size_t'; both are 32-bits on the platform, but a
difference in underlying type would definitely explain why you get a
compiler warning if you use %zd with size_t or %zu with ssize_t.  On the
other hand, the warning I've most often seen is that native windows
completely lacks %z support, so gcc is smart enough to warn that ANY use
of %z is not going to work, for a function marked
__attribute__((__printf__)) on mingw.

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I suspect that this is a compiler bug, but this is the only instance
> in the codebase so I'm prepared to work around it to get to a point
> where we can turn on warnings-as-errors for w32, because some of the
> w32-specific errors really are flagging up issues we need to fix.

With new enough mingw headers, the alternative solution is to guarantee
that #__USE_MINGW_ANSI_STDIO is defined before including standard
headers, then %z, %j, %lld, and other required constructs magically work
(because you are no longer using the native printf, but the fixed mingw
shim).  Once you do that, you want to use
__attribute__((__gnu_printf__)) instead of __attribute__((__printf__))
in order to teach gcc that yes, your use of %z is really going to work.

For some more background reading:
http://sourceforge.net/p/mingw-w64/wiki2/gnu%20printf/
https://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00029.html
> ---
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 08401e0..9aaa800 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>      }
>      if (spapr->rtas_size > RTAS_MAX_SIZE) {
>          hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
> -                 spapr->rtas_size, RTAS_MAX_SIZE);
> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);

What's the actual compiler warning you got? This feels like it won't
work if the real problem is the compiler telling you that %z is unknown;
conversely, if it is a mismatch between 'size_t' vs. 'ssize_t', we
probably ought to report it to the mingw folks to fix their environment
to use the same underlying type for signed vs. unsigned sizes.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

[Peter Maydell]

On 23 December 2014 at 22:50, Eric Blake <eblake@redhat.com> wrote:
> What's the actual compiler warning you got? This feels like it won't
> work if the real problem is the compiler telling you that %z is unknown;
> conversely, if it is a mismatch between 'size_t' vs. 'ssize_t', we
> probably ought to report it to the mingw folks to fix their environment
> to use the same underlying type for signed vs. unsigned sizes.

It complains:
hw/ppc/spapr.c: In function ‘ppc_spapr_init’:
hw/ppc/spapr.c:1441: warning: format ‘%zx’ expects type ‘size_t’, but
argument 2 has type ‘ssize_t’

and unsurprisingly casting to size_t suppresses it.

%zd gets me:

hw/ppc/spapr.c:1441: warning: format ‘%zd’ expects type ‘signed
size_t’, but argument 2 has type ‘ssize_t’

which is curious. (But we want the hex output anyway...)

Grepping through the headers suggests we have

typedef long _ssize_t;
typedef _ssize_t ssize_t;

I think 'size_t' ends up 'long unsigned int', though stddef.h is
tricky to interpret because of all the macros.

In any case, AIUI mingw isn't really maintained any more, and
so the answer is either "do what is needed to shut up the compiler"
or "declare mingw actually not supported and require mingw-w64"...

-- PMM

Re: [Qemu-devel] [PATCH 0/2] target-ppc: Fix clang and w32 warnings

[Alexander Graf]

On 23.12.14 23:22, Peter Maydell wrote:
> These patches fix compiler warnings from clang and the mingw32
> compiler; the aim is to get to a point where we can make the
> warnings into errors for both these builds.

Thanks, applied both to ppc-next.


Alex

Re: [Qemu-devel] [PATCH 2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

[Stefan Weil]

Am 23.12.2014 um 23:47 schrieb Peter Maydell:
> On 23 December 2014 at 22:36, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 23.12.2014 um 23:22 schrieb Peter Maydell:
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>>>       }
>>>       if (spapr->rtas_size > RTAS_MAX_SIZE) {
>>>           hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
>>> -                 spapr->rtas_size, RTAS_MAX_SIZE);
>>> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>>>           exit(1);
>>>       }
>>>       g_free(filename);
>>
>>
>> Which compiler did you use? I get no warning with Debian's
>> x86_64-w64-mingw32-gcc 4.6.3 or
>> native MinGW-w32 compilers.
> 
> $ i586-mingw32msvc-gcc --version
> i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2)
> 
> Yes, this is ancient... it's from the Debian mingw32 package.
> I just use this for compile testing, not for trying to run.
> I should probably switch to the w64 compiler for build tests;
> I forget now if there was a reason why I hadn't.
> 
> I suspect, as I say, that this is just a generic old-gcc bug,
> but it's the only one in the codebase, so it seems easiest
> just to fix it.
> 
> -- PMM

"git grep" finds 6 "%zx", 66 "%zu" and 80 "%zd" in my QEMU source tree.
Some of those are in debug traces, others are conditionally compiled
depending on your configuration. Are you sure that none of those needs
the same kind of modification, too? I don't thing fixing one of them for
an ancient compiler is worth the trouble of explaining why there is a
type cast.

What about using hwaddr instead of ssize_t for rtas_size (and
HWADDR_PRIx in the format string)? It looks like that would be more
consistent with the rest of the code. See for example the arguments of
function spapr_finalize_fdt.

Stefan