C++ Invalid Pointer error when compiling with g++ on Linux, but not in CLion on Windows

Question

I am having a rather strange problem. I wrote a program that, for each value of n between 2 and 100, generates two identical c-type arrays of random numbers, and passes one to a merge sort algorithm, and one to a selection sort algorithm. It runs 10,000 tests with each, and determines the worse-case scenario number of operations for each, for each value of n out of the 10,000 tests. It then writes each value of n, along with the worst-case number of merge sort algorithm operations, and the worst-case number of selection sort operations, to a .csv file.

My problem is this: When I build/compile and run the code in my CLion IDE (on Windows 10), it works perfectly fine, and finishes with an exit code of 0.

When I use FTP to copy the files over to a linux server and try to compile and run everything, it compiles fine, but I get a runtime error about an invalid pointer.

I am compiling the code with g++ -std=c++14 -Wall main.cpp SortFunctions.cpp -o program3.out.

Here is a screenshot of my error message:

Here is my main.cpp file:

#include <iostream>
#include <string>
#include <cstdlib>          // FOR RANDOM NUMBER GENERATION
#include <ctime>            // FOR SEEDING RANDOM NUMBER GENERATION
#include "SortFunctions.h"
#include <fstream>

using std::cout;
using std::cin;
using std::endl;
using std::string;
using std::ofstream;
using std::ios;


int main() {

    // CREATE ARRAYS TO STORE NUMBER OF WORSE-CASE OPERATIONS
    long long int worst_case_merge_operations[99]; // 99
    long long int worst_case_selection_operations[99]; // 99

    // FOR ALL VALUES OF n BETWEEN 2 AND 100, INCLUSIVE
    for (int n = 2; n <= 100; n++){

        cout << "Trial with n = " << n << endl;

        // VARIABLES TO STORE THE VALUE UNTIL ADDED TO THE ARRAY
        long long int worst_num_merge_operations = 0;
        long long int worst_num_selection_operations = 0;
        // FOR EACH
        for (int i = 1; i <= 10000; i++) {

            if (i % 100 == 0){
                cout << "Beginning test " << i << " of 10,000 for n = " << n << endl;
            }

            // CREATE ARRAYS (OF LENGTH N)TO POPULATE WITH RANDOM NUMBERS
            int array1[n];
            int array2[n];



            // POPULATE ARRAYS WITH RANDOM NUMBERS
            for (int k = 0; k < n; k++) {

                // GENERATE A RANDOM INTEGER IN THE RANGE [1,1000]
                int rand_num = (rand() % 1000) + 1;

                // ADD THE NUMBER TO EACH ARRAY, THUS MAKING THE ARRAYS IDENTICAL
                array1[k] = rand_num;
                array2[k] = rand_num;

            }

            // CALL MERGE SORT WITH THE ARRAY, INDEX OF FIRST ELEMENT, AND INDEX OF LAST ELEMENT

            long long int merge_ops = MergeSort(array1, 0, n - 1);
            if (merge_ops > worst_num_merge_operations){
                worst_num_merge_operations = merge_ops;
            }


            // CALL SELECTION SORT, COUNTING OPERATIONS
            long long int selection_ops = SelectionSort(array2, n);

            if (selection_ops > worst_num_selection_operations){
                worst_num_selection_operations = selection_ops;
            }


            // NOTE: DO NOT FORGET TO DELETE THE ARRAYS/POINTERS, OTHERWISE YOU'LL HAVE A MEMORY LEAK!!!
            delete[] array1;
            delete[] array2;


            if (i % 100 == 0){
                cout << "Completed test " << i << " of 10,000 for n = " << n << endl;
            }

        }

        // ADD WORST-CASE NUMBER OF OPERATIONS OF EACH TO THEIR REPSECTIVE ARRAY
        worst_case_merge_operations[n - 2] = worst_num_merge_operations;
        worst_case_selection_operations[n - 2] = worst_num_selection_operations;
    }

    // PERFORM FILE IO TO CSV FILE
    // COLUMNS: [N] [WORST CASE MERGE] [WORST CASE SELECTION]
    cout << endl << "Attempting to perform file IO" << endl;
    ofstream data_file("algorithm_data.csv");

    // CHECK IF FILE IS OPEN
    if(!data_file.is_open()){
        cout << "Unable to open data file algorithm_data.csv" << endl;
        return -1;
    } else {
        cout << "Data file opened" << endl;
    }

    // WRITE HEADER ROW FOR FILE
    data_file << "n,worst case merge operations,worst case selection operations" << endl;
    cout << "Header row written, attempting to write data rows" << endl;

    // WRITE VALUES TO THE FILE
    for (int i = 0; i < 98; i++){

        //WRITE N (= I+2),WORST_CASE_MERGE,WORST_CASE_SELECTION \n
        data_file << i+2 << "," << worst_case_merge_operations[i] << "," << worst_case_selection_operations[i] << endl;
    }

    cout << "Data rows written, attempting to close the file" << endl;

    // CLOSE THE FILE
    data_file.close();
    cout << "Data file closed" << endl;


    return 0;
}

Here is my SortFunctions.cpp file:



#include "SortFunctions.h"
#include <iostream>

using namespace std;

/*
 * NOTE: MERGE SORT ALGORITHM
 */

long long int Merge(int numbers[], int i, int j, int k) {

    // COUNT OPERATIONS
    long long int operations = 0;

    // INCREMENT OPERATIONS
    operations += 1;

    // ALGORITHM
    int mergedSize;                            // Size of merged partition
    int mergePos;                              // Position to insert merged number
    int leftPos;                               // Position of elements in left partition
    int rightPos;                              // Position of elements in right partition
    int* mergedNumbers = nullptr;

    mergePos = 0;
    mergedSize = k - i + 1;
    leftPos = i;                               // Initialize left partition position
    rightPos = j + 1;                          // Initialize right partition position
    mergedNumbers = new int[mergedSize];       // Dynamically allocates temporary array
    // for merged numbers

    // Add smallest element from left or right partition to merged numbers
    while (leftPos <= j && rightPos <= k) {

        // INCREMENT OPERATIONS
        operations += 1;

        if (numbers[leftPos] < numbers[rightPos]) {
            mergedNumbers[mergePos] = numbers[leftPos];
            ++leftPos;
        }
        else {
            mergedNumbers[mergePos] = numbers[rightPos];
            ++rightPos;

        }
        ++mergePos;
    }

    // If left partition is not empty, add remaining elements to merged numbers
    while (leftPos <= j) {

        // INCREMENT OPERATIONS
        operations += 1;

        mergedNumbers[mergePos] = numbers[leftPos];
        ++leftPos;
        ++mergePos;
    }

    // If right partition is not empty, add remaining elements to merged numbers
    while (rightPos <= k) {

        // INCREMENT OPERATIONS
        operations += 1;

        mergedNumbers[mergePos] = numbers[rightPos];
        ++rightPos;
        ++mergePos;
    }

    // Copy merge number back to numbers
    for (mergePos = 0; mergePos < mergedSize; ++mergePos) {

        // INCREMENT OPERATIONS
        operations += 1;

        numbers[i + mergePos] = mergedNumbers[mergePos];
    }

    return operations;
}

long long int MergeSort(int numbers[], int i, int k) {

    // COUNT OPERATIONS
    long long int operations = 0;

    // INCREMENT OPERATIONS
    operations += 1;

    //ALGORITHM
    int j;

    if (i < k) {
        j = (i + k) / 2;  // Find the midpoint in the partition

        // Recursively sort left and right partitions
        operations = operations + MergeSort(numbers, i, j);
        operations = operations + MergeSort(numbers, j + 1, k);

        // Merge left and right partition in sorted order
        operations = operations + Merge(numbers, i, j, k);
    }

    return operations;
}

/*
 * NOTE:SELECTION SORT ALGORITHM
 */

long long int SelectionSort(int numbers[], int numbersSize) {

    // CREATE COUNTER
    long long int counter = 0;

    // INCREMENT COUNTER
    counter += 1;

    int i;
    int j;
    int indexSmallest;
    int temp;      // Temporary variable for swap

    for (i = 0; i < numbersSize - 1; ++i) {

        // INCREMENT COUNTER
        counter += 1;
        // Find index of smallest remaining element
        indexSmallest = i;
        for (j = i + 1; j < numbersSize; ++j) {

            // INCREMENT COUNTER
            counter += 1;
            if ( numbers[j] < numbers[indexSmallest] ) {
                indexSmallest = j;
            }
        }

        // Swap numbers[i] and numbers[indexSmallest]
        temp = numbers[i];
        numbers[i] = numbers[indexSmallest];
        numbers[indexSmallest] = temp;
    }
    return counter;
}

And finally, my SortFunctions.h file:


#ifndef PROGRAM_3_SORTFUNCTIONS_H
#define PROGRAM_3_SORTFUNCTIONS_H
//code here


long long int SelectionSort(int numbers[], int numbersSize);
long long int MergeSort(int numbers[], int i, int k);

#endif //PROGRAM_3_SORTFUNCTIONS_H

I am relatively new to pointers, and would appreciate it if someone could help. I have spent an hour on Google trying to find a solution to no avail.

`int array1[n];` is a variable length array. They are not legal in Standard C++, but g++ allows them by extension. I recommend against using them, but that's my opinion. g++ implements variable length arrays as Automatic variables, so `delete[] array1;` is an extremely bad idea. If you don't `new[]`, don't `delete[]`. Is this your bug? I dunno. Could be dozens of bugs in this much code. That's one of the things a [mcve] takes care of. — user4581301, Mar 07 '19 at 00:08
Sidenote: Compiler should warn about deleting an Automatic. Don't ignore warnings. They are the first line of defense against mistakes like this. — user4581301, Mar 07 '19 at 00:11

score 3 · Answer 1 · edited Jun 20 '20 at 09:12

Compiled with -Wall -Wextra -Werror

> g++ -Wall -Wextra -Werror main.cpp 
main.cpp:72:13: error: cannot delete expression of type 'int [n]'
            delete[] array1;
            ^        ~~~~~~
main.cpp:73:13: error: cannot delete expression of type 'int [n]'
            delete[] array2;
            ^        ~~~~~~

Seems like you have two errors in your code.

Since both these arrays are declared as automatic variables:

        int array1[n];
        int array2[n];

It is an error to call delete on them (automatic variables should never be passed to delete). So remove the two lines with delete.

You should be able to update CLion to give treat your errors as warnings:

How to enable all compiler warnings in CLion?
https://www.jetbrains.com/help/clion/configuring-inspection-severities.html

Remember warnings are actual logical errors in your thinking. Your should attempt to compile all your code with zero warnings. Or explicitly acknowledge the warning in the code and document it.

Note 1:

You should only call delete on variables that were generated with new.
You should only call delete [] on variables that were generated with new []

Note 2:

It is very unusual in modern C++ to see bare new/delete in the code. This is not exception safe and thus is usually wrapped inside a class. See note 4 to handle most situations were dynamic memory is needed.

Note 3:

Dynamically allocated objects are usually controlled via smart pointers. While groups of objets usually controlled via the std:: containers (usually std::vector when you are unsure of what to use).

Note 4:

Variable length arrays VLA are technically not a standard part of the language. Some compilers support them but it is usally better to use std::vector

int                array1[n];  // Non standard if n is not a compile time constant.
std::vector<int>   array1(n);

Interesting, thank you! For some reason, when CLion compiled it for me, it just warned me "Warning: deleting array 'array1'". I'll make sure to use those flags in the future! — Kyle M., Mar 07 '19 at 01:21
By default those are warnings. I use the flag `-Werror` as it changes all warnings into errors. Which then prevents my code from compiling if there are any warnings. — Martin York, Mar 07 '19 at 01:23

C++ Invalid Pointer error when compiling with g++ on Linux, but not in CLion on Windows

1 Answers1

Note 1:

Note 2:

Note 3:

Note 4: