[PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Stefan Hajnoczi via lttng-dev]

LTTng Userspace Tracer 2.11 with sdt enabled can break builds of
applications that include <sys/sdt.h> themselves. QEMU
(https://qemu.org/) is one example.

When applications include <sys/sdt.h> themselves before LTTng Userspace
Tracer header files and the SDT_USE_VARIADIC macro is not defined,
compilation fails due to the absense of STAP_PROBEV() macro that LTTng
Userspace Tracer requires.

Define SDT_USE_VARIADIC in the pkg-config file so that all compilation
units that use LTTng Userspace Tracer are guaranteed to have the macro
defined even if <sys/sdt.h> is included elsewhere before LTTng Userspace
Tracer header files.

In other words, define SDT_USE_VARIADIC on the compiler command-line
instead of in <lttng/tracepoint.h>. This way it will always been defined
when <sys/sdt.h> is included.

Make sure to define SDT_USE_VARIADIC when <lttng/tracepoint.h> is
included without it. This happens when applications do not use
pkg-config.

Fixes: b2f60c7986bb69f81b79b68f1bfe83aafa3278a7 ("Add sdt.h integration")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Warning: this patch doesn't work yet. SDT_CFLAGS also needs to be passed
into LTTng Userspace Tracer's own CFLAGS so that compilation can
succeed. I don't know the autoconf/automake syntax to do this and wanted
to see what you think of this solution before investing more time.
---
 include/lttng/tracepoint.h | 2 ++
 configure.ac               | 2 ++
 lttng-ust.pc.in            | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 7cfae3e4..6b699373 100644
--- a/include/lttng/tracepoint.h
+++ b/include/lttng/tracepoint.h
@@ -35,7 +35,9 @@
 #include <lttng/ust-compiler.h>
 
 #ifdef LTTNG_UST_HAVE_SDT_INTEGRATION
+#ifndef SDT_USE_VARIADIC
 #define SDT_USE_VARIADIC
+#endif
 #include <sys/sdt.h>
 #define LTTNG_STAP_PROBEV STAP_PROBEV
 #else
diff --git a/configure.ac b/configure.ac
index eb1bb3e6..bb3eeb60 100644
--- a/configure.ac
+++ b/configure.ac
@@ -427,6 +427,7 @@ AS_IF([test "x$with_sdt" = "xyes"], [
 	]])], [
 		AC_MSG_RESULT([yes])
 		AC_DEFINE([LTTNG_UST_HAVE_SDT_INTEGRATION], [1])
+		SDT_CFLAGS="-DSDT_USE_VARIADIC"
 	], [
 		AC_MSG_RESULT([no])
 		AC_MSG_ERROR([The sdt.h integration was requested but the STAP_PROBEV define cannot be used. Make sure it is installed, and up to date, or use CPPFLAGS=-I/path/ to specify a non-standard path to sys/sdt.h])
@@ -522,6 +523,7 @@ _AC_DEFINE_AND_SUBST([LTTNG_UST_DEFAULT_CONSTRUCTOR_TIMEOUT_MS], [3000])
 AM_CFLAGS="-Wall"
 AC_SUBST(AM_CFLAGS)
 AC_SUBST(JNI_CPPFLAGS)
+AC_SUBST(SDT_CFLAGS)
 
 AC_CONFIG_FILES([
 	Makefile
diff --git a/lttng-ust.pc.in b/lttng-ust.pc.in
index f9adde87..1b96db06 100644
--- a/lttng-ust.pc.in
+++ b/lttng-ust.pc.in
@@ -9,5 +9,5 @@ Version: @PACKAGE_VERSION@
 Requires:
 Requires.private: liburcu-bp
 Libs: -L${libdir} -llttng-ust -ldl
-Cflags: -I${includedir} 
+Cflags: -I${includedir} @SDT_CFLAGS@
 
-- 
2.26.2

[lttng-dev] [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Stefan Hajnoczi via lttng-dev]

LTTng Userspace Tracer 2.11 with sdt enabled can break builds of
applications that include <sys/sdt.h> themselves. QEMU
(https://qemu.org/) is one example.

When applications include <sys/sdt.h> themselves before LTTng Userspace
Tracer header files and the SDT_USE_VARIADIC macro is not defined,
compilation fails due to the absense of STAP_PROBEV() macro that LTTng
Userspace Tracer requires.

Define SDT_USE_VARIADIC in the pkg-config file so that all compilation
units that use LTTng Userspace Tracer are guaranteed to have the macro
defined even if <sys/sdt.h> is included elsewhere before LTTng Userspace
Tracer header files.

In other words, define SDT_USE_VARIADIC on the compiler command-line
instead of in <lttng/tracepoint.h>. This way it will always been defined
when <sys/sdt.h> is included.

Make sure to define SDT_USE_VARIADIC when <lttng/tracepoint.h> is
included without it. This happens when applications do not use
pkg-config.

Fixes: b2f60c7986bb69f81b79b68f1bfe83aafa3278a7 ("Add sdt.h integration")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Warning: this patch doesn't work yet. SDT_CFLAGS also needs to be passed
into LTTng Userspace Tracer's own CFLAGS so that compilation can
succeed. I don't know the autoconf/automake syntax to do this and wanted
to see what you think of this solution before investing more time.
---
 include/lttng/tracepoint.h | 2 ++
 configure.ac               | 2 ++
 lttng-ust.pc.in            | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 7cfae3e4..6b699373 100644
--- a/include/lttng/tracepoint.h
+++ b/include/lttng/tracepoint.h
@@ -35,7 +35,9 @@
 #include <lttng/ust-compiler.h>
 
 #ifdef LTTNG_UST_HAVE_SDT_INTEGRATION
+#ifndef SDT_USE_VARIADIC
 #define SDT_USE_VARIADIC
+#endif
 #include <sys/sdt.h>
 #define LTTNG_STAP_PROBEV STAP_PROBEV
 #else
diff --git a/configure.ac b/configure.ac
index eb1bb3e6..bb3eeb60 100644
--- a/configure.ac
+++ b/configure.ac
@@ -427,6 +427,7 @@ AS_IF([test "x$with_sdt" = "xyes"], [
 	]])], [
 		AC_MSG_RESULT([yes])
 		AC_DEFINE([LTTNG_UST_HAVE_SDT_INTEGRATION], [1])
+		SDT_CFLAGS="-DSDT_USE_VARIADIC"
 	], [
 		AC_MSG_RESULT([no])
 		AC_MSG_ERROR([The sdt.h integration was requested but the STAP_PROBEV define cannot be used. Make sure it is installed, and up to date, or use CPPFLAGS=-I/path/ to specify a non-standard path to sys/sdt.h])
@@ -522,6 +523,7 @@ _AC_DEFINE_AND_SUBST([LTTNG_UST_DEFAULT_CONSTRUCTOR_TIMEOUT_MS], [3000])
 AM_CFLAGS="-Wall"
 AC_SUBST(AM_CFLAGS)
 AC_SUBST(JNI_CPPFLAGS)
+AC_SUBST(SDT_CFLAGS)
 
 AC_CONFIG_FILES([
 	Makefile
diff --git a/lttng-ust.pc.in b/lttng-ust.pc.in
index f9adde87..1b96db06 100644
--- a/lttng-ust.pc.in
+++ b/lttng-ust.pc.in
@@ -9,5 +9,5 @@ Version: @PACKAGE_VERSION@
 Requires:
 Requires.private: liburcu-bp
 Libs: -L${libdir} -llttng-ust -ldl
-Cflags: -I${includedir} 
+Cflags: -I${includedir} @SDT_CFLAGS@
 
-- 
2.26.2

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Michael Jeanson via lttng-dev]

Hi Stefan,

I proposed a patch here[1] that addresses this problem in a generic way by
duplicating the macro and namespacing it under LTTNG_. Honestly this is more
of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
of _SYS_SDT_H or in it's own header file but we have to work with what's there.

Can you have a look at the patch and confirm that it fixes your problem?

Cheers,

Michael

[1] https://review.lttng.org/c/lttng-ust/+/3685 


----- On 24 Jun, 2020, at 12:09, lttng-dev lttng-dev@lists.lttng.org wrote:

> LTTng Userspace Tracer 2.11 with sdt enabled can break builds of
> applications that include <sys/sdt.h> themselves. QEMU
> (https://qemu.org/) is one example.
> 
> When applications include <sys/sdt.h> themselves before LTTng Userspace
> Tracer header files and the SDT_USE_VARIADIC macro is not defined,
> compilation fails due to the absense of STAP_PROBEV() macro that LTTng
> Userspace Tracer requires.
> 
> Define SDT_USE_VARIADIC in the pkg-config file so that all compilation
> units that use LTTng Userspace Tracer are guaranteed to have the macro
> defined even if <sys/sdt.h> is included elsewhere before LTTng Userspace
> Tracer header files.
> 
> In other words, define SDT_USE_VARIADIC on the compiler command-line
> instead of in <lttng/tracepoint.h>. This way it will always been defined
> when <sys/sdt.h> is included.
> 
> Make sure to define SDT_USE_VARIADIC when <lttng/tracepoint.h> is
> included without it. This happens when applications do not use
> pkg-config.
> 
> Fixes: b2f60c7986bb69f81b79b68f1bfe83aafa3278a7 ("Add sdt.h integration")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> Warning: this patch doesn't work yet. SDT_CFLAGS also needs to be passed
> into LTTng Userspace Tracer's own CFLAGS so that compilation can
> succeed. I don't know the autoconf/automake syntax to do this and wanted
> to see what you think of this solution before investing more time.

Re: [lttng-dev] [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Michael Jeanson via lttng-dev]

Hi Stefan,

I proposed a patch here[1] that addresses this problem in a generic way by
duplicating the macro and namespacing it under LTTNG_. Honestly this is more
of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
of _SYS_SDT_H or in it's own header file but we have to work with what's there.

Can you have a look at the patch and confirm that it fixes your problem?

Cheers,

Michael

[1] https://review.lttng.org/c/lttng-ust/+/3685 


----- On 24 Jun, 2020, at 12:09, lttng-dev lttng-dev@lists.lttng.org wrote:

> LTTng Userspace Tracer 2.11 with sdt enabled can break builds of
> applications that include <sys/sdt.h> themselves. QEMU
> (https://qemu.org/) is one example.
> 
> When applications include <sys/sdt.h> themselves before LTTng Userspace
> Tracer header files and the SDT_USE_VARIADIC macro is not defined,
> compilation fails due to the absense of STAP_PROBEV() macro that LTTng
> Userspace Tracer requires.
> 
> Define SDT_USE_VARIADIC in the pkg-config file so that all compilation
> units that use LTTng Userspace Tracer are guaranteed to have the macro
> defined even if <sys/sdt.h> is included elsewhere before LTTng Userspace
> Tracer header files.
> 
> In other words, define SDT_USE_VARIADIC on the compiler command-line
> instead of in <lttng/tracepoint.h>. This way it will always been defined
> when <sys/sdt.h> is included.
> 
> Make sure to define SDT_USE_VARIADIC when <lttng/tracepoint.h> is
> included without it. This happens when applications do not use
> pkg-config.
> 
> Fixes: b2f60c7986bb69f81b79b68f1bfe83aafa3278a7 ("Add sdt.h integration")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> Warning: this patch doesn't work yet. SDT_CFLAGS also needs to be passed
> into LTTng Userspace Tracer's own CFLAGS so that compilation can
> succeed. I don't know the autoconf/automake syntax to do this and wanted
> to see what you think of this solution before investing more time.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Stefan Hajnoczi via lttng-dev]

[-- Attachment #1.1: Type: text/plain, Size: 829 bytes --]

On Fri, Jun 26, 2020 at 05:02:00PM -0400, Michael Jeanson wrote:
> I proposed a patch here[1] that addresses this problem in a generic way by
> duplicating the macro and namespacing it under LTTNG_. Honestly this is more
> of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
> of _SYS_SDT_H or in it's own header file but we have to work with what's there.
> 
> Can you have a look at the patch and confirm that it fixes your problem?

Great, glad there is already a fix under review.

When I try out the patch the application does not compile. The error
messages are the same as without the patch because _LTTNG_SDT_PROBE() is
not defined. Was that supposed to be _SDT_PROBE() instead?

The application compiles successfully after replacing _LTTNG_SDT_PROBE()
with _SDT_PROBE().

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Stefan Hajnoczi via lttng-dev]

[-- Attachment #1.1: Type: text/plain, Size: 829 bytes --]

On Fri, Jun 26, 2020 at 05:02:00PM -0400, Michael Jeanson wrote:
> I proposed a patch here[1] that addresses this problem in a generic way by
> duplicating the macro and namespacing it under LTTNG_. Honestly this is more
> of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
> of _SYS_SDT_H or in it's own header file but we have to work with what's there.
> 
> Can you have a look at the patch and confirm that it fixes your problem?

Great, glad there is already a fix under review.

When I try out the patch the application does not compile. The error
messages are the same as without the patch because _LTTNG_SDT_PROBE() is
not defined. Was that supposed to be _SDT_PROBE() instead?

The application compiles successfully after replacing _LTTNG_SDT_PROBE()
with _SDT_PROBE().

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Michael Jeanson via lttng-dev]

----- On 29 Jun, 2020, at 12:26, Stefan Hajnoczi stefanha@redhat.com wrote:

> On Fri, Jun 26, 2020 at 05:02:00PM -0400, Michael Jeanson wrote:
>> I proposed a patch here[1] that addresses this problem in a generic way by
>> duplicating the macro and namespacing it under LTTNG_. Honestly this is more
>> of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
>> of _SYS_SDT_H or in it's own header file but we have to work with what's there.
>> 
>> Can you have a look at the patch and confirm that it fixes your problem?
> 
> Great, glad there is already a fix under review.
> 
> When I try out the patch the application does not compile. The error
> messages are the same as without the patch because _LTTNG_SDT_PROBE() is
> not defined. Was that supposed to be _SDT_PROBE() instead?
> 
> The application compiles successfully after replacing _LTTNG_SDT_PROBE()
> with _SDT_PROBE().
> 
> Stefan

Yes indeed, I reworked the patch after testing it and didn't catch this, I've
just pushed an updated patch. Thanks for testing it and catching this.

Cheers,

Michael

Re: [lttng-dev] [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Michael Jeanson via lttng-dev]

----- On 29 Jun, 2020, at 12:26, Stefan Hajnoczi stefanha@redhat.com wrote:

> On Fri, Jun 26, 2020 at 05:02:00PM -0400, Michael Jeanson wrote:
>> I proposed a patch here[1] that addresses this problem in a generic way by
>> duplicating the macro and namespacing it under LTTNG_. Honestly this is more
>> of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
>> of _SYS_SDT_H or in it's own header file but we have to work with what's there.
>> 
>> Can you have a look at the patch and confirm that it fixes your problem?
> 
> Great, glad there is already a fix under review.
> 
> When I try out the patch the application does not compile. The error
> messages are the same as without the patch because _LTTNG_SDT_PROBE() is
> not defined. Was that supposed to be _SDT_PROBE() instead?
> 
> The application compiles successfully after replacing _LTTNG_SDT_PROBE()
> with _SDT_PROBE().
> 
> Stefan

Yes indeed, I reworked the patch after testing it and didn't catch this, I've
just pushed an updated patch. Thanks for testing it and catching this.

Cheers,

Michael
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Stefan Hajnoczi via lttng-dev]

[-- Attachment #1.1: Type: text/plain, Size: 1242 bytes --]

On Mon, Jun 29, 2020 at 12:36:57PM -0400, Michael Jeanson wrote:
> ----- On 29 Jun, 2020, at 12:26, Stefan Hajnoczi stefanha@redhat.com wrote:
> 
> > On Fri, Jun 26, 2020 at 05:02:00PM -0400, Michael Jeanson wrote:
> >> I proposed a patch here[1] that addresses this problem in a generic way by
> >> duplicating the macro and namespacing it under LTTNG_. Honestly this is more
> >> of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
> >> of _SYS_SDT_H or in it's own header file but we have to work with what's there.
> >> 
> >> Can you have a look at the patch and confirm that it fixes your problem?
> > 
> > Great, glad there is already a fix under review.
> > 
> > When I try out the patch the application does not compile. The error
> > messages are the same as without the patch because _LTTNG_SDT_PROBE() is
> > not defined. Was that supposed to be _SDT_PROBE() instead?
> > 
> > The application compiles successfully after replacing _LTTNG_SDT_PROBE()
> > with _SDT_PROBE().
> > 
> > Stefan
> 
> Yes indeed, I reworked the patch after testing it and didn't catch this, I've
> just pushed an updated patch. Thanks for testing it and catching this.

Nice! Thanks for the fix.

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH] Fix: tracepoint.h: define SDT_USE_VARIADIC in pkg-config CFLAGS

[Stefan Hajnoczi via lttng-dev]

[-- Attachment #1.1: Type: text/plain, Size: 1242 bytes --]

On Mon, Jun 29, 2020 at 12:36:57PM -0400, Michael Jeanson wrote:
> ----- On 29 Jun, 2020, at 12:26, Stefan Hajnoczi stefanha@redhat.com wrote:
> 
> > On Fri, Jun 26, 2020 at 05:02:00PM -0400, Michael Jeanson wrote:
> >> I proposed a patch here[1] that addresses this problem in a generic way by
> >> duplicating the macro and namespacing it under LTTNG_. Honestly this is more
> >> of an sdt bug, the STAP_PROBEV should be in it's own include guard outside
> >> of _SYS_SDT_H or in it's own header file but we have to work with what's there.
> >> 
> >> Can you have a look at the patch and confirm that it fixes your problem?
> > 
> > Great, glad there is already a fix under review.
> > 
> > When I try out the patch the application does not compile. The error
> > messages are the same as without the patch because _LTTNG_SDT_PROBE() is
> > not defined. Was that supposed to be _SDT_PROBE() instead?
> > 
> > The application compiles successfully after replacing _LTTNG_SDT_PROBE()
> > with _SDT_PROBE().
> > 
> > Stefan
> 
> Yes indeed, I reworked the patch after testing it and didn't catch this, I've
> just pushed an updated patch. Thanks for testing it and catching this.

Nice! Thanks for the fix.

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev