-3

I am going to write 4 way-merge-sort and I get segmentation failure as an error.

I feel like it is not about recursion depth, since it recursively called for 20 times for my test case.

when I removed the merge_4_way in mergesort_4_way_rec, it can run smoothly. I can spot out any function designing problems for the merge_4_way causing the error. I am using clang for compile and MacOS for testing.

#include <stdio.h>
#include "merge_sort.h"

// merge the four sub-array
// [low, mid1), [mid1, mid2), [mid2, mid3), [mid3, high)
void merge_4_way(int* array, int low, int mid1, int mid2, int mid3, int high) {
    int i = low, j = mid1, k = mid2, p = mid3, l = low;

    int destArray[high - low];
    
    while((i < mid1) && (j < mid2) && (k < mid3) && (p < high))  
    {  
        if( (array[i] <= array[j]) && (array[i] <= array[k]) && (array[i] <= array[p])) 
        {
            destArray[l++] = array[i++]; 
        }
        else if ((array[j] <= array[i]) && (array[j] <= array[k]) && (array[j] <= array[p]))
        {
            destArray[l++] = array[j++]; 
        }
        else if ((array[k] <= array[i]) && (array[k] <= array[j]) && (array[k] <= array[p]))
        {
            destArray[l++] = array[k++];
        }
        else if ((array[p] <= array[i]) && (array[p] <= array[j]) && (array[p] <= array[k]))
        {
            destArray[l++] = array[p++];
        }
    }


    // case where first and second ranges 
    // have remaining values  
    while ((i < mid1) && (j < mid2) && (k < mid3))  
    {  
        if((array[i] <= array[j]) && (array[i] <= array[k])) 
        { 
            destArray[l++] = array[i++]; 
        } 
        else if ((array[j] <= array[i]) && (array[j] <= array[k])) 
        { 
            destArray[l++] = array[j++]; 
        }
        else if ((array[k] <= array[i]) && (array[k] <= array[j])) 
        { 
            destArray[l++] = array[k++]; 
        }  

    } 
  
    // case where first and second ranges 
    // have remaining values  
    while ((i < mid1) && (j < mid2) && (p < high))  
    {  
        if((array[i] <= array[j]) && (array[i] <= array[p])) 
        { 
            destArray[l++] = array[i++]; 
        } 
        else if ((array[j] <= array[i]) && (array[j] <= array[p])) 
        { 
            destArray[l++] = array[j++]; 
        }
        else if ((array[p] <= array[i]) && (array[p] <= array[j])) 
        { 
            destArray[l++] = array[p++]; 
        }  

    }

    while ((i < mid1) && (k < mid3) && (p < high))  
    {  
        if((array[i] <= array[k]) && (array[i] <= array[p])) 
        { 
            destArray[l++] = array[i++]; 
        } 
        else if ((array[k] <= array[i]) && (array[k] <= array[p])) 
        { 
            destArray[l++] = array[k++]; 
        }
        else if ((array[p] <= array[i]) && (array[p] <= array[k])) 
        { 
            destArray[l++] = array[p++]; 
        }  

    } 

    while ((j < mid2) && (k < mid3) && (p < high))  
    {  
        if((array[j] <= array[k]) && (array[j] <= array[p])) 
        { 
            destArray[l++] = array[j++]; 
        } 
        else if ((array[k] <= array[i]) && (array[k] <= array[p])) 
        { 
            destArray[l++] = array[k++]; 
        }
        else if ((array[p] <= array[j]) && (array[p] <= array[k])) 
        { 
            destArray[l++] = array[p++]; 
        }  

    }

    // case where first and second ranges 
    // have remaining values  
    while ((i < mid1) && (j < mid2))  
    {  
        if(array[i] <= array[j]) 
        { 
            destArray[l++] = array[i++]; 
        } 
        else
        { 
            destArray[l++] = array[j++]; 
        } 
    }  
  
    // case where second and third ranges 
    // have remaining values  
    while ((j < mid2) && (k < high))  
    {  
        if(array[j] <= array[k]) 
        { 
            destArray[l++] = array[j++]; 
        } 
        else
        { 
            destArray[l++] = array[k++]; 
        }  
    }    

    // case where third and forth ranges 
    // have remaining values  
    while ((k < mid3) && (p < high))  
    {  
        if(array[k] <= array[p]) 
        { 
            destArray[l++] = array[k++]; 
        } 
        else
        { 
            destArray[l++] = array[p++]; 
        }  
    }  
  
    // case where first and third ranges have  
    // remaining values  
    while ((i < mid1) && (k < mid3))  
    {  
        if(array[i] <= array[k]) 
        { 
            destArray[l++] = array[i++]; 
        } 
        else
        { 
            destArray[l++] = array[k++]; 
        }  
    }
    // case where first and forth ranges have  
    // remaining values  
    while ((i < mid1) && (p < high))  
    {  
        if(array[i] <= array[p]) 
        { 
            destArray[l++] = array[i++]; 
        } 
        else
        { 
            destArray[l++] = array[p++]; 
        }  
    } 
    // case where second and forth ranges have  
    // remaining values  
    while ((j < mid2) && (p < high))  
    {  
        if(array[j] <= array[p]) 
        { 
            destArray[l++] = array[j++]; 
        } 
        else
        { 
            destArray[l++] = array[p++]; 
        }  
    }

    // copy remaining values from the first range  
    while (i < mid1)  
        destArray[l++] = array[i++];  
  
    // copy remaining values from the second range  
    while (j < mid2)  
        destArray[l++] = array[j++];  
  
    // copy remaining values from the third range  
    while (k < mid3)  
        destArray[l++] = array[k++];
    
    // copy remaining values from the third range  
    while (p < high)  
        destArray[l++] = array[p++];

    for (int i = low; i < high; i++){
        array[i] = destArray[i];
    } 
    
}

// divide the array [low, high) into 4 parts (roughly same size).
// For each part, if # of items > 3, recursively call mergesort_4_way_rec; 
// Otherwise sort them as you like
// Finally use merge_4_way merge them
void mergesort_4_way_rec(int* array, int low, int high) {
    int mid1 = low + ((high-low) / 4);
    int mid2 = low + 2 * ((high-low) / 4);
    int mid3 = low + 3 * ((high-low) / 4);
     printf("%d\n", (high - low));
    if ((high - low) > 3){

        mergesort_4_way_rec(array, low, mid1);
        mergesort_4_way_rec(array, mid1, mid2);
        mergesort_4_way_rec(array, mid2, mid3);
        mergesort_4_way_rec(array, mid3, high);

        merge_4_way(array, low, mid1, mid2, mid3, high);
    } else {
        for(int i=low; i<high;i++)
        {
            for(int j=low;j<high-i-1;j++)
            {
                if(array[j]<array[j+1])
                {
                    int temp=array[j];
                    array[j]=array[j+1];
                    array[j+1]=temp;
                }
            }
        } 
        return;  
    }


    
}
Chak Wing Mak
  • 41
  • 7
  • Please try with [clang address sanitizer](https://stackoverflow.com/questions/31177633/how-to-find-memory-leaks-with-clang) or [valgrind](https://valgrind.org/docs/manual/mc-manual.html). Sadly SO does not provide a bot suggestion "did you try valgrind/clang sanitizer?" for C/C++ code. – Jay-Pi Sep 12 '20 at 18:36
  • 2
    The debugger is your friend. If it was a [mcve] one of us might run it for you and potentially spot the issue. – Retired Ninja Sep 12 '20 at 18:39

1 Answers1

0

If low isn't 0 then you'll end up writing past the end of destArray.

The size of destArray is high - low. You initialize l (the index variable you use when you write to destArray) to low, so you end up writing to elements destArray[low] thru destArray[high - 1], which will be past the end if low isn't zero.

One solution is to initialize l = 0; and change the loop at the end of merge_4_way to index destArray properly.

Another solution would be to declare destArray to have space for all the elements, even the unused ones, with int destArray[high]; although this will waste stack space.

1201ProgramAlarm
  • 32,384
  • 7
  • 42
  • 56