Fix: src.ctf.lttng-live: using `last_inactivity_ts` uninitialized

Observed issue
==============
I saw the following error message in the logs of a `lttng clear` test case
crash:

  bt_clock_class_cycles_to_ns_from_origin@clock-class.c:311 Cannot convert cycles to nanoseconds from origin for given clock class: value overflows the signed 64-bit integer range:
    cc-addr=0x55f5b33280f0, cc-name="monotonic", cc-freq=1000000000,
    cc-partial-descr="Monotonic Clock",
    cc-uuid="c812f5e1-ceda-4f13-8b99-23a5be8a2908",
    cc-is-frozen=0, cc-precision=0, cc-offset-s=1634653310,
    cc-offset-cycles=492621904, cc-origin-is-unix-epoch=1,
    cc-base-offset-ns=1634653310492621904, cc-cs-pool-size=6, cc-cs-pool-cap=15,
    cycles=9223372036854775808

Take note that last line of this log statement shows a `cycles` value of
9223372036854775808. This value is `INT64_MIN` stored on a 64bits
unsigned integer.

Cause
=====
This error occurs because the
`lttng_live_stream_iterator::last_inactivity_ts` field is used without
being initialized to an actual value.

This field is a `uint64_t` and is initialized to `INT64_MIN` with the
intent of setting it to the minimum value and used as is in that error
statement.

Fix
===
Change the `last_inactivity_ts` field to an anonymous struct that
contains an `is_set` boolean and the value. Use the `is_set` flag to
ensure we only use this field once initialized.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I24ba0dd9267508b2c9f0de99386a16b1ce022329
Reviewed-on: https://review.lttng.org/c/babeltrace/+/6677
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/src/plugins/ctf/lttng-live/data-stream.c b/src/plugins/ctf/lttng-live/data-stream.c
index 618af36d643910715ff5ad77ed461e556cb39701..d7fdf34635cdc5bcbe6ce68889ca88bc131541f2 100644 (file)
--- a/src/plugins/ctf/lttng-live/data-stream.c
+++ b/src/plugins/ctf/lttng-live/data-stream.c
@@ -223,7 +223,8 @@ struct lttng_live_stream_iterator *lttng_live_stream_iterator_create(
        stream_iter->state = LTTNG_LIVE_STREAM_ACTIVE_NO_DATA;
        stream_iter->viewer_stream_id = stream_id;
        stream_iter->ctf_stream_class_id = -1ULL;
-       stream_iter->last_inactivity_ts = INT64_MIN;
+       stream_iter->last_inactivity_ts.is_set = false;
+       stream_iter->last_inactivity_ts.value = 0;
 
        if (trace->trace) {
                struct ctf_trace_class *ctf_tc =

diff --git a/src/plugins/ctf/lttng-live/lttng-live.c b/src/plugins/ctf/lttng-live/lttng-live.c
index b0833061dfad1ffc0cb68b2ef3674a67a92e5059..2eafb2a6be2d5dedbb9c7f0ea3410ab770774c5f 100644 (file)
--- a/src/plugins/ctf/lttng-live/lttng-live.c
+++ b/src/plugins/ctf/lttng-live/lttng-live.c
@@ -103,10 +103,12 @@ void lttng_live_stream_iterator_set_state(struct lttng_live_stream_iterator *str
 
 #define LTTNG_LIVE_LOGD_STREAM_ITER(live_stream_iter) \
        do { \
-               BT_COMP_LOGD("Live stream iterator state=%s, last-inact-ts=%" PRId64  \
-                       ", curr-inact-ts %" PRId64, \
+               BT_COMP_LOGD("Live stream iterator state=%s, " \
+                       "last-inact-ts-is-set=%d, last-inact-ts-value=%" PRId64 "), " \
+                       "curr-inact-ts %" PRId64, \
                        lttng_live_stream_state_string(live_stream_iter->state), \
-                       live_stream_iter->last_inactivity_ts, \
+                       live_stream_iter->last_inactivity_ts.is_set, \
+                       live_stream_iter->last_inactivity_ts.value, \
                        live_stream_iter->current_inactivity_ts); \
        } while (0);
 
@@ -418,11 +420,18 @@ enum lttng_live_iterator_status lttng_live_iterator_next_handle_one_no_data_stre
        BT_ASSERT_DBG(lttng_live_stream->state != LTTNG_LIVE_STREAM_EOF);
 
        if (lttng_live_stream->state == LTTNG_LIVE_STREAM_QUIESCENT) {
-               uint64_t last_inact_ts = lttng_live_stream->last_inactivity_ts,
+               uint64_t last_inact_ts = lttng_live_stream->last_inactivity_ts.value,
                         curr_inact_ts = lttng_live_stream->current_inactivity_ts;
 
                if (orig_state == LTTNG_LIVE_STREAM_QUIESCENT_NO_DATA &&
                                last_inact_ts == curr_inact_ts) {
+                       /*
+                        * Because the stream is in the QUIESCENT_NO_DATA
+                        * state, we can assert that the last_inactivity_ts was
+                        * set and can be safely used in the `if` above.
+                        */
+                       BT_ASSERT(lttng_live_stream->last_inactivity_ts.is_set);
+
                        ret = LTTNG_LIVE_ITERATOR_STATUS_AGAIN;
                        LTTNG_LIVE_LOGD_STREAM_ITER(lttng_live_stream);
                } else {
@@ -700,8 +709,9 @@ enum lttng_live_iterator_status lttng_live_iterator_next_handle_one_quiescent_st
         * Check if we already sent an inactivty message downstream for this
         * `current_inactivity_ts` value.
         */
-       if (lttng_live_stream->current_inactivity_ts ==
-                       lttng_live_stream->last_inactivity_ts) {
+       if (lttng_live_stream->last_inactivity_ts.is_set &&
+                       lttng_live_stream->current_inactivity_ts ==
+                               lttng_live_stream->last_inactivity_ts.value) {
                lttng_live_stream_iterator_set_state(lttng_live_stream,
                        LTTNG_LIVE_STREAM_QUIESCENT_NO_DATA);
 
@@ -712,8 +722,9 @@ enum lttng_live_iterator_status lttng_live_iterator_next_handle_one_quiescent_st
        ret = emit_inactivity_message(lttng_live_msg_iter, lttng_live_stream,
                message, lttng_live_stream->current_inactivity_ts);
 
-       lttng_live_stream->last_inactivity_ts =
+       lttng_live_stream->last_inactivity_ts.value =
                lttng_live_stream->current_inactivity_ts;
+       lttng_live_stream->last_inactivity_ts.is_set = true;
 end:
        return ret;
 }
@@ -1152,7 +1163,7 @@ enum lttng_live_iterator_status handle_late_message(
        clock_class = bt_stream_class_borrow_default_clock_class_const(stream_class);
 
        ts_ns_status = bt_clock_class_cycles_to_ns_from_origin(clock_class,
-                       stream_iter->last_inactivity_ts, &last_inactivity_ts_ns);
+                       stream_iter->last_inactivity_ts.value, &last_inactivity_ts_ns);
        if (ts_ns_status != BT_CLOCK_CLASS_CYCLES_TO_NS_FROM_ORIGIN_STATUS_OK) {
                stream_iter_status = LTTNG_LIVE_ITERATOR_STATUS_ERROR;
                goto end;
@@ -1185,14 +1196,14 @@ enum lttng_live_iterator_status handle_late_message(
                                        lttng_live_msg_iter->self_msg_iter,
                                        stream_iter->stream,
                                        late_msg, &adjusted_message,
-                                       stream_iter->last_inactivity_ts);
+                                       stream_iter->last_inactivity_ts.value);
                        break;
                case BT_MESSAGE_TYPE_DISCARDED_PACKETS:
                        adjust_status = adjust_discarded_packets_message(
                                        lttng_live_msg_iter->self_msg_iter,
                                        stream_iter->stream,
                                        late_msg, &adjusted_message,
-                                       stream_iter->last_inactivity_ts);
+                                       stream_iter->last_inactivity_ts.value);
                        break;
                default:
                        bt_common_abort();

diff --git a/src/plugins/ctf/lttng-live/lttng-live.h b/src/plugins/ctf/lttng-live/lttng-live.h
index 82f39a0fabb4baf115135a60a2f21f040c9b6f3a..be235a7d4b04c633f086babe88833c369ee53060 100644 (file)
--- a/src/plugins/ctf/lttng-live/lttng-live.h
+++ b/src/plugins/ctf/lttng-live/lttng-live.h
@@ -78,7 +78,10 @@ struct lttng_live_stream_iterator {
         * Clock Snapshot value of the last message iterator inactivity message
         * sent downstream.
         */
-       uint64_t last_inactivity_ts;
+       struct {
+               bool is_set;
+               uint64_t value;
+       } last_inactivity_ts;
 
        /*
         * Clock Snapshot value of the current message iterator inactivity