Condition creates segmentation fault when fulfilled

Question

Checking if matrix is Toeplitz, when it is not, the code crashes on ***. Don't know why the if statement can't stop it.

for (i = 0; i < M; i++) {
    for (j = 0; j < N; j++) {
        if (j == 0 || i == 0) {
            for (k = 0;; k++) {
                if ((i + k) < M && (j + k) < N) {
                    // *** the code crashed here
                    if (matrix[i + k][j + k] != matrix[i][j]) {
                        z = 1;
                    } else if ((i + k) == (M - 1) || (j + k) == (N - 1)) {
                        break;
                    }
                }
            }
        }
    }
}

Not all control paths reach `break`. Just as bad is checking exact value of `k` instead of a bounds limit. Please make the loop conditions for `k` more transparent. — Weather Vane, Aug 12 '23 at 13:00
Once `k` is large enough for the `if` test to be false, the loop will just keep incrementing `k` until you get an overflow condition for `i + k` or `j + k`. Once that happens, the `if` body may be executed, which could lead to invalid array references. The change suggested above should prevent that, exiting the loop when `k` becomes too large. — Tom Karzes, Aug 12 '23 at 13:16

chqrlie · Answer 1 · 2023-08-12T13:41:10.127

The inner for loop can keep going without ever reaching the break statement. In this case, k keeps increasing until it reaches INT_MAX and then you have undefined behavior when incrementing it or even before when computing i + k and/or j + k. What is probably happening is i + k or j + k becomes a large negative value and passes the test i + k < N but causes a segmentation fault when you try and read from matrix[i + k][j + k].

Here is a modified version:

for (i = 0; i < M; i++) {
    for (j = 0; j < N; j++) {
        if (j == 0 || i == 0) {
            for (k = 0; k < M - i && k < N - j; k++) {
                if (matrix[i + k][j + k] != matrix[i][j]) {
                    z = 1;
                }
            }
        }
    }
}

Note however that these nested loop will iterate many more times than necessary to determine the matrix property. You could rewrite the loops as:

for (j = 0; z != 1 && j < N; j++) {
    for (k = 1; k < M && k < N - j; k++) {
        if (matrix[k][j + k] != matrix[0][j]) {
            z = 1;
            break;
        }
    }
}
for (i = 0; z != 1 && i < M; i++) {
    for (k = 1; k < M - i && k < N; k++) {
        if (matrix[i + k][k] != matrix[i][0]) {
            z = 1;
            break;
        }
    }
}

a Toeplitz matrix or diagonal-constant matrix, named after Otto Toeplitz, is a matrix in which each descending diagonal from left to right is constant. A more direct way to test this property is this:

for (i = 1; i < M; i++) {
    for (j = 1; j < N; j++) {
        if (matrix[i - 1][j - 1] != matrix[i][j]) {
            z = 1;
            break;
        }
    }
    if (z == 1)
        break;
}

Or as a function:

int is_Toeplitz_matrix(int matrix[M][N]) {
    for (int i = 1; i < M; i++) {
        for (int j = 1; j < N; j++) {
            if (matrix[i - 1][j - 1] != matrix[i][j]) {
                return 0;
            }
        }
    }
    return 1;
}

Condition creates segmentation fault when fulfilled

1 Answers1