issues when compiling latest Janson from GIT (Debian Sid 64 bits gcc 4.5)

397 views
Skip to first unread message

Basile Starynkevitch

unread,
Feb 2, 2011, 4:46:55 PM2/2/11
to jansso...@googlegroups.com
Hello All

I have some issues when compiling the latest janson GIT
% git log
commit f25698d08f72c16be9867d137474b6578c864091
Author: Petri Lehtinen <pe...@digip.org>
Date: Sun Jan 30 12:53:52 2011 +0200

Fix an unpack example in the docs

My gcc is a 4.5.2 (actually, from experimental in Debian)
% gcc -v 2
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.5.2/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.5.2-2'
--with-bugurl=file:///usr/share/doc/gcc-4.5/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.5 --enable-shared --enable-multiarch
--enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.5 --libdir=/usr/lib
--enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --enable-plugin --enable-gold
--enable-ld=default --with-plugin-ld=ld.gold --enable-objc-gc
--with-arch-32=i586 --with-tune=generic --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu
--target=x86_64-linux-gnu Thread model: posix gcc version 4.5.2 (Debian
4.5.2-2) % make Making all in src make[2]: Entering directory
`/xtra2/Libs/jansson/src' /bin/bash ../libtool --tag=CC
--mode=compile gcc -DHAVE_CONFIG_H -I. -I.. -Wall -Wextra -Werror -g
-O2 -MT pack_unpack.lo -MD -MP -MF .deps/pack_unpack.Tpo -c -o
pack_unpack.lo pack_unpack.c libtool: compile: gcc -DHAVE_CONFIG_H -I.
-I.. -Wall -Wextra -Werror -g -O2 -MT pack_unpack.lo -MD -MP
-MF .deps/pack_unpack.Tpo -c pack_unpack.c -fPIC -DPIC
-o .libs/pack_unpack.o cc1: warnings being treated as errors
pack_unpack.c: In function 'json_vpack_ex': pack_unpack.c:460:5: error:
passing argument 2 of 'pack' from incompatible pointer type
pack_unpack.c:144:16: note: expected 'struct __va_list_tag (*)[1]' but
argument is of type 'struct __va_list_tag **' pack_unpack.c: In
function 'json_vunpack_ex': pack_unpack.c:516:5: error: passing
argument 3 of 'unpack' from incompatible pointer type
pack_unpack.c:330:12: note: expected 'struct __va_list_tag (*)[1]' but
argument is of type 'struct __va_list_tag **' make[2]: ***
[pack_unpack.lo] Error 1 make[2]: Leaving directory
`/xtra2/Libs/jansson/src' make[1]: *** [all-recursive] Error 1 make[1]:
Leaving directory `/xtra2/Libs/jansson' make: *** [all] Error 2

I am not sure your use of pointers to va_list is absolutely standard
conforming (but maybe I am wrong). A quick glance at the man page seems
to suggest avoiding that, and using va_copy instead. Anyway, the va_arg
of <stdarg.h> is deeply builtin inside compilers such as GCC and should
be handled with extreme care.

By the way, the future gcc-4.6 produce the same error message, and so
does gcc 4.4 and clang 2.7 (from LLVM) also.
libtool: compile: clang -DHAVE_CONFIG_H -I. -I.. -Wall -Wextra -Werror -g -O2 -MT pack_unpack.lo -MD -MP -MF .deps/pack_unpack.Tpo -c pack_unpack.c -fPIC -DPIC -o .libs/pack_unpack.o
pack_unpack.c:460:22: error: incompatible pointer types passing
'__va_list_tag **', expected 'va_list *'
value = pack(&s, &ap);
^~~
pack_unpack.c:516:25: error: incompatible pointer types passing
'__va_list_tag **', expected 'va_list *'
if(unpack(&s, root, &ap))
^~~
2 diagnostics generated.

Regards.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

gsme...@gmail.com

unread,
Feb 2, 2011, 6:30:55 PM2/2/11
to jansso...@googlegroups.com, Graeme Smecher
From: Graeme Smecher <graeme....@mail.mcgill.ca>

Reported-By: Basile Starynkevitch <bas...@starynkevitch.net>

Functions taking va_args are munged to receive arguments of type
'__va_list_tag *'. This patch uses va_copy to coerce them to the expected type
so we don't get compiler errors.

Tested on x86_64, both 32-bit and 64-bit compiles.
---
src/pack_unpack.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/pack_unpack.c b/src/pack_unpack.c
index 758e2f9..27d73e0 100644
--- a/src/pack_unpack.c
+++ b/src/pack_unpack.c
@@ -441,6 +441,7 @@ json_t *json_vpack_ex(json_error_t *error, size_t flags,
const char *fmt, va_list ap)
{
scanner_t s;
+ va_list ap_copy;
json_t *value;

jsonp_error_init(error, "");
@@ -457,7 +458,8 @@ json_t *json_vpack_ex(json_error_t *error, size_t flags,
s.column = 0;

next_token(&s);
- value = pack(&s, &ap);
+ va_copy(ap_copy, ap);
+ value = pack(&s, &ap_copy);

next_token(&s);
if(s.token) {
@@ -497,6 +499,7 @@ int json_vunpack_ex(json_t *root, json_error_t *error, size_t flags,
const char *fmt, va_list ap)
{
scanner_t s;
+ va_list ap_copy;

jsonp_error_init(error, "");

@@ -513,7 +516,8 @@ int json_vunpack_ex(json_t *root, json_error_t *error, size_t flags,

next_token(&s);

- if(unpack(&s, root, &ap))
+ va_copy(ap_copy, ap);
+ if(unpack(&s, root, &ap_copy))
return -1;

next_token(&s);
--
1.7.2.3

Graeme Smecher

unread,
Feb 2, 2011, 6:40:19 PM2/2/11
to jansso...@googlegroups.com
Hi Basile,


On 02/02/11 01:46 PM, Basile Starynkevitch wrote:
I am not sure your use of pointers to va_list is absolutely standard
conforming (but maybe I am wrong). A quick glance at the man page seems
to suggest avoiding that, and using va_copy instead. Anyway, the va_arg
of <stdarg.h> is deeply builtin inside compilers such as GCC and should
be handled with extreme care.
  

Thanks for the post -- I was able to reproduce these errors using clang on x86_64. I've just posted a patch to the list that resolves these problems for me. Here are the gory details.

I *think* the use of va_arg pointers is valid. From the c99 standard, section 7.15:

212) It is permitted to create a pointer to a va_list and pass that pointer to another function, in which
     case the original function may make further use of the original list after the other function returns.

Now, I'm not familiar enough with the standard (or its implementations) to completely trust a quick reading of an unfamiliar document. We're forced into using va_arg pointers instead of va_copy due to the recursive implementation of the pack/unpack functions. Prior to this, we tried passing va_args, but the results weren't portable. According to the manpage:

If  ap is passed to a function that uses va_arg(ap,type) then the value of ap is undefined after the return of that function.

It's worth noting that Python uses a similar approach for the same reasons.

Anyhow, if anyone on the list is able to confirm (or condemn) this approach, I'm eager to hear it.

cheers,
Graeme

gsme...@gmail.com

unread,
Feb 2, 2011, 6:48:41 PM2/2/11
to jansso...@googlegroups.com, Graeme Smecher

Graeme Smecher

unread,
Feb 2, 2011, 6:51:46 PM2/2/11
to jansso...@googlegroups.com
Hi all,

Sorry for the clutter.

This version of the patch ignores the rules: "Each invocation of
va_copy() must be matched by a corresponding invocation of va_end() in
the same function." I've just posted a revised version of the patch.

best,
Graeme


Basile Starynkevitch

unread,
Feb 3, 2011, 2:22:26 AM2/3/11
to jansso...@googlegroups.com, Graeme Smecher
On Wed, 02 Feb 2011 15:51:46 -0800
Graeme Smecher <gsme...@gmail.com> wrote:

> Hi all,
>
> Sorry for the clutter.
>
> On 02/02/11 03:30 PM, gsme...@gmail.com wrote:
> > From: Graeme Smecher<graeme....@mail.mcgill.ca>
> >
> > Reported-By: Basile Starynkevitch<bas...@starynkevitch.net>
> >
> > Functions taking va_args are munged to receive arguments of type
> > '__va_list_tag *'. This patch uses va_copy to coerce them to the expected type
> > so we don't get compiler errors.
> >
> > Tested on x86_64, both 32-bit and 64-bit compiles.

[....]


> This version of the patch ignores the rules: "Each invocation of
> va_copy() must be matched by a corresponding invocation of va_end() in
> the same function." I've just posted a revised version of the patch.


I am not very familiar with git and still learning it, but apparently,
that patch was not yet pushed to the master branch of
https://github.com/akheron/jansson.git at least I did not get it with
git pull.

I hope you will push it soon.

Or did I miss something?

Petri Lehtinen

unread,
Feb 3, 2011, 2:32:30 AM2/3/11
to jansso...@googlegroups.com, Graeme Smecher
Hi all.

Basile: Good catch. I didn't realize that you cannot take a pointer of
an existing va_list.

Graeme: Thanks for quickly finding out what's going on :) I'm happy
that the standard says that taking pointers of va_list is the way to
go. Now we're backed up by something else than only the CPython code.

Is this the revised version of the patch? I still don't see any
va_end() calls...

> --
> Jansson users mailing list
> jansso...@googlegroups.com
> http://groups.google.com/group/jansson-users

gsme...@gmail.com

unread,
Feb 3, 2011, 10:51:26 AM2/3/11
to jansso...@googlegroups.com, Graeme Smecher
From: Graeme Smecher <graeme....@mail.mcgill.ca>

Reported-By: Basile Starynkevitch <bas...@starynkevitch.net>

Functions taking va_args are munged to receive arguments of type
'__va_list_tag *'. This patch uses va_copy to coerce them to the expected type
so we don't get compiler errors.

Tested on x86_64, both 32-bit and 64-bit compiles.
---

src/pack_unpack.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/pack_unpack.c b/src/pack_unpack.c
index 758e2f9..e00660d 100644


--- a/src/pack_unpack.c
+++ b/src/pack_unpack.c
@@ -441,6 +441,7 @@ json_t *json_vpack_ex(json_error_t *error, size_t flags,
const char *fmt, va_list ap)
{
scanner_t s;
+ va_list ap_copy;
json_t *value;

jsonp_error_init(error, "");

@@ -457,7 +458,9 @@ json_t *json_vpack_ex(json_error_t *error, size_t flags,


s.column = 0;

next_token(&s);
- value = pack(&s, &ap);
+ va_copy(ap_copy, ap);
+ value = pack(&s, &ap_copy);

+ va_end(ap_copy);

next_token(&s);
if(s.token) {
@@ -497,6 +500,7 @@ int json_vunpack_ex(json_t *root, json_error_t *error, size_t flags,


const char *fmt, va_list ap)
{
scanner_t s;
+ va_list ap_copy;

jsonp_error_init(error, "");

@@ -513,8 +517,12 @@ int json_vunpack_ex(json_t *root, json_error_t *error, size_t flags,



next_token(&s);

- if(unpack(&s, root, &ap))
+ va_copy(ap_copy, ap);

+ if(unpack(&s, root, &ap_copy)) {
+ va_end(ap_copy);
return -1;
+ }
+ va_end(ap_copy);

next_token(&s);
if(s.token) {
--
1.7.2.3

Petri Lehtinen

unread,
Feb 3, 2011, 1:44:21 PM2/3/11
to jansso...@googlegroups.com, Graeme Smecher, Basile Starynkevitch
gsme...@gmail.com wrote:
> From: Graeme Smecher <graeme....@mail.mcgill.ca>
>
> Reported-By: Basile Starynkevitch <bas...@starynkevitch.net>
>
> Functions taking va_args are munged to receive arguments of type
> '__va_list_tag *'. This patch uses va_copy to coerce them to the expected type
> so we don't get compiler errors.
>
> Tested on x86_64, both 32-bit and 64-bit compiles.
> ---
> src/pack_unpack.c | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)

Applied, thanks.

Basile: Now it's in master, feel free to test.

Basile Starynkevitch

unread,
Feb 3, 2011, 2:17:05 PM2/3/11
to jansso...@googlegroups.com, Petri Lehtinen, Graeme Smecher
On Thu, 3 Feb 2011 20:44:21 +0200
Petri Lehtinen <pe...@digip.org> wrote:

> gsme...@gmail.com wrote:
> > From: Graeme Smecher <graeme....@mail.mcgill.ca>

> > Functions taking va_args are munged to receive arguments of type


> > '__va_list_tag *'. This patch uses va_copy to coerce them to the expected type
> > so we don't get compiler errors.

> Basile: Now it's in master, feel free to test.

Thanks. It compiles!

Reply all
Reply to author
Forward
0 new messages