C6386 warning in VS 22 buffer overrun

Question

I am trying to make a function that gets a matrix and shifts its columns to the right or left, depend on the user's repeat input. While doing this, I noticed compiler warnings (C6386, C6001, C6385), all in the same code area, and I can't find the problem.

Warning C6386 Buffer overrun while writing to 'temp_row'. Warning C6001 Using uninitialized memory '*temp_row'. Warning C6385 Reading invalid data from 'temp_row'.

This is the code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAX_MATRIX_ALLOWED 2
#define MAX_COMMEND_LINE 81
#define MAX_MATRIX_NAME_LEN 13

int matrix_count = 0;
char variebels_names[MAX_MATRIX_ALLOWED][MAX_MATRIX_NAME_LEN];
int* matrix_data[MAX_MATRIX_ALLOWED];
int matrix_rows[MAX_MATRIX_ALLOWED];
int matrix_cols[MAX_MATRIX_ALLOWED];

float StringToFloat(const char* str) {
    float result = 0.0;
    sscanf_s(str, "%f", &result);
    return result;
}

void createMatrix(const char* name, float rows, float cols) {
    // check name length and letters
    if (strlen(name) > MAX_MATRIX_NAME_LEN) {
        printf_s("Error: '%s' - invalid variable name!\n", name);
        return;
    }
    // check if name is already in use
    for (int i = 0; i < matrix_count; i++) {
        if (strcmp(name, variebels_names[i]) == 0) {
            printf_s("Error: variable name '%s' is already in use!\n", name);
            return;
        }
    }
    // check if the rows and cols are int
    if (rows - (int)rows != 0 || cols - (int)cols != 0 || cols <= 0 || rows <= 0) {
        printf_s("Error: invalid dimension!\n");
        return;
    }
    int cols_int = (int)cols;
    int rows_int = (int)rows;
    // check if there is the same variable name
    if (matrix_count >= MAX_MATRIX_ALLOWED) {
        printf_s("Error: zeros cannot save more than 2 variables!\n");
        return;
    }

    /*save memory for the matrix data*/

    int** matrix = (int**)malloc (rows_int *sizeof(int*));
    if (!matrix) exit(1);
    for (int i = 0; i < rows_int; i++)
    {
       matrix[i] = (int*)malloc(cols_int * sizeof(int));
       if (!matrix[i]) exit(1);
    }

    for (int i = 0; i < rows_int; i++)
    {
        for (int j = 0; j < cols_int; j++)
        {
            matrix[i][j] = 0;
        }
    }
    matrix_data[matrix_count] = matrix;
    matrix_rows[matrix_count] = rows_int;
    matrix_cols[matrix_count] = cols_int;
    strncpy_s(variebels_names[matrix_count], MAX_MATRIX_NAME_LEN, name, MAX_MATRIX_NAME_LEN - 1);
    variebels_names[matrix_count][MAX_MATRIX_NAME_LEN - 1] = '\0';

    matrix_count++;
    fflush(stdout);
}

void matrixShift(const char* name, int repeat) {
    //check if name is defined
    int matrix_index = -1;
    for (int i = 0; i < matrix_count; i++)
    {
        if (strcmp(name, variebels_names[i]) == 0)
        {
            matrix_index = i;
            break;
        }
    }
    if (matrix_index == -1)
    {
        printf_s("Error: '%s' - unknown variable!\n", name);
        return;
    }
    int rows = matrix_rows[matrix_index];
    int cols = matrix_cols[matrix_index];
    int** matrix = matrix_data[matrix_index];
    int shift = repeat % cols;
    if (shift >= 0) {
        shift = (cols - (repeat % cols)) % cols;
    }
    else
    {
        shift = (-repeat) % cols;
    }


    for (int i = 0; i < rows; i++) {
        // Create a temporary row
        int* temp_row = (int*)malloc(cols * sizeof(int));
        if (!temp_row) exit(1);
        int temp_index = 0;

        // Copy elements from the original row to the temporary row
        for (int j = shift; j < cols; j++) {
            temp_row[temp_index] = matrix[i][j];
            temp_index++;
        }
        for (int j = 0; j < shift; j++) {
            temp_row[temp_index] = matrix[i][j];
            temp_index++;
        }

        // Copy elements back to the original row
        for (int j = 0; j < cols; j++) {
            matrix[i][j] = temp_row[j];
        }

        // Free the temporary row
        free(temp_row);
    }
}

int main() {
    char command[MAX_COMMEND_LINE];
    printf_s("$ ");
    fflush(stdout);
    while (fgets(command, sizeof(command), stdin) != NULL)
    {
        char* token;
        char* context = NULL;
        // split the command string
        token = strtok_s(command, " \n", &context);
        // check if the user didn't put input well
        if (token == NULL) {
            printf_s("$ ");
            fflush(stdout);
            continue;
        }
        // compare the tokens to the commands
        if (strcmp(token, "exit") == 0) {
            break;
        }
        else if (strcmp(token, "zeros") == 0) {
            token = strtok_s(NULL, " \n", &context);
            if (token == NULL) {
                printf_s("Error: illegal command!\n");
                printf_s("$ ");
                fflush(stdout);
                continue;
            }
            char variable_name[MAX_MATRIX_NAME_LEN];
            strncpy_s(variable_name, MAX_MATRIX_NAME_LEN, token, MAX_MATRIX_NAME_LEN - 1);
            variable_name[MAX_MATRIX_NAME_LEN - 1] = '\0';
            // checking the length of the variable name
            if (strlen(variable_name) > MAX_MATRIX_NAME_LEN) {
                printf_s("Error: '%s' - invalid variable name!\n", variable_name);
                printf_s("$ ");
                fflush(stdout);
                continue;
            }

            token = strtok_s(NULL, " \n", &context);
            if (token == NULL) {
                printf_s("Error: illegal command!\n");
                printf_s("$ ");
                fflush(stdout);
                continue;
            }
            float rows = StringToFloat(token);

            token = strtok_s(NULL, " \n", &context);
            if (token == NULL) {
                printf_s("Error: illegal command!\n");
                printf_s("$ ");
                fflush(stdout);
                continue;
            }
            float cols = StringToFloat(token);

            createMatrix(variable_name, rows, cols);
        }
        else if (strcmp(token, "shift") == 0) {
            token = strtok_s(NULL, " \n", &context);
            if (token == NULL) {
                printf_s("Error: illegal command!\n");
                printf_s("$ ");
                fflush(stdout);
                continue;
            }
            char variable_name[MAX_MATRIX_NAME_LEN];
            strncpy_s(variable_name, MAX_MATRIX_NAME_LEN, token, MAX_MATRIX_NAME_LEN - 1);
            variable_name[MAX_MATRIX_NAME_LEN - 1] = '\0';

            token = strtok_s(NULL, " \n", &context);
            if (token == NULL) {
                printf_s("Error: illegal command!\n");
                printf_s("$ ");
                fflush(stdout);
                continue;
            }
            int repeat = StringToFloat(token);
            matrixShift(variable_name, repeat);
        }
        else {
            printf_s("Error: unknown command!\n");
        }
        printf_s("$ ");
        fflush(stdout);
    }

    // free allocated memory for matrices
    for (int i = 0; i < matrix_count; i++) {
        free(matrix_data[i]);
    }
    return 0;
}

I tried to solve it using -1 from the temp_row size but it didn't work.

Answer 1

There are numerous posts on Stack Overflow about "false positive" C6386 warnings from the Visual Studio (MSVC) compiler. This is a 'weakness' in the static analyser (I don't like to use the term, "bug," because static analysis is a highly complex process and the compiler can't always see all possible outcomes of code execution, even though you – as the author – may know that certain conditions can't occur).

However, despite the numerous closely related posts, I can't find one that proposes a workaround for your particular issue, so I shall offer a possibility.

But first, some attempts at explaining the warnings:

1. warning C6001: Using uninitialized memory '*temp_row'
   warning C6385: Reading invalid data from 'temp_row'.

When the analyser sees the last of the three nested for (int j = 0 ...) loops, it can't be 100% sure that the previous two loops have actually written to all elements of the temp_row array (even though you know they have). You can silence these by using calloc instead of malloc, which will initialize all elements to zero.

2. warning C6386: Buffer overrun while writing to 'temp_row'.

In this case, the analyser isn't aware that the temp_index value is guaranteed to be in the [0..cols) range (again, even though you know it will be). You can silence this warning by adding an additional check (that temp_index < cols) in the second of your inner for (int j = 0 ...) loops.

Here is the relevant section of your code with these changes made (I have left your original code lines but commented them out):

    for (int i = 0; i < rows; i++) {
        // Create a temporary row
        int* temp_row = calloc(cols, sizeof(int)); 
    //  int* temp_row = (int*)malloc(cols * sizeof(int));
        if (!temp_row) exit(1);
        int temp_index = 0;

        // Copy elements from the original row to the temporary row
        for (int j = shift; j < cols; j++) {
            temp_row[temp_index] = matrix[i][j];
            temp_index++;
        }
    //  for (int j = 0; j < shift; j++) {
        for (int j = 0; j < shift && temp_index < cols; j++) {
            temp_row[temp_index] = matrix[i][j];
            temp_index++;
        }

        // Copy elements back to the original row
        for (int j = 0; j < cols; j++) {
            matrix[i][j] = temp_row[j];
        }

        // Free the temporary row
        free(temp_row);
    }

Making these changes silences all three warnings on my version of MSVC (Visual Studio 2022). However, they also cause me some confusion, because keeping only the first change (i.e. using calloc) removes the C6383 and C6385 but leaves the C6001; further, keeping only the second change removes the C6001. This is the exact opposite of what I was expecting!