0

I'm new to C++, coming from a python/kotlin background and so am having some trouble understanding what's going on behind the scenes here...

The Issue

I call the calculateWeights (public) method with its required parameters, it then calls a series of methods, including conjugateGradientMethod (private) and should return a vector of doubles. conjugateGradientMethod returns the vector of doubles to calculateWeights just fine, but calculateWeights doesn't return that to its caller:

The code

Callsite of calculateWeights:

        Matrix cov = estimator.estimateCovariances(&firstWindow, &meanReturns);

        cout << "before" << endl; // this prints
        vector<double> portfolioWeights = optimiser.calculateWeights(&cov, &meanReturns);
        cout << "after" << endl; // this does not print

Here's calculateWeights:

vector<double> PortfolioOptimiser::calculateWeights
        (Matrix *covariances, vector<double> *meanReturns) {

    vector<double> X0 = this->calculateX0();
    Matrix Q = this->generateQ(covariances, meanReturns);
    vector<double> B = this->generateB0(); 

    vector<double> weights = this->conjugateGradientMethod(&Q, &X0, &B);

    cout << "inside calculateWeights" << endl;
    print(&weights); // this prints just fine
    cout << "returning from calculateWeights..." << endl; // also prints

    return weights; //this is where the SIGABRT shows up

The output

The output looks like this (I've checked and the weights it outputs are indeed numerically correct):

before
inside calculateWeights
1.78998
0.429836
-0.62228
-0.597534
-0.0365409
0.000401613
returning from calculateWeights...

And then nothing.

I appreciate this is printf debugging which isn't ideal and so I used Cion's debugger to find the following:

When I used CLion's debugger

I put a break point on the returns of the conjugateGradient method and calculateWeights methods. The debugger steppeed through the first one just fine. After I stepped over the return from calculateWeights, it showed me a SIGABRT with the following error:

Thread 1 "markowitzportfoliooptimiser" received signal SIGABRT, Aborted.
__gnu_cxx::new_allocator<std::vector<double, std::allocator<double> > >::deallocate (this=0x6, __p=0x303e900000762) at /usr/lib/gcc/x86_64-pc-cygwin/9.3.0/include/c++/ext/new_allocator.h:129

This is probably wrong but my first stab at understanding that is that I've overflowed over the size of vector<double> weights? It's only 6 doubles long and I never append anything to it after the loop below. This is how it's created inside conjugateGradientMethod:

How weights is created inside conjugateGradientMethod

    vector<double> weights= vector<double>(aSize);
    for (int i = 0; i < aSize; i++) {
        weights[i] = aCoeff * a->at(i) + bCoeff* b->at(i);
    }


Things I've tried

  1. Initialising a vector for double weights in calculateWeights and passed a pointer to it to conjugateGradientMethod. Same result.
  2. Having a public attribute on the class calculateWeights and conjugateGradientMethod both live on, and having it assign the weights to that (so both functions return void). Same result.

More generally, I've had this kind of issue before with passing up a return value from two functions deep. (If that makes sense?) ie passing from private method up to public method up to public method's callsite.

I'd be grateful for any advice on SIGABRT's in this context, I've read it's when abort() sends the calling process the SIGABRT signal, but am unsure how to make use of that in this example.

Also, I'm all ears for any other style/best practices that would help avoid this in future

Edit: Solution found

After much work, I installed and got Ubuntu 20.04 LTS up and running since I couldn't get the Address Sanitizer nor Valgrind to work via WSL on Windows 10 (first time on Linux - I kinda like it).

With Address Sanitizer now working, I was able to see that I was writing too many elements to a vector of doubles on two separate accounts, nothing to do with my weights vector as @Lukas Matena rightly spotted. Confusingly this was long before it ever got to the snippets above.

If anyone is finding this in the future, these helped me massively:

Heap Buffer Overflow

Heap vs Stack 1

Heap vs Stack 2

IndiP
  • 1
  • 2
  • I don't see anything wrong with those snippets. My guess would be stack corruption somewhere, in a way that happens to trigger an assertion failure that calls `abort()`. Try enabling [AddressSanitizer](https://en.wikipedia.org/wiki/AddressSanitizer) if your compiler supports it, or running under valgrind. – Thomas Jun 03 '20 at 15:48
  • it seems you are under a Linux/Unix, install *valgrind* and run your program with it, it will signal your invalid memory access/free/delete/... – bruno Jun 03 '20 at 15:50
  • The smart bet is that you have Undefined Behavior in your code. The first step, in that case... Do you have compiler warnings fully turned on and have you addresed them all? (And to the credit of a comment entered while I was typing, yes, run your program in valgrind and it will take you to your mistake) – Drew Dormann Jun 03 '20 at 15:50
  • Thanks all for your speedy replies. I'm running on a windows pc with g++ installled via cygwin. Am currently installing and figuring out valgrind for clion on windows. I've just turned on compiler warnings and the only ones I see are to do with unused variables [for functionality I was about to implement]. Thanks again all. – IndiP Jun 03 '20 at 16:09
  • Turns out the version of CLion (or more recent) I'm on has a bug that doesn't allow the WSL required for valgrind since I'm on a PC: https://youtrack.jetbrains.com/issue/CPP-11852 I am now onto Address Sanitiser... – IndiP Jun 03 '20 at 19:25

1 Answers1

1

The error message says that it failed to deallocate an std::vector<double> when calculateWeights was about to return. That likely means that at least one of the local variables (which are being destroyed at that point) in the function is corrupted.

You seem to be focusing on weights, but since the attempts that you mention have failed, I would rather suspect X0 or B (weights is maybe not even deallocated at that point due to return value optimization).

Things you can try:

  • start using an address sanitizer like others have suggested
  • comment out parts of the code to see if it leads you closer (in other words, make a minimal example)
  • make one of the vectors a member variable so it is not destroyed at that point (not a fix, but it might give a clue about who is the offender)

You're likely doing something bad to the respective vector somewhere, possibly in calculateX0 or generateB0 (which you haven't shared). It may be delete-ing part of the vector, returning a reference to a temporary instead of a copy, etc. The SIGABRT at that return is where you were caught, but memory corruption issues often surface later than they're caused.

(I would have made this shorter and posted as a comment but I cannot as a newbie. Hopefully it will count as an "advice on SIGABRT's in this context", which is what was in fact asked for).