Why is the code throwing segmentation fault?

Question

The problem is to find the number i<=n, n<=500000 for which the longest collatz series exists. Collatz series for a number n terminates at 1, and the conditions are if n is even, next term = n/2 if n is odd, next term = 3*n + 1

Well as a matter of fact, the collatz series always terminates at 1 for all numbers.

Hence any number won't repeat in its collatz series. Using this fact, I have written the following code LOGIC: I start a while loop, that goes till n and for each iteration, I store the length of the series for that i. If i occurs in the series of some n >= r > i, then i terminate the loop and add the length of i to r. For example, say series of 3 is 3, 10, 5, 16, 8, 4, 2, 1. Now the length corresponding to 2 will already be stored in the series_length array, so I use that value.

Then the for loop next to that, finds the longest series and displays the answer.

The code works fine for n <= 1818 to be precise, but shows segmentation fault onwards (dunno why :(). Please help

CODE :

#include <stdio.h>

int length = 0, series_length[500000], maxlength = 0;

void store_length(int n) {
    while(n > 1 && series_length[n] == 0) {
        length++;
        if(n%2 == 0) {
            n = n/2;
        }
        else {
            n = 3*n + 1;
        }
    }
    length += series_length[n];
}

int main() {
    int n, i = 1, result;
    scanf("%d", &n);
    series_length[1] = 1;//redundant statement

    while(i <= n) {
        store_length(i);
        series_length[i] = length;
        length = 0;
        i++;
    }

    for(int i = 1;i <= n; i++) {
        if(maxlength <= series_length[i]) {
            maxlength = series_length[i];
            result = i;
        }
    }

    printf("%d %d\n", result, maxlength);

    return 0;
}

INPUT- 10 OUTPUT- 9 20 (AS Expected)

INPUT- 100000 OUTPUT- Segmentation Fault Expected- 77031 351

Most likely an out of bounds array access; check trivially with `if` statements around all array indexing. — Bathsheba, Jan 29 '20 at 07:34
You seem to believe `series_length[500000]` creates an array with indexes that go from 1 to 500000, while they go from 0 to 499999. — ikegami, Jan 29 '20 at 07:44
Your `store_length` function isn't checking to see if `n` has grown larger than 50000 (actually your code can only handle up to 49999, the largest index supported by your array). With input 1819, n eventually becomes 52487, at which point you index past the end of your array. You need to add a check to stop accessing the array when your index is >= 50000 (or > 50000 if you increase your array size by one). — Tom Karzes, Jan 29 '20 at 07:46
Running your code with the value 100000 overflows the array `series_length`. n reaches the value 504466. You must grow the size of the `series_length` array or change your algorithm. — chmike, Jan 29 '20 at 07:49
Testing a bit more, I see that for n = 500000, the value can be bigger than 1,047,216,490 ! I didn’t explore further. You certainly exceed the magnitude of 32 bits ints. You need to use `long long int` as type for `n`. An array of this size becomes huge. You should probe the biggest value you get when `n` is 500000. — chmike, Jan 29 '20 at 08:03

score 3 · Answer 1 · answered Jan 29 '20 at 07:39

3

Your value for n goes outside the range.

You have a line n = 3*n + 1; in the function store_length

Running this with the gdb with input as 100000 gives

 Thread 1 received signal SIGSEGV, Segmentation fault.
 0x0000000000401545 in store_length (n=532060) at 29_01.c:6
6           while(n > 1 && series_length[n] == 0) {
(gdb) p n
$1 = 532060

answered Jan 29 '20 at 07:39

Rishikesh Raje

8,556
2
16
31

But increasing the array size or making n long is still not working – Sukrit Kapil Jan 29 '20 at 07:43
1

Do you know the limit for the array size? If not you can look at dynamic memory allocation and `realloc` when required. Even then, you might go above the available memory limit, in which case you need to rework on the algorithm – Rishikesh Raje Jan 29 '20 at 07:45

wildplasser · Answer 2 · 2020-01-29T13:04:57.470

only store it if it fits
... and use it if it already has been computed
avoid global variables
prefer unsigned values
[use descriptive variable names]

#include <stdio.h>

#define THE_SIZE 500000

unsigned series_length[THE_SIZE]= {0,};

unsigned get_length(unsigned val) {
    unsigned steps;
    for (steps=0; val > 1 ; steps++) {
        if (val < THE_SIZE && series_length[val]) { steps += series_length[val]; break; }
        if(val %2 ) val = 3*val + 1;
        else val /= 2;
    }
    return steps;
}

int main( int argc, char **argv) {
    unsigned top, val , result;
    unsigned  best,maxlength ;

    sscanf(argv[1], "%u", &top);

    series_length[1] = 1;//redundant statement

    best = maxlength = 0;
    for(val=1;val <= top; val++) {
        result = get_length(val);
        // store it if it fits;
        if(val<THE_SIZE) series_length[val] = result;
        if (result < maxlength) continue;
        best = val; maxlength = result;
    }


    printf("%u %u\n", best, maxlength);

    return 0;
}

Finally, just for fun, make the array smaller

#define THE_SIZE 500

, and the program should give the same result for a given value. (it does)

score 1 · Answer 3 · answered Jan 29 '20 at 09:16

You get the maximum value 24,648,077,896 with n = 487039.

You must thus use the type long long int for n and you should use an array of 24,648,077,896 integers to avoid a segmentation fault. Unfortunately I never succeeded in allocating a block of 100GB. Your optimization is thus not viable.

Without the array optimization I can scan all 500000 n values in 265ms.

Here is my code:

#include <stdio.h>

int collatz_length(int n) {
    int length = 0;
    long long int v = (long long int)n;
    while (v > 1) {
        if ((v&1) == 0)
            v = v / 2;
        else
            v = v*3 + 1;
        length++;
    }
    return length;
}

int main() {
    int max_i, max_l = 0;
    for (int i = 500000; i > 0; i--) {
        int l = collatz_length(i);
        if (l > max_l){
            max_l = l;
            max_i = i;
        }
    }
    printf("i: %d l: %d\n", max_i, max_l);
    return 0;
}

Why is the code throwing segmentation fault?

3 Answers3