Trouble pinpointing source of uninitialized value-CodePudding

I am writing a simple earthquake detection program for the BeagleBone Green using a vibration sensor that I have hooked up, together with a cape that has a built in accelerometer and digit display. The brains of the earthquake detector are located in the EarthquakeDetector class.

earthquake_detector.h:

#pragma once
#include <atomic>
#include <thread>

#include <Accelerometer/accelerometer.h>
#include <DigitDisplay/digit_display.h>
#include <VibrationSensor/vibration_sensor.h>

namespace earthquake_detection_unit {

class EarthquakeDetector {
public:
    EarthquakeDetector();
    ~EarthquakeDetector();

private:
    // Worker thread for continuous earthquake monitoring.
    void Worker();

    // Monitors accelerometer readings. This function returns after a certain 
    // number of consecutive low readings.
    void AccelerometerMonitor();

    // Displays current magnitude on the digit display.
    void DisplayMagnitude();

    // Worker thread member variable.
    std::thread worker_thread;
    // Signal to shutdown worker thread.
    std::atomic<bool> shutdown;

    // Pointers to devices.
    Accelerometer *accelerometer;
    DigitDisplay *digit_display;
    VibrationSensor *vibration_sensor;
};

} // earthquake_detection_unit

earthquake_detector.cc:

#include <iostream>

#include "earthquake_detector.h"

namespace earthquake_detection_unit {

const double kScaleValue1Threshold = 0.144;
const double kScaleValue2Threshold = 0.281;
const double kScaleValue3Threshold = 0.418;
const double kScaleValue4Threshold = 0.555;
const double kScaleValue5Threshold = 0.692;
const double kScaleValue6Threshold = 0.829;
const double kScaleValue7Threshold = 0.966;
const double kScaleValue8Threshold = 1.103;
const double kScaleValue9Threshold = 1.24;

const int kAccelerometerTimeoutTotal_ms = 10000;
const int kAccelerometerSamplePeriod_ms = 100;
const int kAccelerometerTimeoutNumPeriods = kAccelerometerTimeoutTotal_ms / kAccelerometerSamplePeriod_ms;

EarthquakeDetector::EarthquakeDetector() : shutdown(false) {
    // Initialize digit display.
    digit_display = new DigitDisplay();

    worker_thread = std::thread(&EarthquakeDetector::Worker, this);
}

EarthquakeDetector::~EarthquakeDetector() {
    shutdown.store(true, std::memory_order_relaxed);
    worker_thread.join();

    // Shutdown digit display.
    delete digit_display;
}

void EarthquakeDetector::Worker() {
    while (!shutdown) {
        std::cout << "\t<EarthquakeDetector> ";
        std::cout << "Launching vibration sensor to listen for a vibration." << std::endl;
        // First, wait for a vibration.
        vibration_sensor = new VibrationSensor();
        vibration_sensor->WaitForVibration();
        delete vibration_sensor;

        // After detecting a vibration, launch accelerometer.
        std::cout << "\t<EarthquakeDetector> ";
        std::cout << "Vibration detected -- vibration sensor shutdown, launching accelerometer." << std::endl;
        accelerometer = new Accelerometer();

        // Monitor accelerometer readings.
        AccelerometerMonitor();

        // Flash magnitude.
        digit_display->FlashDisplay();
        // Reset digit display to display 0.
        digit_display->SetDigit(0);

        // Shutdown accelerometer for lack of activity.
        std::cout << "\t<EarthquakeDetector> ";
        std::cout << "Shutting down accelerometer due to inactivity." << std::endl;
        delete accelerometer;
    }
}

void EarthquakeDetector::AccelerometerMonitor() {
    // This function returns if we obtain kAccelerometerTimeoutNumPeriods
    // consecutive low readings from the accelerometer.
    int consecutive_readings = 0;
    while (consecutive_readings != kAccelerometerTimeoutNumPeriods) {
        DisplayMagnitude();
        if (accelerometer->GetCurrentReading() >= kScaleValue1Threshold) {
            consecutive_readings = 0;
        }
        else {
              consecutive_readings;
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(kAccelerometerSamplePeriod_ms));
    }
}

void EarthquakeDetector::DisplayMagnitude() {
    double acc_reading = accelerometer->GetHighestReading();

    unsigned int digit_to_display = 0;
    if (acc_reading >= kScaleValue9Threshold) {
        digit_to_display = 9;
    }
    else if (acc_reading >= kScaleValue8Threshold) {
        digit_to_display = 8;
    }
    else if (acc_reading >= kScaleValue7Threshold) {
        digit_to_display = 7;
    }
    else if (acc_reading >= kScaleValue6Threshold) {
        digit_to_display = 6;
    }
    else if (acc_reading >= kScaleValue5Threshold) {
        digit_to_display = 5;
    }
    else if (acc_reading >= kScaleValue4Threshold) {
        digit_to_display = 4;
    }
    else if (acc_reading >= kScaleValue3Threshold) {
        digit_to_display = 3;
    }
    else if (acc_reading >= kScaleValue2Threshold) {
        digit_to_display = 2;
    }
    else if (acc_reading >= kScaleValue1Threshold) {
        digit_to_display = 1;
    }

    digit_display->SetDigit(digit_to_display);
}

} // earthquake_detection_unit

Problem: Valgrind complains about an unitialized unsigned int (I used the --track-origins=yes option) and I can't find where I could possibly have an unitialized value in my implementation. I get the following error several times, presumably because as the code is being executed, it's falling through the line of if-statements in DisplayMagnitude.

==1023== Thread 2:
==1023== Conditional jump or move depends on uninitialised value(s)
==1023==    at 0x10A2DA: earthquake_detection_unit::EarthquakeDetector::DisplayMagnitude() (earthquake_detector.cc:85)
==1023==    by 0x10A227: earthquake_detection_unit::EarthquakeDetector::AccelerometerMonitor() (earthquake_detector.cc:70)
==1023==    by 0x10A15B: earthquake_detection_unit::EarthquakeDetector::Worker() (earthquake_detector.cc:51)
==1023==    by 0x10B705: void std::__invoke_impl<void, void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*>(std::__invoke_memfun_deref, void (earthquake_detection_unit::EarthquakeDetector::*&&)(), earthquake_detection_unit::EarthquakeDetector*&&) (invoke.h:73)
==1023==    by 0x10B633: std::__invoke_result<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*>::type std::__invoke<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*>(void (earthquake_detection_unit::EarthquakeDetector::*&&)(), earthquake_detection_unit::EarthquakeDetector*&&) (invoke.h:95)
==1023==    by 0x10B5A1: void std::thread::_Invoker<std::tuple<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*> >::_M_invoke<0u, 1u>(std::_Index_tuple<0u, 1u>) (thread:244)
==1023==    by 0x10B53F: std::thread::_Invoker<std::tuple<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*> >::operator()() (thread:251)
==1023==    by 0x10B515: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*> > >::_M_run() (thread:195)
==1023==    by 0x48FAA69: ??? (in /usr/lib/arm-linux-gnueabihf/libstdc  .so.6.0.22)
==1023==  Uninitialised value was created by a heap allocation
==1023==    at 0x4840194: operator new(unsigned int) (vg_replace_malloc.c:415)
==1023==    by 0x10A147: earthquake_detection_unit::EarthquakeDetector::Worker() (earthquake_detector.cc:48)
==1023==    by 0x10B705: void std::__invoke_impl<void, void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*>(std::__invoke_memfun_deref, void (earthquake_detection_unit::EarthquakeDetector::*&&)(), earthquake_detection_unit::EarthquakeDetector*&&) (invoke.h:73)
==1023==    by 0x10B633: std::__invoke_result<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*>::type std::__invoke<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*>(void (earthquake_detection_unit::EarthquakeDetector::*&&)(), earthquake_detection_unit::EarthquakeDetector*&&) (invoke.h:95)
==1023==    by 0x10B5A1: void std::thread::_Invoker<std::tuple<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*> >::_M_invoke<0u, 1u>(std::_Index_tuple<0u, 1u>) (thread:244)
==1023==    by 0x10B53F: std::thread::_Invoker<std::tuple<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*> >::operator()() (thread:251)
==1023==    by 0x10B515: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (earthquake_detection_unit::EarthquakeDetector::*)(), earthquake_detection_unit::EarthquakeDetector*> > >::_M_run() (thread:195)
==1023==    by 0x48FAA69: ??? (in /usr/lib/arm-linux-gnueabihf/libstdc  .so.6.0.22)

I'm wondering if someone could point me in the right direction of what could possibly be causing these errors. I have looked at various SO posts on similar issues but nothing really seemed to apply to my situation. Thanks!

Edit 1: Stripped down version of the accelerometer interface. I edited out a lot of the I2C file reading and writing to make this as brief as possible.

accelerometer.h:

namespace earthquake_detection_unit {

class Accelerometer {
public:
    typedef struct Vector {
        Vector(int16_t x_reading, int16_t y_reading, int16_t z_reading);

        double x;
        double y;
        double z;
        double magnitude;
    } Vector;

    Accelerometer();
    ~Accelerometer();

    inline double GetCurrentReading() { return current_reading; }
    inline double GetHighestReading() { return highest_reading; }

private:
    // Worker thread for sampling accelerometer readings.
    void Worker();

    void CollectReading();

    void ActivateAccelerometer();
    void ShutDownAccelerometer();

    // Signal to shutdown worker thread.
    std::atomic<bool> shutdown;
    // Worker thread.
    std::thread worker_thread;
    // Exponentially smoothed accelerometer magnitude reading.
    std::atomic<double> current_reading;
    // Highest reading detected by accelerometer.
    std::atomic<double> highest_reading;
};

} // earthquake_detection_unit

accelerometer.cc:

namespace earthquake_detection_unit {

Accelerometer::Accelerometer() {
    std::atomic<bool> shutdown(false);
    std::atomic<double> current_reading(0.0f);
    std::atomic<double> highest_reading(0.0f);

    // Activate accelerometer using I2C.
    ActivateAccelerometer();

    worker_thread = std::thread(&Accelerometer::Worker, this);
}

Accelerometer::~Accelerometer() {
    shutdown = true;
    worker_thread.join();
    // Shut down accelerometer via I2C.
    ShutDownAccelerometer();
}

Accelerometer::Vector::Vector(int16_t x_reading, int16_t y_reading, int16_t z_reading) {
    // Omitted: constructs Vector object containing accelerations and magnitude.
}

void Accelerometer::Worker() {
    while (!shutdown) {
        CollectReading();
        if (current_reading > highest_reading) {
            highest_reading.store(current_reading, std::memory_order_relaxed);
        }

        // Gather samples every 2 ms.
        std::this_thread::sleep_for(std::chrono::milliseconds(kSleepTime_ms));
    }
}

void Accelerometer::CollectReading() {
    // Omitted: collect accelerometer sample and store it in
    //          latest_vector_reading.
    Vector latest_vector_reading(x_reading, y_reading, z_reading);

    double latest_magnitude_reading = latest_vector_reading.magnitude;

    current_reading.store(kSmoothingFactor * latest_magnitude_reading  
                          (1.0f - kSmoothingFactor) * current_reading,
                          std::memory_order_relaxed);
}

} // earthquake_detection_unit

CodePudding user response：

Unless I'm mistaken your problem is your constructor

Accelerometer::Accelerometer() {
    std::atomic<bool> shutdown(false);
    std::atomic<double> current_reading(0.0f);
    std::atomic<double> highest_reading(0.0f);

    // Activate accelerometer using I2C.
    ActivateAccelerometer();

    worker_thread = std::thread(&Accelerometer::Worker, this);
}

In the above highest_reading is a local variable that hides the variable of the same name at class scope. You are initializing this local variable and leaving the class member uninitialized.

Try something like

Accelerometer::Accelerometer() : shutdown(false), current_reading(0.0), highest_reading(0.0) {
    // Activate accelerometer using I2C.
    ActivateAccelerometer();

    worker_thread = std::thread(&Accelerometer::Worker, this);
}

I'm using an initializer list, generally the preferred way of initializing class members.