C++ Error, Visual Studio 2013 Bug?

[patrickjp93]

I created this trivial little column sum program as part of a basic into to optimization of one's code under various frameworks. I was then going to use it to show the weaknesses and strengths of various C++ compilers. The issue is it runs fine under G++, Clang++, and ICPC. On Visual Studio it crashes saying the vector indices are out of bounds. Am I missing something totally obvious after being away from vectors and arrays for a long time, or is this a Visual Studio issue like I think it is?

 

#include <iostream>#include <vector>#include <stdexcept>#include <sstream>#include <chrono>#include <omp.h>using namespace std::chrono;void fill_vector(std::vector<std::vector<short>>& data, unsigned int rows,	unsigned int cols) {	data.reserve(rows);	#pragma omp parallel for	for (unsigned int i = 0; i < rows; i++) {		data[i] = std::vector<short>(cols);	}}std::vector<int> col_sums(const std::vector<std::vector<short>>& data) {	unsigned int height = data.size(), width = data[0].size();	std::vector<int> totalSums(width, 0), threadSums(width, 0);	#pragma omp parallel firstprivate(threadSums)	{		#pragma omp for nowait		for (unsigned int i = 0; i < height; i++) {			for (unsigned int j = 0; j < width; j++) {				threadSums[j] += data[i][j];			}		}		#pragma omp critical		{			for (unsigned int i = 0; i < width; i++) {				totalSums[i] += threadSums[i];			}		}	}	return totalSums;}int main(int argc, char** argv) {	high_resolution_clock::time_point t1 = high_resolution_clock::now();	if (argc < 3) {		std::cout << "Run program as \"executable <rows> <columns>\n";	}	else {		std::stringstream args;		args << argv[1] << " " << argv[2];		unsigned int rows, columns;		args >> rows >> columns;		std::vector<std::vector<short>> data;		fill_vector(data, rows, columns);		std::vector<int> columnSums = col_sums(data);	}	high_resolution_clock::time_point t2 = high_resolution_clock::now();	duration<double> time_span = duration_cast<duration<double>>(t2 - t1);	std::cout << "Total elapsed time: " << duration_cast<seconds>(t2 - t1).count() << ":";	std::cout << duration_cast<microseconds>(t2 - t1).count() << ":";}

 

under /usr/bin/time -v ./dummy 115000 20000, I get no error messages or exits with any abnormal signals/codes whether compiled under G++/Clang++/ICPC nor any warnings from the -Wall -Wextra flags. I'm stumped.

 

@LukaP, got any ideas?

[fizzlesticks]

vector.reserve only allocates the memory you ask it for, it doesn't change the size of the vector. So if the current size is 0 even after you call reserve the size is still 0, when you do data where i=0 you get an out of range error because element 0 doesn't exist yet. In your case you should probably be using vector.resize instead of reserve.

 

I don't know why it works on the other compilers but it shouldn't

[patrickjp93]

vector.reserve only allocates the memory you ask it for, it doesn't change the size of the vector. So if the current size is 0 even after you call reserve the size is still 0, when you do data where i=0 you get an out of range error because element 0 doesn't exist yet. In your case you should probably be using vector.resize instead of reserve.

 

I don't know why it works on the other compilers but it shouldn't

Not true, Please read cppreference. If your vector size is smaller than the proposed n, the capacity and size are changed, which allows pointer attachments such as in the given implementation and the cppreference example code. Unless I'm somehow misinterpreting this, it seems pretty clear, and I'd expect system crashes at least on my Macbook through XCode, but that's not happening either.

 

http://en.cppreference.com/w/cpp/container/vector/reserve

 

Increase the capacity of the container to a value that's greater or equal to new_cap. If new_cap is greater than the current capacity(), new storage is allocated, otherwise the method does nothing.

If new_cap is greater than capacity(), all iterators and references, including the past-the-end iterator, are invalidated. Otherwise, no iterators or references are invalidated.

[Ciccioo]

Not true, Please read cppreference. If your vector size is smaller than the proposed n, the capacity and size are changed, which allows pointer attachments such as in the given implementation and the cppreference example code. Unless I'm somehow misinterpreting this, it seems pretty clear, and I'd expect system crashes at least on my Macbook through XCode, but that's not happening either.

 

http://en.cppreference.com/w/cpp/container/vector/reserve

it works in the sample code because they use push_back(). it would be valid without even using reserve().

 

if you reserve() storage you can't use it until you resize() the vector, or perform another action that actually extends the size() of the vector.

the operator[] doesn't perform bound checking so i think you're just running undefined behaviour. you should get an exception if you use the at() method, which does perform bounds checking.

 

edit: the storage is actually allocated so it makes sense te me that on some compilers that code runs. VS is probably more strict, which is good because now you will avoid relying on UB

[patrickjp93]

it works in the sample code because they use push_back(). it would be valid without even using reserve().

 

if you reserve() storage you can't use it until you resize() the vector, or perform another action that actually extends the size() of the vector.

the operator[] doesn't perform bound checking so i think you're just running undefined behaviour. you should get an exception if you use the at() method, which does perform bounds checking.

 

edit: the storage is actually allocated so it makes sense te me that on some compilers that code runs. VS is probably more strict, which is good because now you will avoid relying on UB

More strict? The lowest performing of the big compilers that supports the least of the C++11 standard? Maybe I'm misunderstanding that language, but okay. Poor choice of words I guess.

 

And technically it's not undefined behavior, because the data structure knows exactly where those pointers go. It should be no different than using large arrays. The problem is these days you can't use large arrays due to buffer overflows and stack smashing concerns.

[manon_gfx]

What is happening is that reserve allocates enough memory for the data you are putting in, but doesn't change the size of the vector. When you access the vector, it should give an error (visual studio is giving you correct behavior), but if there is no error checking, it will go out of bounds for you. Which is no problem since you are only really accessing already allocated memory.

If you run compile your program with debug settings in G++ or Clang++, it should give you those errors.

The correct way is to reserve, and then push_back all the elements, or call resize, and set them like you are doing right now.
Calling reserve is faster, since resize initializes everything to it's default values.


EDIT: I looked up the exception safety of std::vector on cplusplus.com, and this is what it says:
 

"Exception safety

If the container size is greater than n, the function never throws exceptions (no-throw guarantee).
Otherwise, the behavior is undefined."

This means that not crashing is also correct behavior, since the compiler is allowed to do whatever it likes.

[patrickjp93]

What is happening is that reserve allocates enough memory for the data you are putting in, but doesn't change the size of the vector. When you access the vector, it should give an error (visual studio is giving you correct behavior), but if there is no error checking, it will go out of bounds for you. Which is no problem since you are only really accessing already allocated memory.

If you run compile your program with debug settings in G++ or Clang++, it should give you those errors.

The correct way is to reserve, and then push_back all the elements, or call resize, and set them like you are doing right now.

Calling reserve is faster, since resize initializes everything to it's default values.

 

Even with -g on clang++/g++/icpc I get no thrown errors. XCode doesn't throw any either with its inherent debugging settings.

 

It seems we need a happy medium then, because building the vector serially only wastes time. Resizing shouldn't be setting default values imho, but w/e.

 

Secondary question. I'm not getting text output in my command prompt when giving "start dummy.exe 115000 20000" and giving the extra "> out.txt" isn't capturing the couts either. Why is Windows so incredibly convoluted? Why execute the command in a separate window instead of in-line? ARGH!

[trag1c]

What others said about std::vector is correct. It doesn't resize until you use vector::push_back or vector::resize.

[patrickjp93]

What others said about std::vector is correct. It doesn't resize until you use vector::push_back or vector::resize.

It's truly asinine how Microsoft is the only one who makes a stink out of it when in truth I should be able to just get my memory without setting everything to 0 which only wastes time. Now if I could just figure out why my standard out won't redirect to a file...

 

"start dummy.exe 115000 20000 > out.txt" gives me a file with nothing in it.

[fizzlesticks]

It's truly asinine how Microsoft is the only one who makes a stink out of it when in truth I should be able to just get my memory without setting everything to 0 which only wastes time. Now if I could just figure out why my standard out won't redirect to a file...

 

"start dummy.exe 115000 20000 > out.txt" gives me a file with nothing in it.

If you run in release mode your original code "works" in VS too, but that doesn't mean you should do it.

As for the file thing try leaving out the "start"

[patrickjp93]

If you run in release mode your original code "works" in VS too, but that doesn't mean you should do it.

As for the file thing try leaving out the "start"

Same results with no shown outputs on screen or in the file. This is why I grew to hate the Windows platform with a passion. Nothing works the intuitive way or even the mostly logical ways. And yes it's a console application.

 

EDIT: tried including the string class, still no results.

EDIT2: Double checked the linker, and the subsystem is console, so that's not the issue.

EDIT3: If I run through the built-in debugger, I get text output, but now I'd need to figure out to to put in command line arguments.

EDIT4: Upon adding the 115000 and 20000 to the command line arguments in the debugging section of the project properties, I get a bad alloc exception given exactly below and the source provided by the Windows debugger. Apparently MSVC is horrifically wasteful with its memory or Windows is too constraining to allow data sets of certain sizes to be generated instantaneously. Just one more thing to add to reasons I despise Microsoft.

 

Unhandled exception at 0x76C3C42D in dummy.exe: Microsoft C++ exception: std::bad_alloc at memory location 0x004FFD5C.

 

/****new.cxx - defines C++ new routine**       Copyright (c) Microsoft Corporation.  All rights reserved.**Purpose:*       Defines C++ new routine.********************************************************************************/#ifdef _SYSCRT#include <cruntime.h>#include <crtdbg.h>#include <malloc.h>#include <new.h>#include <stdlib.h>#include <winheap.h>#include <rtcsup.h>#include <internal.h>void * operator new( size_t cb ){    void *res;    for (; {        //  allocate memory block        res = _heap_alloc(cb);        //  if successful allocation, return pointer to memory        if (res)            break;        //  call installed new handler        if (!_callnewh(cb))            break;        //  new handler was successful -- try to allocate again    }    RTCCALLBACK(_RTC_Allocate_hook, (res, cb, 0));    return res;}#else  /* _SYSCRT */#include <cstdlib>#include <new>_C_LIB_DECLint __cdecl _callnewh(size_t size) _THROW1(_STD bad_alloc);_END_C_LIB_DECLvoid *__CRTDECL operator new(size_t size) _THROW1(_STD bad_alloc)        {       // try to allocate size bytes        void *p;        while ((p = malloc(size)) == 0)                if (_callnewh(size) == 0)                {       // report no memory                        _THROW_NCEE(_XSTD bad_alloc, );                }        return (p);        }/* * Copyright (c) 1992-2002 by P.J. Plauger.  ALL RIGHTS RESERVED. * Consult your license regarding permissions and restrictions. V3.13:0009 */#endif  /* _SYSCRT */ 

[Ciccioo]

More strict? The lowest performing of the big compilers that supports the least of the C++11 standard? Maybe I'm misunderstanding that language, but okay. Poor choice of words I guess.

 

And technically it's not undefined behavior, because the data structure knows exactly where those pointers go. It should be no different than using large arrays. The problem is these days you can't use large arrays due to buffer overflows and stack smashing concerns.

it might be a couple nanoseconds slower than other compilers, but this far it's the only compiler that showed you that your code is bad/poorly written/undefined behavioured/not standard savy/error prone/crash friendly/straight up wrong. that's very valuable, it's making you learn stuff.

 

technically it is undefined behaviour, as it is clearly stated in the documentation. i get it, you think that it is a good idea to make it work anyway, but there is a reason if it's not like that, and your beloved standard states that you cannot use memory that you only reserve()d. go ahead and make your own c++ compiler if you think you can do smarter things.

 

Secondary question. I'm not getting text output in my command prompt when giving "start dummy.exe 115000 20000" and giving the extra "> out.txt" isn't capturing the couts either. Why is Windows so incredibly convoluted? Why execute the command in a separate window instead of in-line? ARGH!

the "start" command has one job: to open a new window. now go wonder why it opens up a new window.

if you take away the "start" it works. tested. if it doesn't, you're probably doing something wrong. windows is probably doing just fine.

[patrickjp93]

it might be a couple nanoseconds slower than other compilers, but this far it's the only compiler that showed you that your code is bad/poorly written/undefined behavioured/not standard savy/error prone/crash friendly/straight up wrong. that's very valuable, it's making you learn stuff.

 

technically it is undefined behaviour, as it is clearly stated in the documentation. i get it, you think that it is a good idea to make it work anyway, but there is a reason if it's not like that, and your beloved standard states that you cannot use memory that you only reserve()d. go ahead and make your own c++ compiler if you think you can do smarter things.

 

the "start" command has one job: to open a new window. now go wonder why it opens up a new window.

if you take away the "start" it works. tested. if it doesn't, you're probably doing something wrong. windows is probably doing just fine.

Nanoseconds? It's produces code on average 40% slower than GCC and up to 75% slower than ICC. That's nothing to turn your nose up at. Carmack and several other big wigs in the gaming industry have all begun showing up at Microsoft presentations and showing how bad it's getting.

 

I removed the start and still get nothing. 

dummy.exe 115000 20000

 

Interestingly enough if I provide no arguments, I get nothing, but under the debugger I get the expected error text and the time taken under the clocks. However when I give it those arguments, I get a memory allocation error and can't reach the end to get my timing anyway. I have 12GB of RAM. I have plenty of room for this, and the memory allocation works under Linux just fine with this version of the program. I'm doing nothing wrong. Windows is just built poorly, and so is Visual Studio. Now, if you care to take the snideness out of your comments and actually not assume I'm stupid, that would be more helpful.

[fizzlesticks]

 

EDIT4: Upon adding the 115000 and 20000 to the command line arguments in the debugging section of the project properties, I get a bad alloc exception given exactly below and the source provided by the Windows debugger. Apparently MSVC is horrifically wasteful with its memory or Windows is too constraining to allow data sets of certain sizes to be generated instantaneously. Just one more thing to add to reasons I despise Microsoft.

 

115000 * 20000 * sizeof(short) is a big number, you're going over the 32 bit limit. Make a new configuration for 64bit. If you stop instantly jumping to the conclusion that Microsoft is terrible you might be able to figure some stuff out.

[patrickjp93]

115000 * 20000 * sizeof(short) is a big number, you're going over the 32 bit limit. Make a new configuration for 64bit. If you stop instantly jumping to the conclusion that Microsoft is terrible you might be able to figure some stuff out.

You do realize this only supports my point... It should be in 64-bit mode anyway! Legacy support is one of the big things holding the software industry back. Supporting 32-bit mode by default is insane these days. Fine, off to figure out how to get around another poorly implemented feature. If they wanted a 32-bit option on a 2013 IDE, it should have been the secondary option, not the primary.

 

EDIT: Thank you. The program works under the debugger and start without debugging, and now I have my proof. Even with all optimization flags set to maximize speed, even this trivial program runs at 1/2 the speed it runs under GCC even with all optimization flags set to value speed over memory. That's actually worse than I was expecting. That said, I still can't find out why the executable won't print to the standard command prompt when calling in the local directory 

"dummy 115000 20000" In fact it even seems the correct version of the executable may not be running based on the time.