Segfault when converting vector into vector>

Question

Good day. I was wondering if I could get some help on this. I've got the following:

#include <vector>
#include <complex>
#include <iostream>

using messageScalar = std::complex<double>;
using messageVector = std::vector<messageScalar>;
using messageMatrix = std::vector<messageVector>;

class Tester {
 public:
  Tester(messageVector t) {
      messageMatrix container(1, t);
      messages = &container;

  }

  Tester(messageMatrix t) {
      messages = &t;

  }
  void debug() {
      std::cout << (*messages).size() << std::endl;
      for (auto &vector: *messages) {  // <- Debugger # 1
          for (auto &scalar: vector) {  // <- Debugger # 2

              std::cout << scalar << std::endl;
          }
      }
  }

 private:
  messageMatrix *messages = nullptr;

};

int main() {
    messageMatrix cMatrix = {{1, 2, 3}, {3, 4, 5}};
    Tester first(cMatrix);
    first.debug();
}

At the very end, I'm getting a segfault on this. It's telling me that I have 2 entries (which I expect - the number of "rows") but it's not clear to me why the segfault is happening.

2
18446744072976776191

Process finished with exit code 139 (interrupted by signal 11: SIGSEGV)

When using the debugger at #1, I get back a variable of "this" where I try to open it and get back

Cannot access memory at address X

and when I move the debugger to the next line it apparently has more entries than I've put in (3) for that row.

Am I missing something obvious? I've also tried doing an

Tester(messageVector t) {
      messageMatrix container;
      container.emplace_back(t);
      messages = &container;

  }

but that didn't work either (not that it should but I'm going out of my mind)

`messages` is pointing to a local variable who's destroyed at the end of the constructor's scope. — Stephen Newell, Jan 03 '21 at 00:27
Nothing that you've tried is valid C++. Objects declared locally in any constructor, or a class method, or a function get destroyed when the constructor/method/function gets returned. Saving a pointer to it, push_back-ing a pointer to it, none of that makes any difference. C++ does not work this way. [A good C++ textbook](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) should give you a more complete explanation and tutorial on the differences between automatically and dynamically scoped objects in C++. — Sam Varshavchik, Jan 03 '21 at 00:29
Is there a quick fix or do I need to restructure everything? — IanQ, Jan 03 '21 at 00:30
Very few problems in C++ have a "quick fix". You need to properly understand how object lifetimes work in C++. Also, only you know exactly when the objects in your program need to get created, what owns them, and when they get destroyed. There is no universal rule for this that all C++ programs must follow. It all depends on each C++ program's unique requirements and functionality. You need to properly define the object lifetimes for all objects in your C++ program, and implement them accordingly. — Sam Varshavchik, Jan 03 '21 at 00:35

Christophe · Accepted Answer · 2021-01-03T00:46:42.917

1

The problem

The problem is in your constructors, see comments:

  messageMatrix container(1, t);   // <--- container is a local variable
  messages = &container;           // <--- you take the address of the local variable
}                                  // <--- you leave constructor: local is destroyed

Or for the second constructor:

  messages = &t;                   // <--- t is a parameter
}                                  // <--- you leave constructor, t is destroyed

In both cases, once the construction is over, messages points to a destroyed object. Dereferencing it is UB. Anything can happen.

A solution

To make the code more robust, the simplest way is to just clone the message matrix:

class Tester {
 public:
  Tester(const messageVector &t) : messages(1,t) {
  }

  Tester(const messageMatrix &t) : messages(t) {
  }

  void debug() {
      std::cout << messages.size() << std::endl;
      for (const auto &vector: messages) {  // <- Debugger # 1
          for (const auto &scalar: vector) {  // <- Debugger # 2
              std::cout << scalar << std::endl;
          }
      }
  }

 private:
  messageMatrix messages; // local copy 

};

edited Jan 03 '21 at 00:46

answered Jan 03 '21 at 00:34

Christophe

68,716
7
72
138

Ahh, thank you! I was trying to keep messages as a pointer though (to check for nullptr). Is that possible? I'll also need to try it out and see if this method works for messageScalar types as well – IanQ Jan 03 '21 at 00:46
1

@IanQuah it is possible to keep a pointer, but you must make sure that 1) you use the address of the original object; 2) that the original object exists and is not moved during the existence of Tester; 3) that the original object getes destroyed once it is no longer needed but not before Tester is destroyed. The case of the messageVector makes this tricky, because you'd need to construct a temporary matrix. For this you could use a shared_ptr, but you'd no longer work with the original vector (i.e. if it changes, the temporary matrix is not updated). – Christophe Jan 03 '21 at 00:53
Sorry, could you elaborate on how I'd use shared_ptr? Everythin else you're saying( address of original, original not moved, original not destroyed before) makes sense to me. Having said that, it's not clear to me how and why a shared_ptr would help? Also, it's totally cool if changes to the original are not reflected in the temporary – IanQ Jan 03 '21 at 01:28
@IanQuah see [shared_ptr](https://en.cppreference.com/w/cpp/memory/shared_ptr) and [make_shared](https://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared) for using shared pointers. See [rule of 3](https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)) about some hints why shared pointer are preferable to raw pointers. If it's ok not to follow changes to the original, the non-pointer version is much safer. From the point of view of performance, your version would anyway make 1 or 2 copies of the original data. The proposed alternative makes just one. – Christophe Jan 03 '21 at 02:04

Segfault when converting vector into vector>

1 Answers1

The problem

A solution