--with-babeltrace generates many FAILs
Commit Message
On 08/19/2014 10:07 PM, Jan Kratochvil wrote:
> * '#if HAVE_LIBBABELTRACE1_1_0' could have a comment that >=1.1.1 rejects the
> faked packet (what you described in the mail but not in the patch).
Fixed. To be precise, >= 1.1.2 rejects the faked packet, 1.1.1
doesn't. See the table I posted.
> * It is always better to check for feature/defect than to check for version.
> For example because various distros backport various fixes (unfortunately
> including their possible regressions/defects) and so version checks may be
> misleading then. At least in this case it seems to me as possible to check
> how libbacktrace behaves from configure; although maybe it is not easy
> enough, not sure.
In order to check libbabeltrace's behaviour in configure, we have to write
a c program to generate CTF data and read the trace data via
babeltrace or any program (using libbabeltrace) written by ourselves.
It is not easy to do so.
The patch is updated. OK to apply?
Comments
On 08/20/2014 05:02 AM, Yao Qi wrote:
> The patch is updated. OK to apply?
I'm wondering whether we really need all this complication for an old
version of a library that is quite new-ish, and not relied on for
anything really core? I'm wondering whether this check works on
Windows, for example.
As there's been fixed babeltrace versions for a while, I'd go with
simply dropping the workaround, and have integrators build newer
GDB with newer babeltrace. I suppose we have a testcase in our
testsuite that fails if we remove the workaround and GDB is built with
broken babeltrace? That should let the integrator know that it's
building again a broken lib.
IOW, why do we still need to support 1.1.0?
Thanks,
Pedro Alves
On 08/20/2014 05:41 PM, Pedro Alves wrote:
> As there's been fixed babeltrace versions for a while, I'd go with
> simply dropping the workaround, and have integrators build newer
> GDB with newer babeltrace. I suppose we have a testcase in our
> testsuite that fails if we remove the workaround and GDB is built with
> broken babeltrace? That should let the integrator know that it's
> building again a broken lib.
Yes, we have such test case, such as actions.exp, at least. Without
the workaround, GDB with libbabeltrace 1.1.0 will fail in actions.exp.
However, is it a good idea that let test failure signal a wrong version
lib is used? I am not sure. It is the configure's job to check whether
the library is wrong or broken.
>
> IOW, why do we still need to support 1.1.0?
No special reason, 1.1.0 was just used when I did the CTF work in GDB,
and was used on my laptop since then. IIRC, 1.1.0 was released in 2013
March, so it isn't very old and it might be used somewhere. Shouldn't
we be conservative in this case?
In general, GDB and GDBserver uses a set of libraries, what are the
criteria of
1. stop supporting a version of a library, such as libbabeltrace 1.1.0
2. stop supporting or using a library, such as the UST stuff in GDBserver,
On 08/20/2014 12:34 PM, Yao Qi wrote:
> On 08/20/2014 05:41 PM, Pedro Alves wrote:
>> As there's been fixed babeltrace versions for a while, I'd go with
>> simply dropping the workaround, and have integrators build newer
>> GDB with newer babeltrace. I suppose we have a testcase in our
>> testsuite that fails if we remove the workaround and GDB is built with
>> broken babeltrace? That should let the integrator know that it's
>> building again a broken lib.
>
> Yes, we have such test case, such as actions.exp, at least. Without
> the workaround, GDB with libbabeltrace 1.1.0 will fail in actions.exp.
> However, is it a good idea that let test failure signal a wrong version
> lib is used?
IMO, in this case, it's sufficient. There are point releases that fix
the issue, which are supposedly compatible with 1.1.0 -- updating those
is just a normal maintenance issue, doesn't involve any sort of
porting in gdb, etc.
> I am not sure. It is the configure's job to check whether
> the library is wrong or broken.
Note that there's a fundamental issue with the workaround -- it
assumes that the gdb that generates CTF is the same that consumes it.
It's certainly easy to imagine a CTF file generated by a gdb not
affected by the bug be consumed by a gdb with the bug. Or the other way
around.
>> IOW, why do we still need to support 1.1.0?
>
> No special reason, 1.1.0 was just used when I did the CTF work in GDB,
> and was used on my laptop since then. IIRC, 1.1.0 was released in 2013
> March, so it isn't very old and it might be used somewhere.
> Shouldn't we be conservative in this case?
My point is exactly that this is new-ish code, and a moving target.
If bugs are fixed promptly, why go through trouble working around
them in gdb instead of just updating the version in the distro?
It'd be different if we were talking about a one liner instead of
a good chunk of autoconf/pkg-config glue.
I'm not strictly objecting your patch, but it does look like
the kind of code that: #1 will need further tweaking in the future;
#2 will become obsolete anyway as time passes anyway. Couple that
with the generator != consumer issue, and it raises eyebrows.
> In general, GDB and GDBserver uses a set of libraries, what are the
> criteria of
>
> 1. stop supporting a version of a library, such as libbabeltrace 1.1.0
When the burden of supporting it outweighs the benefits?
> 2. stop supporting or using a library, such as the UST stuff in GDBserver,
When nobody wants to maintain it anymore (I personally don't)?
Thanks,
Pedro Alves
@@ -183,6 +183,9 @@
/* Define if you have the babeltrace library. */
#undef HAVE_LIBBABELTRACE
+/* Define to 1 if you have libbabeltrace 1.1.0 */
+#undef HAVE_LIBBABELTRACE1_1_0
+
/* Define to 1 if you have the `dl' library (-ldl). */
#undef HAVE_LIBDL
@@ -14817,6 +14817,11 @@ $as_echo "$with_babeltrace" >&6; }
if test "x$with_babeltrace" = "xno"; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: babletrace support disabled; GDB is unable to read CTF data." >&5
$as_echo "$as_me: WARNING: babletrace support disabled; GDB is unable to read CTF data." >&2;}
+elif test "${pkg_config_prog_path}" = "missing"; then
+ # pkg-config is used to check the version of libbabeltrace. If pkg-config
+ # is missing, we have to disable babeltrace support.
+ { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: pkg-config not found, babletrace support disabled" >&5
+$as_echo "$as_me: WARNING: pkg-config not found, babletrace support disabled" >&2;}
else
# Append -Werror to CFLAGS so that configure can catch the warning
# "assignment from incompatible pointer type", which is related to
@@ -15307,6 +15312,26 @@ $as_echo "$LIBBABELTRACE" >&6; }
{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: babeltrace is missing or unusable; GDB is unable to read CTF data." >&5
$as_echo "$as_me: WARNING: babeltrace is missing or unusable; GDB is unable to read CTF data." >&2;}
fi
+ else
+ # Need to know whether libbabeltrace is 1.1.0.
+ pkg_config_path=
+ for x in $LTLIBBABELTRACE; do
+ case "$x" in
+ -L*)
+ dir=`echo "X$x" | sed -e 's/^X-L//'`
+ if test -d "$dir/pkgconfig"; then
+ pkg_config_path="${pkg_config_path}${pkg_config_path:+:}$dir/pkgconfig"
+ fi
+ ;;
+ esac
+ done
+
+ `PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$pkg_config_path ${pkg_config_prog_path} babeltrace = 1.1.0`
+ if test "$?" -eq 0 ; then
+
+$as_echo "#define HAVE_LIBBABELTRACE1_1_0 1" >>confdefs.h
+
+ fi
fi
fi
@@ -2420,6 +2420,10 @@ AC_MSG_RESULT([$with_babeltrace])
if test "x$with_babeltrace" = "xno"; then
AC_MSG_WARN([babletrace support disabled; GDB is unable to read CTF data.])
+elif test "${pkg_config_prog_path}" = "missing"; then
+ # pkg-config is used to check the version of libbabeltrace. If pkg-config
+ # is missing, we have to disable babeltrace support.
+ AC_MSG_WARN([pkg-config not found, babletrace support disabled])
else
# Append -Werror to CFLAGS so that configure can catch the warning
# "assignment from incompatible pointer type", which is related to
@@ -2450,6 +2454,24 @@ else
else
AC_MSG_WARN([babeltrace is missing or unusable; GDB is unable to read CTF data.])
fi
+ else
+ # Need to know whether libbabeltrace is 1.1.0.
+ pkg_config_path=
+ for x in $LTLIBBABELTRACE; do
+ case "$x" in
+ -L*)
+ dir=`echo "X$x" | sed -e 's/^X-L//'`
+ if test -d "$dir/pkgconfig"; then
+ pkg_config_path="${pkg_config_path}${pkg_config_path:+:}$dir/pkgconfig"
+ fi
+ ;;
+ esac
+ done
+
+ `PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$pkg_config_path ${pkg_config_prog_path} babeltrace = 1.1.0`
+ if test "$?" -eq 0 ; then
+ AC_DEFINE([HAVE_LIBBABELTRACE1_1_0], [1], [Define to 1 if you have libbabeltrace 1.1.0])
+ fi
fi
fi
@@ -623,11 +623,6 @@ ctf_write_definition_end (struct trace_file_writer *self)
self->ops->frame_ops->end (self);
}
-/* The minimal file size of data stream. It is required by
- babeltrace. */
-
-#define CTF_FILE_MIN_SIZE 4096
-
/* This is the implementation of trace_file_write_ops method
end. */
@@ -637,10 +632,21 @@ ctf_end (struct trace_file_writer *self)
struct ctf_trace_file_writer *writer = (struct ctf_trace_file_writer *) self;
gdb_assert (writer->tcs.content_size == 0);
- /* The babeltrace requires or assumes that the size of datastream
- file is greater than 4096 bytes. If we don't generate enough
- packets and events, create a fake packet which has zero event,
- to use up the space. */
+
+#if HAVE_LIBBABELTRACE1_1_0
+ /* The babeltrace-1.1.0 requires or assumes that the size of datastream
+ file is greater than 4096 bytes. This was fixed after 1.1.0 release.
+ See https://bugs.lttng.org/issues/450
+ If we don't generate enough packets and events, create a fake packet
+ which has zero event, to use up the space. However, babeltrace
+ release (since 1.1.2) starts to complain about such faked packet,
+ we include this workaround only for babeltrace 1.1.0. */
+
+ /* The minimal file size of data stream. It is required by
+ babeltrace. */
+
+#define CTF_FILE_MIN_SIZE 4096
+
if (writer->tcs.packet_start < CTF_FILE_MIN_SIZE)
{
uint32_t u32;
@@ -681,6 +687,7 @@ ctf_end (struct trace_file_writer *self)
ctf_save_write (&writer->tcs, &b, 1);
}
}
+#endif /* HAVE_LIBBABELTRACE1_1_0 */
}
/* This is the implementation of trace_frame_write_ops method