diff mbox

[babeltrace] debug info: Call register_event_debug_infos for all events

Message ID 20160619151429.19053-1-simon.marchi@polymtl.ca
State Accepted, archived
Headers show

Commit Message

Simon Marchi June 19, 2016, 3:14 p.m. UTC
The register_event_debug_infos function is responsible for setting the
debug_info_src field of the struct definition_integer representing the
"ip" context field.  This pointer is used later to print the
source and binary location of the tracepoint.

Currently, it's not called for the events that have a special meaning
for the debug info analysis (statedump start, dlopen, bin_info, etc).
This means that for these events, the debug_info_src pointer keeps the
value of the previous event from that stream (since the ip field is in
stream event context, there is one instance per stream), which leads to
wrong information and/or crash.  A crash can happen when the debug info
structures are cleared because a statedump start is encountered.
debug_info_src becomes a stale pointer and babeltrace tries to print a
string in free memory.

The fix is to call register_event_debug_infos for all events, which will
always set debug_info_src properly.  The events used to do the debug
info analysis are still regular events and have their own location
information.

The trace in bug #1018 shows a crash caused by this issue.

Here's an example of wrong info from the same trace:

... lttng_ust_statedump:bin_info: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD26665657D, debug_info = { bin = "libX11.so.6.3.0+0x213fa" } }, ...
... lttng_ust_statedump:build_id: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD266656656, debug_info = { bin = "libX11.so.6.3.0+0x213fa" } }, ...

We can see that the ip values are different, but the debug info refers
to the same location, which is impossible.  Here's the result with this
patch applied.

... lttng_ust_statedump:bin_info: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD26665657D, debug_info = { bin = "liblttng-ust.so.0.0.0+0x3557d", func = "trace_bin_info_cb+0xfa" } }, ...
... lttng_ust_statedump:build_id: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD266656656, debug_info = { bin = "liblttng-ust.so.0.0.0+0x35656", func = "trace_build_id_cb+0xbc" } }, ...

which seems more reasonnable.

Signed-off-by: Simon Marchi <simon.marchi at polymtl.ca>
---
 lib/debug-info.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jérémie Galarneau July 25, 2016, 9:30 p.m. UTC | #1
Merged as part of master and stable-1.4.

Thanks!
Jérémie

On Sun, Jun 19, 2016 at 11:14 AM, Simon Marchi <simon.marchi at polymtl.ca> wrote:
> The register_event_debug_infos function is responsible for setting the
> debug_info_src field of the struct definition_integer representing the
> "ip" context field.  This pointer is used later to print the
> source and binary location of the tracepoint.
>
> Currently, it's not called for the events that have a special meaning
> for the debug info analysis (statedump start, dlopen, bin_info, etc).
> This means that for these events, the debug_info_src pointer keeps the
> value of the previous event from that stream (since the ip field is in
> stream event context, there is one instance per stream), which leads to
> wrong information and/or crash.  A crash can happen when the debug info
> structures are cleared because a statedump start is encountered.
> debug_info_src becomes a stale pointer and babeltrace tries to print a
> string in free memory.
>
> The fix is to call register_event_debug_infos for all events, which will
> always set debug_info_src properly.  The events used to do the debug
> info analysis are still regular events and have their own location
> information.
>
> The trace in bug #1018 shows a crash caused by this issue.
>
> Here's an example of wrong info from the same trace:
>
> ... lttng_ust_statedump:bin_info: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD26665657D, debug_info = { bin = "libX11.so.6.3.0+0x213fa" } }, ...
> ... lttng_ust_statedump:build_id: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD266656656, debug_info = { bin = "libX11.so.6.3.0+0x213fa" } }, ...
>
> We can see that the ip values are different, but the debug info refers
> to the same location, which is impossible.  Here's the result with this
> patch applied.
>
> ... lttng_ust_statedump:bin_info: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD26665657D, debug_info = { bin = "liblttng-ust.so.0.0.0+0x3557d", func = "trace_bin_info_cb+0xfa" } }, ...
> ... lttng_ust_statedump:build_id: { cpu_id = 1 }, { vpid = 28991, ip = 0x7FD266656656, debug_info = { bin = "liblttng-ust.so.0.0.0+0x35656", func = "trace_build_id_cb+0xbc" } }, ...
>
> which seems more reasonnable.
>
> Signed-off-by: Simon Marchi <simon.marchi at polymtl.ca>
> ---
>  lib/debug-info.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/debug-info.c b/lib/debug-info.c
> index aa19ce4..02ae582 100644
> --- a/lib/debug-info.c
> +++ b/lib/debug-info.c
> @@ -795,11 +795,11 @@ void debug_info_handle_event(struct debug_info *debug_info,
>         } else if (event_class->name == debug_info->q_statedump_build_id) {
>                 /* Build ID info */
>                 handle_statedump_build_id_event(debug_info, event);
> -       } else {
> -               /* Other events: register debug infos */
> -               register_event_debug_infos(debug_info, event);
>         }
>
> +       /* All events: register debug infos */
> +       register_event_debug_infos(debug_info, event);
> +
>  end:
>         return;
>  }
> --
> 2.8.3
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
diff mbox

Patch

diff --git a/lib/debug-info.c b/lib/debug-info.c
index aa19ce4..02ae582 100644
--- a/lib/debug-info.c
+++ b/lib/debug-info.c
@@ -795,11 +795,11 @@  void debug_info_handle_event(struct debug_info *debug_info,
 	} else if (event_class->name == debug_info->q_statedump_build_id) {
 		/* Build ID info */
 		handle_statedump_build_id_event(debug_info, event);
-	} else {
-		/* Other events: register debug infos */
-		register_event_debug_infos(debug_info, event);
 	}
 
+	/* All events: register debug infos */
+	register_event_debug_infos(debug_info, event);
+
 end:
 	return;
 }