-6

I'm trying to get the following code for merge sort work with vectors instead of C style arrays and I'm having a hard time figuring out why it crashes with segmentation fault. Can someone help me out in understanding the problem here?

#include <bits/stdc++.h>
using namespace std;

void merge(vector<int>& a, int l, int m, int r) {
    vector<int> L;
    vector<int> R;

    for (int i = 0; i <= m; i++)
        L.push_back(a[i]);
    for (int i = m+1; i <= r; i++)
        R.push_back(a[i]);

    int i = 0, j = 0; // Initial index of first and second subarray
    int k = l;  // Initial index of merged subarray
    while (i < L.size() && j < R.size()) {
        if (L[i] <= R[j]) {
            a[k] = L[i];
            i++;
        }
        else {
            a[k] = R[j];
            j++;
        }
        k++;
    }

    // Filling leftovers
    while (i < L.size()) {
        a[k] = L[i];
        k++;
        i++;
    }
    while (j < R.size()) {
        a[k] = R[j];
        k++;
        j++;
    }
}

void merge_sort(vector<int>& a, int l, int r) {
    if (l < r) {
        int m = l + (r-l) / 2; // Avoids integer overflow.
        merge_sort(a, l, m);
        merge_sort(a, m+1, r);
        merge(a, l, m, r);
    }
}

int main()
{
    vector<int> a = {2, 4, 1, 5, 3, 9};
    int size = a.size();    
    merge_sort(a, 0, size-1);
    for (int i = 0; i < size; i++)
        cout << a[i] << ' ';
    cout << endl;
    return 0;
}
Duh
  • 87
  • 1
  • 1
  • 12
  • 2
    The right tool to solve such problems is your debugger. You should step through your code line-by-line *before* asking on Stack Overflow. For more help, please read [How to debug small programs (by Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). At a minimum, you should \[edit] your question to include a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve) example that reproduces your problem, along with the observations you made in the debugger. – πάντα ῥεῖ Nov 01 '16 at 15:00
  • `L` is overlong - you put `m+1` elements into it, rather than `m-l`. As a result, `k` may walk off the end of `a`. Whereupon your program exhibits undefined behavior, by way of a buffer overrun. – Igor Tandetnik Nov 01 '16 at 15:06
  • ππάντα ῥεῖ I've already tried what you suggest but since I couldn't get through it I suspected I'm missing some detailed point on using vectors which might be the problem here. I'm adding valgrind messages to the question in a minute. Sorry for missing them out. Thanks for pointing that out. – Duh Nov 01 '16 at 15:06
  • 3
    No, the bug has nothing to do with fine details of using vectors. It's a plain logical error, well suited to investigating with a debugger. – Igor Tandetnik Nov 01 '16 at 15:07
  • @IgorTandetnik Indeed. – Duh Nov 01 '16 at 15:14

1 Answers1

0

You need to go from l to m when filling vector L:

void merge(vector<int>& a, int l, int m, int r) {
    vector<int> L;
    vector<int> R;

    for (int i = l; i <= m; i++)
        L.push_back(a[i]);
    ...
}

Otherwise L contains additional elements outside the range of interest. Then, index k steps off the end of vector a when iterating for the size of L.

Sean Kuhlman
  • 502
  • 3
  • 13