-3

I am trying to make a little solution for IK and I ran into a big wall with this error. My lack of pointers and references have kicked me down. I get this error "this was 0xFFFFFFFFFFFFFFF7" because I think I cannot access the Segment to change its values.

Segment class:


struct Vector2 {
    float x;
    float y;
};

class Segment {
    public:
        Segment(float ix, float iy, int len, float ia, Segment* ip = nullptr) {
            x = ix;
            y = iy;
            angle = ia;
            length = len;
            hasParent = (ip != nullptr);
            parent = ip;
        }
        void pointAt(float tx, float ty) {
            float dx = x - tx;
            float dy = y - ty;
            angle = atan2(-dy, -dx);
        }
        void drag(float tx, float ty) {
            pointAt(tx, ty);
            x = tx - cos(angle) * length;
            y = ty - sin(angle) * length;
            if (hasParent) {
                parent->drag(x, y);
            }
        }
        Vector2 getEnd() {
            return { (float)(x + cos(angle) * length), (float)(y + sin(angle) * length)};
        }
        Vector2 getStart() {
            return { x, y };
        }
        void setAngle(float newAngle) {
            angle = newAngle;
        }

    private:
        bool hasParent;
        float angle;
        float x;
        float y;
        int length;
        Segment* parent;
};

I initialize the vector of segments here, notice how im puting in a reference of the last segments parent for the declaration of the new segment.

I then try to drag the last segment but that's when I get a crash on the endSeg.drag call below

int main() {
    std::vector<Segment> segments;
    size_t segment_amt = 10;
    int length = 0;

    for (int i = 0; i < segment_amt; i++) {
        if (i == 0) {
            segments.push_back(Segment(0.0f, 0.0f, length, 0.0f));
        }
        else {
            segments.push_back(Segment(segments[i - 1].getStart().x, segments[i - 1].getStart().y, length, 0.0f, &segments[i - 1]));
        }
    }

    Segment& endSeg = segments[segments.size() - 1];
    endSeg.drag(0, 0); // crashes here
}

selbie
  • 100,020
  • 15
  • 103
  • 173
SoulDaMeep
  • 57
  • 6
  • Extract and provide a [mcve], not random snippets you think could be the culprit. – Ulrich Eckhardt Jun 24 '23 at 05:49
  • would you like the entire file? @UlrichEckhardt – SoulDaMeep Jun 24 '23 at 05:50
  • No, you _extract_ a [mcve] from the code giving you problems. – Ulrich Eckhardt Jun 24 '23 at 05:51
  • **Please re-open. I have the answer.** I filled in the missing gaps to make a [mcve] and got a live repro of his exact error. – selbie Jun 24 '23 at 06:02
  • TLDR: Each time the OP's code invoke `push_back` in that for-loop, the vector can get resized. When the vector is resized, elements are moved and copied into a new allocation. Hence, the `parent` pointer held by earlier instances in the vector become invalid. – selbie Jun 24 '23 at 06:07
  • 1
    A possible solution would be set the `parent` pointer on each instance after the list is initialized. But a more robust solution would be that `segments` is `vector>` Or just invoke `segments.reserve(segment_amt)` prior to starting the for loop. – selbie Jun 24 '23 at 06:09
  • @selbie which is the reason for the nullptr and is why i get the dereference to a pointer? – SoulDaMeep Jun 24 '23 at 06:09
  • 1
    @SoulDaMeep - As you invoke `push_back` referencing pointers to previous items in the list, the vector can and will reallocate more space. Each time it grows the list, it allocates a complete new array and copies over (moves) the segments from the old array to the new array. Hence, the pointers referenced by `parent` are deleted by the time you start to using them. A very simple solution would be to invoke `segments.reserve(segment_amt)` prior to your for loop. – selbie Jun 24 '23 at 06:12
  • @SoulDaMeep - you aren't getting a null pointer. You're accessing a deleted pointer . And I since I get the same 0xfffffff7 address like you do, chances are high we're both using Visual Studio. – selbie Jun 24 '23 at 06:15
  • @serbie how could i get the minimal reproducible code to update this thing so it can be answered? both these options worked for me and it would be very helpful for others to be able to see these answers more clearly. – SoulDaMeep Jun 24 '23 at 06:21
  • I updated your question by declaring `Vector2` and using the same main I put together to generate a local repo. For future reference, when you ask a question on StackOverflow, provide enough code for someone to copy it into a local IDE program and run it themselves. – selbie Jun 24 '23 at 06:32
  • noted for next time – SoulDaMeep Jun 24 '23 at 06:33
  • Side note, get rid of ALL your "C" casts and replace them with std::static_cast – Pepijn Kramer Jun 24 '23 at 06:33
  • Thank you so much guys for the help and support, I will try better next time to make it more user friendly to test next time i have a question – SoulDaMeep Jun 24 '23 at 06:38
  • I think you can also benefit from a bit more C++ training. Good sources to learn cpp from are : A [recent C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) or have a go at https://www.learncpp.com/ (that's pretty decent, and pretty up-to-date). For C++ reference material use : [cppreference](https://en.cppreference.com/w/). And after you learned the C++ basics from those sources, look at the [C++ coreguidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) regularely to keep up-to-date with the latest guidelines. – Pepijn Kramer Jun 24 '23 at 07:18
  • I also see that you probably could have done something like `std::list` to split the implicit list datastructure from your data class (Segment) – Pepijn Kramer Jun 24 '23 at 07:19

1 Answers1

2

Formalizing the answer.

This line:

segments.push_back(Segment(segments[i - 1].getStart().x, segments[i - 1].getStart().y, length, 0.0f, &segments[i - 1]));

Underneath each std::vector is an array of memory allocated to hold a fixed number of items. When push_back is called and the array doesn't have enough capacity to take on another item, a new array is created (with even more capacity) and the items from the previously allocated array are either copied or moved into the new array.

When this happens, any pointer to any element in the old array is immediately invalid, as the object is likely deleted, and accessing those pointers is undefined behavior. (i.e. when the code accesses parent, that's probably a deleted pointer.)

Hence, passing &segments[i-1] as the assigned parent to an instance of segment while the vector is still growing is an unsafe thing to do.

There's lots of potential fixes. But the simplest solution is to just set the capacity of the vector's underlying array prior to adding elements to it. This:

segments.reserve(segment_amt);

That will reserve enough capacity in the initial array size such it won't have to grow and start a new array allocation while items are being added to it.

A more robust solution would be use a vector of shared_ptr instances. This assumes there are no circular references between instances of Segment.

std::vector<std::shared_ptr<Segment>> segments;
size_t segment_amt = 10;
int length = 0;

for (int i = 0; i < segment_amt; i++) {
    if (i == 0) {
        auto spSegment = std::make_shared<Segment>(0.0f, 0.0f, length, 0.0f);
        segments.push_back(spSegment);
    }
    else {
        auto spParent = segments[i-1];
        auto spSegment = std::make_shared<Segment>(0.0f, 0.0f, length, 0.0f, spSegment);

        segments.push_back(spSegment);
    }
}

Then declare `parent` to be of type `std::shared_ptr<Segment>` inside Segment.
selbie
  • 100,020
  • 15
  • 103
  • 173