ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.17k stars 386 forks source link

unordered_map inside class triggers segmentation fault #852

Closed ricardomatias closed 7 months ago

ricardomatias commented 7 months ago

The error is: EXC_BAD_ACCESS (code=1, address=0x10) happening inside the unordered_map on the call to .empty

The interesting part is that the Segmentation Fault only happens in the call from schedule of the timeline belonging to the track, it doesn't happen on the first call.

I've added comments pointing to when the Segmentation Fault is triggered and on which call from the Timeline class.

Minimum reproducible code, it compiles in godbolt with clang 17.0.1 and triggers the segmentation fault.

    #include <etl/unordered_map.h>
    #include <etl/deque.h>

    // Simplified placeholder for playa::Event
    struct Event {
        unsigned int time = 0u;
        unsigned int duration = 0u;
        unsigned int next = 0u;
        bool is_rest = false;

        constexpr Event(unsigned int p_time, unsigned int p_duration, unsigned int p_next, bool p_is_rest = false)
        : time(p_time), duration(p_duration), next(p_next), is_rest(p_is_rest)
        {
        }
        constexpr Event(unsigned int p_time, unsigned int p_duration) : time(p_time), duration(p_duration) {}
    };
    struct TransportPosition
    {
        unsigned int bars;
        unsigned int beats;
        unsigned int units;
    };

    template <std::size_t TimelineEventsCount, typename E = Event>
    using TimelineEvents = etl::vector<E, TimelineEventsCount>;

    template <std::size_t UniqueEventsCount, std::size_t TimelineEventsCount, typename E = Event>
    using Arrangement
        = etl::unordered_map<unsigned int, TimelineEvents<TimelineEventsCount, E>, UniqueEventsCount>;

    template <std::size_t UniqueEventsCount, std::size_t TimelineEventsCount, typename E = Event>
    class Timeline {
    public:
        Timeline() = default;  

        void schedule(Event event) {
            unsigned int time = event.time;

            if (arrangement_.empty()) { // <<<<<<<< SEGMENTATION FAULT
                arrangement_.insert({time, {event}}); 
                // ... (rest of the logic not needed for segfault) 
            } 
        }
        template <std::size_t M>
        void schedule(const TimelineEvents<M, E>& events)
        {
            for(auto& event : events)
            {
                schedule(event);
            }
        }

    private:
        Arrangement<UniqueEventsCount, TimelineEventsCount, E> arrangement_;
        etl::deque<unsigned int, UniqueEventsCount> timeline_;
    };

    // Minimal versions of Track, Pattern
    template <std::size_t UniqueEventsCount, std::size_t TimelineEventsCount, typename E = Event>
    struct Pattern
    {
        Timeline<UniqueEventsCount, TimelineEventsCount, E> timeline;
        std::pair<TransportPosition, TransportPosition> playbackRegion;

        Pattern() = default;
        ~Pattern() = default;
    };

    template <std::size_t PatternsCount, std::size_t UniqueEventsCount, std::size_t TimelineEventsCount, typename E = Event>
    using Patterns = etl::vector<Pattern<UniqueEventsCount, TimelineEventsCount, E>, PatternsCount>;

    template <std::size_t PatternsCount, std::size_t UniqueEventsCount, std::size_t TimelineEventsCount, typename E = Event>
    class Track {
    public:
        uint8_t activePattern_ = 0u;
        Patterns<PatternsCount, UniqueEventsCount, TimelineEventsCount, E> patterns_;
        Pattern<UniqueEventsCount, TimelineEventsCount, E> &pattern() { return patterns_[activePattern_]; }
        Timeline<UniqueEventsCount, TimelineEventsCount, E> &timeline() { return patterns_[activePattern_].timeline; }
    };

    int main() {
        Track<1, 6, 6> track; 
    Timeline<6, 6> timeline;

    auto first = TimelineEvents<4, Event>{Event{0, 1, 1}, Event{0, 1, 1}, Event{2, 1, 2}, Event{4, 1, 2}};

    timeline.schedule(first);

    auto second = TimelineEvents<4, Event>{Event{0, 1, 1}, Event{0, 1, 1}, Event{2, 1, 2}, Event{4, 1, 2}};

    // >>>>SEGMENTATION FAULT TRIGGERED HERE<<<<<
    track.pattern().timeline.schedule(second); 

        return 1;
    }
jwellbelove commented 7 months ago

If I step through the code I can see that the constructor for Timeline is never called, which means that arrangement_ is never initialised.

jwellbelove commented 7 months ago

When you call track.pattern().timeline.schedule(second); you have nothing in the patterns_ vector, therefore you are returning a reference to an uninitialised element.

jwellbelove commented 7 months ago
  Pattern<UniqueEventsCount, TimelineEventsCount, E>& pattern() 
  { 
    bool b = patterns_.empty(); <<<< true

    return patterns_[activePattern_]; <<<< Undefined behaviour!
  }
ricardomatias commented 7 months ago

I see that I'm missing a line that adds a default constructor Track() = default; which doesn't fix the issue, but shouldn't it initialize the patterns_ vector? I thought because the size of the vector is defined ahead of time, when doing value initialization the vector would be initialized.

Do you have a suggestion on how to fix it? I suspected that this was the problem, but I don't see the solution in any case, since the vectors are not supposed to be initialized with any specific values.. in my head it's a bit of a catch 22 problem.

jwellbelove commented 7 months ago

The vector is initialised, but nothing has been put in it yet. An empty vector does not contain any constructed elements, and accessing one is undefined behaviour.

i.e.

etl::vector<Object, 10> objects = { Object(1), Object(2) }; // Capacity = 10, size = 2
Object& obj0 = objects[0]; // Valid
Object& obj1 = objects[1]; // Valid
Object& obj2 = objects[2]; // Undefined behaviour

In the above, only two elements have been constructed. The rest of the vector's storage memory is uninitialised.

ricardomatias commented 7 months ago

Thank you very much for your help. The issue really was that, I was looking at a vector but thinking all the time as if it was an array.

jwellbelove commented 7 months ago

Glad to help.