Why do I receive Runtime Error(Segmentation fault) by using std::copy()?

Question

I was writing a program for homework.

It's about simulating two people playing a card game.

At first, Mr.Y has n cards while Mr.P has m cards and the cards they have are unique.

Mr.Y plays first, and here's how you play the game:

• First, you pick a number x, if the number is among the cards of your competitor, you take it.
• Then, if you already have the card you got, you throw both the card on your hand and the card you just got.
• Then, your competitor does the same thing (he picks a number w).

That's the end of the first round.

The game ends when one of the people ran out of cards.

Also, if the game doesn't end in k rounds, it automatically ends.

The program has to output:

A number l, which represents in how many rounds will the game end.

The cards Mr.Y and Mr.P have when a round ends.

I have tried to simulate the game using vector but the code raises a segmentation fault when I tried to execute it.

Here's my code:

#include<bits/stdc++.h>
using namespace std;
int n,m,k,w,x,a;
vector<int> y,p;
vector<vector<int>> ymoves;
vector<vector<int>> pmoves;
int main(){
    cin>>n>>m;
    for(int i=0;i<n;i++){
        cin>>a;
        y.push_back(a);
    }
    for(int i=0;i<m;i++){
        cin>>a;
        y.push_back(a);
    }
    cin>>k;
    bool b=1;
    for(int i=0;i<k;i++){
        cin>>x>>w;
        for(int j=0;j<p.size();j++){
            if(p[j]==x){
                y.push_back(x);
                p.erase(p.begin()+j);
                break;
            }
        }
        for(int j=0;j<y.size()-1;j++){
            if(y[j]==x){
                y.erase(y.begin()+j);
                y.pop_back();
                break;
            }
        }
        for(int j=0;j<y.size();j++){
            if(y[j]==w){
                p.push_back(w);
                y.erase(y.begin()+j);
                break;
            }
        }
        for(int j=0;j<p.size()-1;j++){
            if(p[j]==w){
                p.erase(p.begin()+j);
                p.pop_back();
                break;
            }
        }
        copy(y.begin(),y.end(),ymoves[i].begin());
        copy(p.begin(),p.end(),pmoves[i].begin());
        if(y.empty()||p.empty()){
            b=0;
            cout<<i<<'\n';
            for(int j=0;j<i;j++){
                cout<<ymoves[j].size()<<' ';
                for(int l=0;l<ymoves[j].size();l++)cout<<ymoves[j].at(l)<<' ';
                cout<<'\n';
                cout<<pmoves[j].size()<<' ';
                for(int l=0;l<pmoves[j].size();l++)cout<<pmoves[j].at(l)<<' ';
                cout<<'\n';
            }
            break;
        }
    }
    if(b){
        cout<<k<<'\n';
        for(int j=0;j<k;j++){
            cout<<ymoves[j].size()<<' ';
            for(int l=0;l<ymoves[j].size();l++)cout<<ymoves[j].at(l)<<' ';
            cout<<'\n';
            cout<<pmoves[j].size()<<' ';
            for(int l=0;l<pmoves[j].size();l++)cout<<pmoves[j].at(l)<<' ';
            cout<<'\n';
        }
    }
}

Answer 1

One problem that will cause undefined behavior and can as of that result in a segmentation fault any time at and from that point is:

copy(y.begin(), y.end(), ymoves[i].begin());
copy(p.begin(), p.end(), pmoves[i].begin());

Both ymoves and pmoves are initialized with a size of 0. So doing ymoves[i]/pmoves[i] results into an out of bounds access, which causes UB.

As you define ymoves and pmoves globally (which does not make to much sense in your case and should be avoided in general) you need to use resize() as soon as you know how large k is:

cin>>k;
ymoves.resize(k);
pmoves.resize(k);

But that still would result in undefined behavior at the point where you do copy. As the vecotrs that are stored in pmoves/ymoves are also initialized with the size of 0 but if you use copy you have to ensure that the target is large enough to hold the data.

So you need to do a resize on those:

ymoves[i].resize(y.size());
pmoves[i].resize(p.size());

copy(y.begin(),y.end(),ymoves[i].begin());
copy(p.begin(),p.end(),pmoves[i].begin());

But I'm not really sure if doing a copy here is the correct way and I don't know if overwriting the existing values is what you want to do.

Two other errors are: for(int j=0;j<p.size()-1;j++){ and for(int j=0;j<y.size()-1;j++){, size() returns a signed value, so if p.size() is 0 you will have a problem, and thats exactly what happens in your code. Those loops should only be done if p.size() is not zero.

But due to the not optimal naming of your variables, it is hard to figure out if that part is correct and if there are more parts that are problematic. But that's at least the most obvious reason that causes UB.