Mysterious error in this C++ pointer program in visual studio

Question

I have a written code for finding missing elements below.

#include <iostream>

int main() {
    int arr[7] = { 1,3,5,7,9,11,13 };
    int max{ arr[0] }, min{ arr[0] };
    for (int i = 0; i < 7; i++) {
        if (max < arr[i])
            max = arr[i];
        if (min > arr[i])
            min = arr[i];
    }
    int* ptr = new int[max];
    for (int j = 0; j < 7; j++)
        ptr[arr[j]] = 1;
    
    for (int k = min; k < max; k++)
        if (ptr[k] == 0)
            std::cout << k << " ";

    delete[] ptr;

    return 0;
}

However this gives me an mysterious error about heap corruption like this :

enter image description here

I don't see any problem here. Anyone see it ? Please tell me.

You forgot to initialize the dynamic array; `new int[max]()` will zero-fill it. — molbdnilo, Feb 09 '21 at 08:18
Yes but, the uninitialized array still contains only garbage integer values right ? How does it give HEAP CORRUPT error ? — Bhuvansai, Feb 09 '21 at 08:20
`max` is 13, so you have 13 elements. What happens when `arr[j]` is 13? — molbdnilo, Feb 09 '21 at 08:20
When this window popups you should click Retry button and debug your program. Accessing arrays out of bounds or reading garbage values is Undefined Behavior. It may lead to heap corruption or even to [time travel](https://devblogs.microsoft.com/oldnewthing/20140627-00/?p=633). — user7860670, Feb 09 '21 at 08:21
@Bhuvansai While your problem is the `max` being `13` and the size is only `13`, I would like to address also your `the uninitialized array still contains only garbage integer values right`: [In C++, is accessing an uninitialized array unspecified behavior or undefined behavior?](https://stackoverflow.com/questions/49696810), [Is reading an indeterminate value undefined behavior?](https://stackoverflow.com/questions/4279264). You should be careful in assuming that reading indeterminate (or as you say garbage) values, is unproblematic. — t.niese, Feb 09 '21 at 08:31

score 3 · Accepted Answer · answered Feb 09 '21 at 08:20

3

max is 13.
So ptr's length is also 13.

int* ptr = new int[max];
for (int j = 0; j < 7; j++)
    ptr[arr[j]] = 1;

this is accessing ptr[13], which is out of bounds.

answered Feb 09 '21 at 08:20

김선달

1,485
9
23

Paul Floyd · Answer 2 · 2021-02-09T10:34:05.127

The output from Valgrind for this is

==24471== Invalid write of size 4
==24471==    at 0x4012E3: main (so2.cpp:14)
==24471==  Address 0x4e57cb4 is 0 bytes after a block of size 52 alloc'd
==24471==    at 0x480B57F: operator new[](unsigned long) (vg_replace_malloc.c:431)
==24471==    by 0x4012B0: main (so2.cpp:12)
==24471==
==24471== Conditional jump or move depends on uninitialised value(s)
==24471==    at 0x401315: main (so2.cpp:17)

(obviously you won't be able to use Valgrind on Windows).

The first error is because you are using 1-based indices in arr but allocating a 0-based array in ptr. This can be fixed by accessing ptr with a 0-based index. (You could make the ptr array 1 element larger, but that is not the idiomatic way of doing things in C++, so I would not recommend that you do this.)

Also as noted in the comments, your code does not initialize the ptr array.

Here is a modified version of your code that is Valgrind-clean, with my comments where there are changes.

#include <iostream>

int main() {
    int arr[7] = { 1,3,5,7,9,11,13 };
    int max{ arr[0] }, min{ arr[0] };
    for (int i = 0; i < 7; i++) {
        if (max < arr[i])
            max = arr[i];
        if (min > arr[i])
            min = arr[i];
    }
    int* ptr = new int[max](); // zero initialized
    for (int j = 0; j < 7; j++)
        ptr[arr[j]-1] = 1; // 0-based index
    
    for (int k = min-1; k < max; k++) // k starts with a 0-based index
        if (ptr[k] == 0)
            std::cout << k << " ";

    delete[] ptr;

    return 0;
}

I haven't adjusted the cout statement to use a 1-based index. It's likely that you would want to output k+1.

I've only suggested fixes for the issues present. I would recommend writing things completely differently:

#include <iostream>
#include <array>
#include <vector>
#include <algorithm>

int main() {
    std::array<int, 7> arr{ 1,3,5,7,9,11,13 };
    int min = *std::min_element(std::begin(arr), std::end(arr));
    int max = *std::max_element(std::begin(arr), std::end(arr));
    std::vector<int> vec(max);
    for (auto j : arr)
        vec[j-1] = 1;
    
    for (int k = min-1; k < max; k++)
       if (vec[k] == 0)
          std::cout << k << " ";
}

Only 1 magic number, fewer loops, using standard algorithms and containers, no use of raw pointers and manual resource control.

thanks :) . Iam new to programming, so i only know little bit about importing classes and using their functions. — Bhuvansai, Feb 09 '21 at 12:38

Mysterious error in this C++ pointer program in visual studio

2 Answers2