ERROR: AddressSanitizer: heap-buffer-overflow on address X at pc Y bp Z sp W
I'm pretty new to dynamic memory management and using Fsanitise flag to find problems with memory management. I cannot use vector to store data - I need to use primitive arrays, as well as "new" and "delete" to manage the heap objects.
I got the following error when I try to run the EuclideanVectorTester compiled program but not sure what the problem is, will someone enlighten me please?
weill % make
g++ -std=c++14 -Wall -Werror -O2 -fsanitize=address -c EuclideanVectorTester.cpp
g++ -std=c++14 -Wall -Werror -O2 -fsanitize=address -c EuclideanVector.cpp
g++ -fsanitize=address EuclideanVectorTester.o EuclideanVector.o -o EuclideanVectorTester
weill % ./EuclideanVectorTester
1
=================================================================
==15341==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb59007e0 at pc 0x8048ca7 bp 0xbfb47388 sp 0xbfb4737c
WRITE of size 8 at 0xb59007e0 thread T0
#0 0x8048ca6 in main (/tmp_amd/kamen/export/kamen/3/z3386180/cs6771/evec/EuclideanVectorTester+0x8048ca6)
#1 0xb6ecae45 in __libc_start_main (/lib/i386-linux-gnu/i686/cmov/libc.so.6+0x16e45)
0xb59007e0 is located 0 bytes to the right of 16-byte region [0xb59007d0,0xb59007e0)
allocated by thread T0 here:
#0 0xb722a4c4 in operator new[](unsigned int) (/usr/lib/libasan.so.1+0x524c4)
#1 0x8048b9a in main (/tmp_amd/kamen/export/kamen/3/z3386180/cs6771/evec/EuclideanVectorTester+0x8048b9a)
#2 0xb6ecae45 in __libc_start_main (/lib/i386-linux-gnu/i686/cmov/libc.so.6+0x16e45)
#3 0x8048d8c (/tmp_amd/kamen/export/kamen/3/z3386180/cs6771/evec/EuclideanVectorTester+0x8048d8c)
SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 main
Shadow bytes around the buggy address:
0x36b200a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b200b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b200c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b200d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b200e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x36b200f0: fa fa fa fa fa fa fa fa fa fa 00 00[fa]fa 04 fa
0x36b20100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b20110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b20120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b20130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36b20140: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Contiguous container OOB:fc
ASan internal: fe
==15341==ABORTING
weill %
The EuclideanVector.h file is this:
#ifndef _EuclideanVector_h
#define _EuclideanVector_h
#include <iostream>
#include <algorithm>
namespace even {
class EuclideanVector {
public:
/*
* A constructor that takes the number of dimensions (as an unsigned int) but no magnitudes,
* sets the magnitude in each dimension as 0.0. This is the default constructor, with the default value being 1.
*/
template <typename NUM>
EuclideanVector(const NUM dimensions = 1):
EuclideanVector(dimensions, 0.0) {}; // delegating constructor; default constructor that takes in dimensions if there is user input, otherwise dimensions = 1 if it is an empty constructor
// target constructor for delegating constructor
template <typename NUM1, typename NUM2> // any numeric types of user input for dimensions and magnitude will be static_cast to unsigned int and double respectively
EuclideanVector(const NUM1 dimensions, const NUM2 magnitude){
// static cast to unsigned int and assign dimensions_ to that
dimensions_ = new unsigned int (static_cast<unsigned int>(dimensions));
// assign pointer "magnitude_" to dynamically-allocated memory of new unnamed array<double> object of size "dimensions_"
magnitude_ = new double [*dimensions_];
// fill the array<double> object "magnitude_" a number of "dimensions_" times, with the <double> value of "magnitude_" for each dimension
std::fill_n(magnitude_, dimensions_, static_cast<double>(magnitude));
}
/*
* Destructor: ~EuclideanVector
* The destructor deallocates any memory acquired by the constructors.
*/
~EuclideanVector();
/*
* Member function: getMagnitude()
* Returns a double containing the number of dimensions in a particular array.
*/
const double& getMagnitude () const;
/*
* Member function: getNumDimensions()
* Returns an unsigned int containing the number of dimensions in a particular vector.
*/
unsigned int getNumDimensions() const;
private:
/* Everything from here to the end of the class is private, so
* not accessible or intended for use for the client */
unsigned int *dimensions_;
double *magnitude_;
//double normal_;
};
}
#endif
The EuclideanVector.cpp file is this:
#include "EuclideanVector.h"
#include <algorithm>
#include <cmath> // for sqrt
#include <sstream>
#include <iterator>
namespace even {
unsigned int EuclideanVector::getNumDimensions () const
{
return *dimensions_;
}
const double& EuclideanVector::getMagnitude () const
{
return *magnitude_;
}
// destructor
EuclideanVector::~EuclideanVector() {
delete dimensions_;
delete [] magnitude_;
}
}
and the EuclideanVectorTester.cpp file is this:
#include <iostream>
#include <vector>
#include <list>
#include "EuclideanVector.h"
int main() {
std::cout << "1" << std::endl;
evec::EuclideanVector a(2);
std::cout << "2" << std::endl;
std::cout << "3" << std::end;
}
Answer
Let me just say that what your professor states has a lot of inaccuracies (to say the least).
Having said that, the issue is that you've declared dimensions to be an unsigned int*, but yet you're using it as if it is a plain unsigned int here:
std::fill_n(magnitude_, dimensions_, static_cast<double>(magnitude));
The immediate fix is to do this:
std::fill_n(magnitude_, *dimensions_, static_cast<double>(magnitude));
However, that begs the question of why a simple unsigned int needs to be a pointer, and then allocated using new. There is no reason for this, as using new is less efficient than what you're doing now.
If you had declared dimensions_ as this:
unsigned int dimensions_;
instead of it being a pointer, then the code to assign to dimensons_ becomes this:
// no call to new is done
dimensions_ = static_cast<unsigned int>(dimensions);
// no dereference of a pointer needs to be done on the two lines below
magnitude_ = new double[dimensions_];
std::fill_n(magnitude_, dimensions_, static_cast<double>(magnitude));
No additional call to the allocator is done, thus the code instantly becomes more efficient.
In addition, the destructor now would look like this:
EuclideanVector::~EuclideanVector()
{
delete [] magnitude_;
}
But even with all of this stated, the answer given only fixes the issue if you use that very same main program to test with. If you changed main to this:
even::EuclideanVector a(2);
even::EuclideanVector b = a;
You will now run into copy semantics being incorrect. So again, the fix above is only required to make your main function work. Change main to something very simple as the above example, and you run into more issues.