[libvirt] building error
When I build libvirt, I meet the following error: make[3]: Entering directory `/home/wency/rpmbuild/BUILD/libvirt-0.9.2/daemon' CC libvirtd-libvirtd.o CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2043: error: invalid initializer make[3]: *** [libvirtd-remote.o] Error 1 This patch can fix the problem. But I do not know whether it is right.
From b03a5c08b751440460bf4cb02fea061b77fb3ef5 Mon Sep 17 00:00:00 2001 From: Wen Congyang <wency@cn.fujitsu.com> Date: Wed, 29 Jun 2011 19:04:19 +0800 Subject: [PATCH 1/2] daemon: Fix building error with polkit1
--- daemon/remote.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 9e6cf77..042894b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2041,7 +2041,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, goto authdeny; } PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", - virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); + virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, (char *)ident); VIR_INFO("Policy allowed action %s from pid %d, uid %d", action, callerPid, callerUid); ret->complete = 1; -- 1.7.1
On 06/29/2011 05:18 AM, Wen Congyang wrote:
When I build libvirt, I meet the following error: make[3]: Entering directory `/home/wency/rpmbuild/BUILD/libvirt-0.9.2/daemon' CC libvirtd-libvirtd.o CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2043: error: invalid initializer make[3]: *** [libvirtd-remote.o] Error 1
This patch can fix the problem. But I do not know whether it is right.
This sounds highly related to Matthias' issue: https://www.redhat.com/archives/libvir-list/2011-June/msg01484.html What version of systemtap headers and gcc are you using, that produce this error?
+++ b/daemon/remote.c @@ -2041,7 +2041,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, goto authdeny; } PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", - virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); + virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, (char *)ident);
If a cast really helps matters, I'd almost rather hide it within the #define PROBE() than make callers have to worry about it, but I'm reluctant to add a cast without knowing exactly why you are hitting compilation failure. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
At 06/29/2011 11:04 PM, Eric Blake Write:
On 06/29/2011 05:18 AM, Wen Congyang wrote:
When I build libvirt, I meet the following error: make[3]: Entering directory `/home/wency/rpmbuild/BUILD/libvirt-0.9.2/daemon' CC libvirtd-libvirtd.o CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2043: error: invalid initializer make[3]: *** [libvirtd-remote.o] Error 1
This patch can fix the problem. But I do not know whether it is right.
This sounds highly related to Matthias' issue: https://www.redhat.com/archives/libvir-list/2011-June/msg01484.html
What version of systemtap headers and gcc are you using, that produce this error?
I use RHEL6RC, and the version of systemtap headers and gcc are: # rpm -qa 'systemtap*' systemtap-sdt-devel-1.2-9.el6.x86_64 systemtap-runtime-1.2-9.el6.x86_64 systemtap-1.2-9.el6.x86_64 # rpm -q gcc gcc-4.4.4-13.el6.x86_64
+++ b/daemon/remote.c @@ -2041,7 +2041,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, goto authdeny; } PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", - virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); + virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, (char *)ident);
If a cast really helps matters, I'd almost rather hide it within the #define PROBE() than make callers have to worry about it, but I'm reluctant to add a cast without knowing exactly why you are hitting compilation failure.
On 06/29/2011 06:13 PM, Wen Congyang wrote:
This sounds highly related to Matthias' issue: https://www.redhat.com/archives/libvir-list/2011-June/msg01484.html
Matthias was using 1.3...
What version of systemtap headers and gcc are you using, that produce this error?
I use RHEL6RC, and the version of systemtap headers and gcc are:
# rpm -qa 'systemtap*' systemtap-sdt-devel-1.2-9.el6.x86_64
but you are using 1.2. The older we go, the worse the systemtap headers were, apparently. Unfortunately, I don't currently have a machine with headers that old to debug it myself.
} PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", - virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); + virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, (char *)ident);
If a cast really helps matters, I'd almost rather hide it within the #define PROBE() than make callers have to worry about it, but I'm reluctant to add a cast without knowing exactly why you are hitting compilation failure.
What does STAP_PROBE3 look like in your /usr/include/sys/sdt.h? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
At 07/01/2011 04:54 AM, Eric Blake Write:
On 06/29/2011 06:13 PM, Wen Congyang wrote:
This sounds highly related to Matthias' issue: https://www.redhat.com/archives/libvir-list/2011-June/msg01484.html
Matthias was using 1.3...
What version of systemtap headers and gcc are you using, that produce this error?
I use RHEL6RC, and the version of systemtap headers and gcc are:
# rpm -qa 'systemtap*' systemtap-sdt-devel-1.2-9.el6.x86_64
but you are using 1.2. The older we go, the worse the systemtap headers were, apparently. Unfortunately, I don't currently have a machine with headers that old to debug it myself.
} PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", - virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); + virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, (char *)ident);
If a cast really helps matters, I'd almost rather hide it within the #define PROBE() than make callers have to worry about it, but I'm reluctant to add a cast without knowing exactly why you are hitting compilation failure.
What does STAP_PROBE3 look like in your /usr/include/sys/sdt.h?
#if ! defined EXPERIMENTAL_KPROBE_SDT .... #define STAP_PROBE3_(probe,label,parm1,parm2,parm3) \ do STAP_SEMAPHORE(probe) { \ STAP_SDT_VOLATILE __typeof__((parm1)) arg1 = parm1; \ STAP_SDT_VOLATILE __typeof__((parm2)) arg2 = parm2; \ STAP_SDT_VOLATILE __typeof__((parm3)) arg3 = parm3; \ STAP_UNINLINE; \ STAP_PROBE_DATA(probe,STAP_UPROBE_GUARD,2f); \ __asm__ volatile ("2:\n" \ STAP_NOP "/* %0 %1 %2 */" :: "g"(arg1), "g"(arg2), "g"(arg3)); \ } while (0) .... #else /* ! defined EXPERIMENTAL_KPROBE_SDT */ ... #define STAP_PROBE3_(probe,label,parm1,parm2,parm3) \ do STAP_SEMAPHORE(probe) { \ __extension__ struct {size_t arg1 __attribute__((aligned(8))); \ size_t arg2 __attribute__((aligned(8))); \ size_t arg3 __attribute__((aligned(8)));} \ stap_probe3_args = {(size_t)parm1, (size_t)parm2, (size_t)parm3}; \ STAP_PROBE_DATA(probe,STAP_GUARD,3); \ syscall (STAP_SYSCALL, #probe, STAP_GUARD, &stap_probe3_args); \ } while (0) ... #endif ... #define STAP_PROBE3(provider,probe,parm1,parm2,parm3) \ STAP_PROBE3_(probe,STAP_LABEL(STAP_LABEL_PREFIX(probe),STAP_COUNTER),(parm1),(parm2),(parm3))
On 06/30/2011 08:34 PM, Wen Congyang wrote:
# rpm -qa 'systemtap*' systemtap-sdt-devel-1.2-9.el6.x86_64
What does STAP_PROBE3 look like in your /usr/include/sys/sdt.h?
#if ! defined EXPERIMENTAL_KPROBE_SDT .... #define STAP_PROBE3_(probe,label,parm1,parm2,parm3) \ do STAP_SEMAPHORE(probe) { \ STAP_SDT_VOLATILE __typeof__((parm1)) arg1 = parm1; \ STAP_SDT_VOLATILE __typeof__((parm2)) arg2 = parm2; \ STAP_SDT_VOLATILE __typeof__((parm3)) arg3 = parm3; \
So is it this variant...
#else /* ! defined EXPERIMENTAL_KPROBE_SDT */ ... #define STAP_PROBE3_(probe,label,parm1,parm2,parm3) \ do STAP_SEMAPHORE(probe) { \ __extension__ struct {size_t arg1 __attribute__((aligned(8))); \ size_t arg2 __attribute__((aligned(8))); \ size_t arg3 __attribute__((aligned(8)));} \ stap_probe3_args = {(size_t)parm1, (size_t)parm2, (size_t)parm3}; \
...or this variant that got used? Would you mind giving the relevant portion of gcc -E output around the line that is failing, so we can see the full macro expansion that the compiler doesn't like? Looking back at your original report: remote.c:2043: error: invalid initializer - virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); + virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, (char *)ident); and the declaration in question: char ident[100]; I'm guessing it's the first variant, and that: volatile __typeof__(ident) arg3 = ident; is getting translated to: volatile char[100] arg3 = ident; which really is invalid, but that adding the cast to char* fixes things, so that the compiler is then doing: volatile char *arg3 = (char*)ident; And if we really are dealing with a macro that broken, then I'm entirely in favor of fixing our definition of PROBE to do the casting in advance, rather than making all PROBE callers worry about broken <sys/sdt.h>. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/01/2011 09:04 AM, Eric Blake wrote:
And if we really are dealing with a macro that broken, then I'm entirely in favor of fixing our definition of PROBE to do the casting in advance, rather than making all PROBE callers worry about broken <sys/sdt.h>.
Yuck - we defined PROBE as a var-arg macro, so it's taking me longer to figure out how to add the cast to each of a variable number of arguments :( -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Systemtap 1.2 <sys/sdt.h> tried to expand STAP_PROBE3 into an initialization: volatile __typeof__(arg) foo = arg; but that fails if arg was declared as 'char arg[100]'. Rather than make all callers to PROBE deal with the stupidity of <sys/sdt.h>, we instead make PROBE cast away the problem. Some of this preprocessor abuse copies ideas in src/libvirt.c. * daemon/libvirtd.h (PROBE): Add casts to all arguments, using... (VIR_ADD_CASTS, VIR_ADD_CAST, VIR_ADD_CAST2, VIR_ADD_CAST3) (VIR_ADD_CAST_EXPAND, VIR_ADD_CAST_PASTE, VIR_COUNT_ARGS) (VIR_ARG5, PROBE_EXPAND): New macros. Reported by Wen Congyang. --- This took me entirely too long to come up with. It works for me with systemtap 1.4; if I can get feedback from systemtap 1.2 and 1.3 users, then I'll gladly apply it, and we can forget about having to do stupid casts or other workarounds at every PROBE client. daemon/libvirtd.h | 32 +++++++++++++++++++++++++++++++- 1 files changed, 31 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 6c604fc..2f0987e 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -46,11 +46,41 @@ # define LIBVIRTD_PROBES_H # include "probes.h" # endif /* LIBVIRTD_PROBES_H */ + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG5(__VA_ARGS__, 4, 3, 2, 1) +# define VIR_ARG5(_1, _2, _3, _4, _5, ...) _5 +# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) +# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__) + +/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a)) +# define VIR_ADD_CAST2(a, b) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b) +# define VIR_ADD_CAST3(a, b, c) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c) + +# define VIR_ADD_CASTS(...) \ + VIR_ADD_CAST_EXPAND(VIR_ADD_CAST, VIR_COUNT_ARGS(__VA_ARGS__), \ + __VA_ARGS__) + +# define PROBE_EXPAND(NAME, ARGS) NAME(ARGS) # define PROBE(NAME, FMT, ...) \ VIR_DEBUG_INT("trace." __FILE__ , __func__, __LINE__, \ #NAME ": " FMT, __VA_ARGS__); \ if (LIBVIRTD_ ## NAME ## _ENABLED()) { \ - LIBVIRTD_ ## NAME(__VA_ARGS__); \ + PROBE_EXPAND(LIBVIRTD_ ## NAME, \ + VIR_ADD_CASTS(__VA_ARGS__)); \ } # else # define PROBE(NAME, FMT, ...) \ -- 1.7.4.4
At 07/02/2011 05:43 AM, Eric Blake Write:
Systemtap 1.2 <sys/sdt.h> tried to expand STAP_PROBE3 into an initialization: volatile __typeof__(arg) foo = arg; but that fails if arg was declared as 'char arg[100]'. Rather than make all callers to PROBE deal with the stupidity of <sys/sdt.h>, we instead make PROBE cast away the problem. Some of this preprocessor abuse copies ideas in src/libvirt.c.
* daemon/libvirtd.h (PROBE): Add casts to all arguments, using... (VIR_ADD_CASTS, VIR_ADD_CAST, VIR_ADD_CAST2, VIR_ADD_CAST3) (VIR_ADD_CAST_EXPAND, VIR_ADD_CAST_PASTE, VIR_COUNT_ARGS) (VIR_ARG5, PROBE_EXPAND): New macros. Reported by Wen Congyang. ---
This took me entirely too long to come up with. It works for me with systemtap 1.4; if I can get feedback from systemtap 1.2 and 1.3 users, then I'll gladly apply it, and we can forget about having to do stupid casts or other workarounds at every PROBE client.
It works for me with systemtap 1.2.9.
daemon/libvirtd.h | 32 +++++++++++++++++++++++++++++++- 1 files changed, 31 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 6c604fc..2f0987e 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -46,11 +46,41 @@ # define LIBVIRTD_PROBES_H # include "probes.h" # endif /* LIBVIRTD_PROBES_H */ + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG5(__VA_ARGS__, 4, 3, 2, 1) +# define VIR_ARG5(_1, _2, _3, _4, _5, ...) _5 +# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) +# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__) + +/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a)) +# define VIR_ADD_CAST2(a, b) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b) +# define VIR_ADD_CAST3(a, b, c) \ + VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c) + +# define VIR_ADD_CASTS(...) \ + VIR_ADD_CAST_EXPAND(VIR_ADD_CAST, VIR_COUNT_ARGS(__VA_ARGS__), \ + __VA_ARGS__) + +# define PROBE_EXPAND(NAME, ARGS) NAME(ARGS) # define PROBE(NAME, FMT, ...) \ VIR_DEBUG_INT("trace." __FILE__ , __func__, __LINE__, \ #NAME ": " FMT, __VA_ARGS__); \ if (LIBVIRTD_ ## NAME ## _ENABLED()) { \ - LIBVIRTD_ ## NAME(__VA_ARGS__); \ + PROBE_EXPAND(LIBVIRTD_ ## NAME, \ + VIR_ADD_CASTS(__VA_ARGS__)); \ } # else # define PROBE(NAME, FMT, ...) \
2011/7/1 Eric Blake <eblake@redhat.com>:
Systemtap 1.2 <sys/sdt.h> tried to expand STAP_PROBE3 into an initialization: volatile __typeof__(arg) foo = arg; but that fails if arg was declared as 'char arg[100]'. Rather than make all callers to PROBE deal with the stupidity of <sys/sdt.h>, we instead make PROBE cast away the problem. Some of this preprocessor abuse copies ideas in src/libvirt.c.
* daemon/libvirtd.h (PROBE): Add casts to all arguments, using... (VIR_ADD_CASTS, VIR_ADD_CAST, VIR_ADD_CAST2, VIR_ADD_CAST3) (VIR_ADD_CAST_EXPAND, VIR_ADD_CAST_PASTE, VIR_COUNT_ARGS) (VIR_ARG5, PROBE_EXPAND): New macros. Reported by Wen Congyang. ---
This took me entirely too long to come up with. It works for me with systemtap 1.4; if I can get feedback from systemtap 1.2 and 1.3 users, then I'll gladly apply it, and we can forget about having to do stupid casts or other workarounds at every PROBE client.
It still compiled with systemtap 1.3.
daemon/libvirtd.h | 32 +++++++++++++++++++++++++++++++- 1 files changed, 31 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 6c604fc..2f0987e 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -46,11 +46,41 @@ # define LIBVIRTD_PROBES_H # include "probes.h" # endif /* LIBVIRTD_PROBES_H */ + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG5(__VA_ARGS__, 4, 3, 2, 1) +# define VIR_ARG5(_1, _2, _3, _4, _5, ...) _5 +# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) +# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__)
Took me a moment to figure it out, but I think I've got it.
+/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a))
Is this really true? What about a long long value (64bit) on a 32bit platform? intptr_t and void* are only 32bit on a 32bit platform, aren't they? -- Matthias Bolte http://photron.blogspot.com
On 07/04/2011 01:10 AM, Matthias Bolte wrote:
+/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a))
Is this really true? What about a long long value (64bit) on a 32bit platform? intptr_t and void* are only 32bit on a 32bit platform, aren't they?
Hmm, interesting point. On the other hand, can PROBE even operate on 'long long', or does it only operate on 'size_t'/'void *' operands? That is, the whole point of system tap is to inject probe points that make it easier to run system tap on a binary, where each use of the PROBE() macro injects enough code to pass arbitrary bits through the kernel so that a running systemtap can then reinterpret those bits with the appropriate type according to the probe point that systemtap is tracing. I also discovered that my cast is not always gcc silent - gcc insists on warning for (intptr_t)(func()) for any func that returns a pointer instead of an integer; but in those cases, the use of an intermediate variable works around the warning. It is really annoying that gcc does not provide any counterpart to __typeof__ that would let you produce the decayed type of an input argument - life would be much simpler if there were a way to decay an expression of type 'char[100]' into an intermediate variable of type 'char *'. At any rate, since none of our current uses of PROBE() pass either 'long long' or a function call that returns 'char *', we aren't adding any additional problems by accepting this patch, but we also might be adding a latent bug that would be hard to diagnose in the future. I'm not sure whether we want to apply this patch, or just require systemtap 1.3 or newer as a prerequisite for compiling libvirt. Thoughts? How prevalent are systemtap 1.2 installations? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/05/2011 10:03 AM, Eric Blake wrote:
It is really annoying that gcc does not provide any counterpart to __typeof__ that would let you produce the decayed type of an input argument - life would be much simpler if there were a way to decay an expression of type 'char[100]' into an intermediate variable of type 'char *'.
At any rate, since none of our current uses of PROBE() pass either 'long long' or a function call that returns 'char *', we aren't adding any additional problems by accepting this patch, but we also might be adding a latent bug that would be hard to diagnose in the future. I'm not sure whether we want to apply this patch, or just require systemtap 1.3 or newer as a prerequisite for compiling libvirt. Thoughts? How prevalent are systemtap 1.2 installations?
Hi - I'm a systemtap developer, and happened upon this thread in a google search. I'm CCing the systemtap list in this reply, so if any of those readers want a fuller context, start around here: https://www.redhat.com/archives/libvir-list/2011-July/msg00053.html I'm sorry to discover that you're fighting old sdt.h bugs. If only we could put on our time-traveling cap and get it right the first time... We do hope that all the corner cases are dealt with correctly in the newest systemtap package. Please let us know if you find otherwise. All these typing issues are indeed very hairy to get right, which is a large part of what inspired our sdt.h rewrite in 1.4. If you really want to maintain compatibility with older sdt.h, then perhaps the newer version can guide you in wrapping around some of the issues. For example, dealing with array types, the new version has: #define _SDT_ARGARRAY(x) (__builtin_classify_type (x) == 14 \ || __builtin_classify_type (x) == 5) So perhaps you could copy that macro and let your compatibility casting do something like this: #define _SDT_CAST(x) \ __builtin_choose_expr (_SDT_ARGARRAY(x), (uintptr_t)(x), (x)) That way array-like types will be cast, but all others will be left with their proper size and signedness, including e.g. any long long that wouldn't fit in a 32-bit intptr. That __builtin_choose_expr only works in C, but hopefully for libvirt that should suffice. HTH, Josh Stone
On 07/05/2011 09:03 PM, Josh Stone wrote:
I'm sorry to discover that you're fighting old sdt.h bugs. If only we could put on our time-traveling cap and get it right the first time... We do hope that all the corner cases are dealt with correctly in the newest systemtap package. Please let us know if you find otherwise.
Thanks for joining the thread. So far, all the problems that libvirt has had were with sdt.h 1.2 and 1.3; 1.4 compiled out of the box, and it was only in porting to older versions where we ran into interesting issues.
All these typing issues are indeed very hairy to get right, which is a large part of what inspired our sdt.h rewrite in 1.4. If you really want to maintain compatibility with older sdt.h, then perhaps the newer version can guide you in wrapping around some of the issues. For example, dealing with array types, the new version has:
#define _SDT_ARGARRAY(x) (__builtin_classify_type (x) == 14 \ || __builtin_classify_type (x) == 5)
Too bad gcc doesn't document this builtin.
So perhaps you could copy that macro and let your compatibility casting do something like this:
#define _SDT_CAST(x) \ __builtin_choose_expr (_SDT_ARGARRAY(x), (uintptr_t)(x), (x))
That depends on gcc - so we'd have to provide a fallback define to a plain cast for other compilers. I'll keep that in mind if the current patch for libvirt (which doesn't use any gcc extensions) proves to be problematic. And we may still end up defining away the problem, by just stating that libvirt requires systemtap 1.4 or newer before libvirt will use systemtap.
That way array-like types will be cast, but all others will be left with their proper size and signedness, including e.g. any long long that wouldn't fit in a 32-bit intptr. That __builtin_choose_expr only works in C, but hopefully for libvirt that should suffice.
Yes, libvirt is C-only. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/06/2011 08:45 AM, Eric Blake wrote:
#define _SDT_ARGARRAY(x) (__builtin_classify_type (x) == 14 \ || __builtin_classify_type (x) == 5)
Too bad gcc doesn't document this builtin.
It's documented in the Internals manual: http://gcc.gnu.org/onlinedocs/gccint/Varargs.html http://gcc.gnu.org/viewcvs/trunk/gcc/typeclass.h?view=markup
So perhaps you could copy that macro and let your compatibility casting do something like this:
#define _SDT_CAST(x) \ __builtin_choose_expr (_SDT_ARGARRAY(x), (uintptr_t)(x), (x))
That depends on gcc - so we'd have to provide a fallback define to a plain cast for other compilers. I'll keep that in mind if the current patch for libvirt (which doesn't use any gcc extensions) proves to be problematic.
Well, sdt.h itself uses such extensions, so you may not get far with very different compilers anyway. But FWIW, it seems to work fine on F15's clang-2.8.
And we may still end up defining away the problem, by just stating that libvirt requires systemtap 1.4 or newer before libvirt will use systemtap.
I wouldn't blame you, if you can get away with that. We think the current version of SDT is more sane overall, with better flexibility as well as better resulting metadata. Josh
participants (4)
-
Eric Blake -
Josh Stone -
Matthias Bolte -
Wen Congyang