Heap corruption in simple C++ program

Question

I wrote a simple C++ program, here’s the code:

// Вариант 72, задача 2.18
#include <iostream>

#define lint long long int

using std::cin;
using std::cout;

void input(lint arr[], lint arrLen) {
    for (lint i = 0; i < arrLen; ++i) {
        cout << "arr[" << i << "] = ";
        cin >> arr[i];
    }
}

void output(lint arr[], lint arrLen) {
    for (lint i = 0; i < arrLen; ++i) {
        cout << "newArr[" << i << "] = " << arr[i] << '\n';
    }
}

bool isNumberInArray(const lint arr[], lint arrLen, lint number) {
    bool isNumberPresent = false;
    for (lint i = 0; i < arrLen; ++i) {
        if (arr[i] == number) isNumberPresent = true;
    }
    return isNumberPresent;
}

void process(const lint arr[], lint arrLen, lint newArr[], lint &newArrLen, lint m, lint M) {
    newArrLen = 0;
    for (lint i = M; i >= m; --i) {
        if (!isNumberInArray(arr, arrLen, i)) {
            newArr[newArrLen] = i;
            ++newArrLen;
        }
    }
}

int main() {
    lint arrLen, m, M;

    cout << "Enter m\n> ";
    cin >> m;
    cout << "Enter M\n> ";
    cin >> M;
    cout << "Enter array length\n> ";
    cin >> arrLen;

    lint *arr = new lint[arrLen];

    cout << "Enter array elements:\n";
    input(arr, arrLen);

    lint *newArr = new lint[arrLen], newArrLen;

    process(arr, arrLen, newArr, newArrLen, m, M);

    cout << "\nResults:\n";
    output(newArr, newArrLen);

    delete[] arr;
    delete[] newArr;
    return 0;
}

When I compile it using MSVC (x86 | Debug) and run it, it normally works and produces the desired result, but after execution shows the following error:

I tried to compile a program under WSL using g++ and debug it with Valgrind. Here is what I got:

root@seiba-laptop : /mnt/c/Users/saber-nyan/source/repos/Project1/Project1
[130] # g++ ./main.cpp -O0 -ggdb -o ./main

root@seiba-laptop : /mnt/c/Users/saber-nyan/source/repos/Project1/Project1
[0] # valgrind ./main
==316== Memcheck, a memory error detector
==316== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==316== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==316== Command: ./main
==316==
==316== error calling PR_SET_PTRACER, vgdb might block
Enter m
> 1
Enter M
> 5
Enter array length
> 2
Enter array elements:
arr[0] = 4
arr[1] = 2
==316== Invalid write of size 8
==316==    at 0x1093A4: process(long long const*, long long, long long*, long long&, long long, long long) (main.cpp:34)
==316==    by 0x1094E4: main (main.cpp:57)
==316==  Address 0x4d60560 is 0 bytes after a block of size 16 alloc'd
==316==    at 0x483850F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==316==    by 0x1094BA: main (main.cpp:55)
==316==

Results:
newArr[0] = 5
newArr[1] = 3
==316== Invalid read of size 8
==316==    at 0x1092B7: output(long long*, long long) (main.cpp:18)
==316==    by 0x10950A: main (main.cpp:60)
==316==  Address 0x4d60560 is 0 bytes after a block of size 16 alloc'd
==316==    at 0x483850F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==316==    by 0x1094BA: main (main.cpp:55)
==316==
newArr[2] = 1
==316==
==316== HEAP SUMMARY:
==316==     in use at exit: 0 bytes in 0 blocks
==316==   total heap usage: 5 allocs, 5 frees, 74,784 bytes allocated
==316==
==316== All heap blocks were freed -- no leaks are possible
==316==
==316== For counts of detected and suppressed errors, rerun with: -v
==316== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

What is the problem and how to fix it?

don't use C-style arrays in C++. Don't use raw pointers that own memory in C++. Don't use explicit memory allocation in C++. You seem to write a C program sparkled with some C++ `cin` / `cout`. Use standard containers and, if needed, smart pointers. — bolov, Mar 08 '19 at 08:17
@bolov If I could, I would do that - but, unfortunately, this is a laboratory work, and using arrays in the C style is said in the task. — saber-nyan, Mar 08 '19 at 08:19
When you get such an error assume your *code* is wrong, not the compiler. In any case you aren't supposed to use raw pointers or `new` in C++. Use `lint newArr[arrLen]` to define an array at least, or use `std::vector` which will actually worn you when you try to write beyond its boundaries. Use smart pointers like `unique_ptr` instead of `new` and `delete` — Panagiotis Kanavos, Mar 08 '19 at 08:20
@saber-nyan then don't ask about *C++* or use C++ constructs like `cin`. Tag the question with `c`. `lint newArr[arrLen]` works in C too — Panagiotis Kanavos, Mar 08 '19 at 08:21
@saber-nyan in any case, nothing prevents you from using `std::vector` to find the problem and then replacing it with a C-style array. Just don't hand-in the code with `vector` — Panagiotis Kanavos, Mar 08 '19 at 08:23
As an aside, you might want to give this link to your instructor: [Stop teaching C.](https://www.youtube.com/watch?v=YnWhqhNdYyk) By "forcing" you to learn "the C way" of doing things first, he's putting you through lots of unnecessary misery. The whole point of C++ is making these things (manual memory management etc.) *go away*. — DevSolar, Mar 08 '19 at 08:28
@DevSolar You misunderstood what is meant in this video. It is an advice not to try teaching C++ starting teaching the C part of C++; not "stop teaching C" in general. — Jean-Baptiste Yunès, Mar 08 '19 at 08:31
@Jean-BaptisteYunès: I very much did *not* misunderstand that video; as you can read from my comment, I am *talking* about learning C++. "Stop learning C" is the title of the lecture picked by the lecturer, Kate Gregory. That it should not be constructed as a bashing of learning C is what she is talking about in the first few minutes. But the OP is setting out to learn C++, and his instructor is holding him back by forcing him to use the C subset of the langugage. I wanted to point out the falacy in that, and Mrs. Gregory does a much better job at explaining than I could. — DevSolar, Mar 08 '19 at 08:36
@DevSolar What is the most fun, I already know Java and Python and, in principle, could study C++ myself, if that was interesting to me. — saber-nyan, Mar 08 '19 at 09:14
@saber-nyan: I'd say that would be better for you, and would love to recommend a good [textbook](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) for you, but unfortunately I haven't had one in hand yet that followed Kate Gregory's advice, so I cannot make recommendations from own experience. (I learned it "the hard way" back in my days.) But give that advice to your instructor nevertheless. He/She's a multiplier, and *should* know better... — DevSolar, Mar 08 '19 at 09:23

Jean-Baptiste Yunès · Accepted Answer · 2019-03-08T08:28:42.860

What I understand is that you try to construct a new array containing values not present in the original one. In your case you first built [4,2] and tried to build [5,3,1] because values in between M=5,m=1 not present in [4,2] are [5,3,1].

Your problem is that you first built newArr as an array of length 2 (the same length as arr). But you can't put 3 values in an array of size 2.

==316== Invalid write of size 8
==316==    at 0x1093A4: process(long long const*, long long, long long*, long long&, long long, long long) (main.cpp:34)
==316==    by 0x1094E4: main (main.cpp:57)
==316==  Address 0x4d60560 is 0 bytes after a block of size 16 alloc'd
==316==    at 0x483850F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==316==    by 0x1094BA: main (main.cpp:55)

Invalid write of size 8 means you tried to write a long long int at some bad address.

Address 0x4d60560 is 0 bytes after a block of size 16 alloc'd means that valgrind detected that your bad write was at the end of an memory chunk of size 16, exactly the size of an array of two long long ints.

vonamedog · Answer 2 · 2019-03-08T16:57:26.583

The problem is that your newArr does not necessarily have enough space to hold all the long long ints; That depends on the values of M and m which determine how many iterations would the for loop in process(...) go through and the values stored in arr.

To fix it, in this line here:

lint *newArr = new lint[arrLen], newArrLen;

Instead of arrLen amount of new lints, allocate (M - m + 1).

You should also make sure that M > m

Heap corruption in simple C++ program

2 Answers2