Buffer Overrun using a Vector

Question

unsigned short* myClass::get_last(size_t _last) const
{
    if (_last == 0) _last = last.size();
    if (_last <= last.size())
    {
        unsigned short* temp = new unsigned short [_last] {};
        while (_last > 0) temp[_last] = last[last.size() - _last--]; //I get a warning HERE
        return temp;
    }
    throw std::runtime_error("Error!");
}

It says:

Buffer overrun while writing to 'temp': the writable size is '_last*2' bytes, but '_last' bytes might be written.

What does it mean? I know for sure that _last is not greater that temp.size() because of the if so what should I do?

It perfectly works at runtime, but I hate having warnings which make my code less understandable by other user or by me in the future.

EDIT: _last is an argument given by the user at runtime, so it could eventually have any value, but if his values are out of the range you get an exception (managed in another function).

The vector that I mentioned in the title is last, whis is a member of myClass.

I know that the elements of an array go from 0 to _last - 1, and this is why I decrement _last before using it the first time (as you probably know assignement associativity is right-to-left).

I hope I answered all your comments ;)

How are you using this function? What values could `_last` have? What is `last` and how is it initialized? Please try to create a [mcve] to show us. Also please take some time to read about [how to ask good questions](http://stackoverflow.com/help/how-to-ask), as well as [this question checklist](https://codeblog.jonskeet.uk/2012/11/24/stack-overflow-question-checklist/). — Some programmer dude, Jan 23 '20 at 14:28
`temp` is an array of `_last` elements, and you then write to `temp[_last]`. Are you aware the in C++ array indexes start from 0? Such that the valid indexes goes from 0 to `_last-1`? — Daniel Langr, Jan 23 '20 at 14:31
Why do I get the feeling that this code is doing something that could easily be done by using a standard library container? — PaulMcKenzie, Jan 23 '20 at 14:58
*It says:* -- What is **It**? The runtime library? A third-party tool? — PaulMcKenzie, Jan 23 '20 at 18:45
Then I think it's time to give us a [mcve]. Also, when I mean **it**, I am talking about where you got that error text "*Buffer overrun while writing to 'temp': the writable size is '_last*2' bytes, but '_last' bytes might be written*". That is not a standard piece of text from a compiler, so what tool is giving you this error? — PaulMcKenzie, Jan 23 '20 at 19:03
*and this is why I decrement _last before using it the first time* -- Why write that line of code in such a manner? What if you simply broke that up into two lines: `while (_last > 0) { temp[_last] = last[last.size() - _last]; --last;}`. Maybe the way you wrote that line is what is fouling things up. — PaulMcKenzie, Jan 23 '20 at 19:11
It is hard to reproduce a minimal reproducible example because there is a whole class which manages this method, but I can tell you that *last* is a `vector` containing some data, which are copied in reverse order into *temp* — Simo Pelle, Jan 23 '20 at 19:18
[See this](https://stackoverflow.com/questions/41943803/visual-studio-2015-code-analysis-c6386-warns-of-buffer-overrun). — PaulMcKenzie, Jan 23 '20 at 19:31
Thanks, I solved using Vectors like someone said on this article — Simo Pelle, Jan 24 '20 at 13:50

Goswin von Brederlow · Answer 1 · 2020-01-23T15:32:57.507

1

The problem is that C++ indexes arrays starting with 0. So an array of size 4 has valid indexes 0, 1,2 and 3.

But you are allocating an array of size _last:

unsigned short* temp = new unsigned short [_last] {};

and then writing to temp[_last]. That is one beyond the size of the array.

edited Jan 23 '20 at 15:32

answered Jan 23 '20 at 14:58

Goswin von Brederlow

11,875
2
24
42

I already verified that __last_ cannot be grater than _temp size_ here `if (_last <= last.size())` – Simo Pelle Jan 23 '20 at 17:15
Yes. But it must be SMALLER. Arrays go from 0 to size - 1. – Goswin von Brederlow Jan 27 '20 at 10:07

Simo Pelle · Accepted Answer · 2020-01-24T13:48:47.433

-1

Solved using Vectors!

std::vector<unsigned short> Tombola::get_last(size_t _last) const
{
    if (_last == 0) _last = last.size();
    if (_last <= last.size())
    {
        std::vector<unsigned short> temp(_last);
        while (_last > 0) temp[_last] = last[last.size() - _last--];
        return temp;
    }
    throw std::runtime_error("Error!");
}

For some reason they always solve all problems even if you don't know how ;)

edited Jan 24 '20 at 13:48

answered Jan 23 '20 at 18:43

Simo Pelle

141
1
11

This is not an answer, only a comment. – PaulMcKenzie Jan 23 '20 at 19:03
Also still has the buffer overflow except that vector will automatically resize as needed. Inefficient as you are allocating the vector twice. – Goswin von Brederlow Jan 27 '20 at 10:08
@GoswinvonBrederlow the fact is that it doesn't really have buffer overflow because I check its size at line 4, but the compiler does not realize that because I allocate my content on another vector, which has a size that will never be grater than *last* size – Simo Pelle Jan 27 '20 at 12:33

Buffer Overrun using a Vector

2 Answers2