Segmentation fault - reading array of initialized pointers

Question

could you help me with little problem?

I have following class;

class Link
{
  private:
    Demand *demand_[NUMBER_OF_CORES][NUMBER_OF_SLICES];

  public:
    Link()
    {
      for (int i = 0; i < NUMBER_OF_CORES; i++)
      {
        for (int j = 0; j < NUMBER_OF_SLICES; j++)
        {
          demand_[i][j] = NULL;
        }
      }
    }

    int virtualPut();
}

There will be problem with demand_ array. In the constructor everything is fine, after initialization I can use if (demand_[i][j] == NULL).

Problem starts in virtualPut()

int Link::virtualPut()
{
  for (int i = 0; i < NUMBER_OF_CORES; i++)
  {
    for (int j = 0; j < NUMBER_OF_SLICES; j++)
    {
      std::cout << "We're in " << i << " " << j << " \n" << std::flush;

      if (demand_[i][j] == NULL)  //SEGMENTATION FAULT
      {
        std::cout << "EMPTY\n";
      }
    }
  }
}

And also - if I call virtualPut() in constructor (just for test) it works fine.

But outside Link class I use.

void someFunction(Link *tab, int links)
{
  tab = new Link[links];

  tab[0].virtualPut();  //also just for test
}

What could be a problem here? I know that I can use vector, but that won't help me understand this memory problem.

One more thing - Dr. Memory says:

UNADDRESSABLE ACCESS: reading 0x0000000000000009-0x0000000000000011 8 byte(s)

But why?

EDIT! Problem solved in comments, thank you

Where do you initialize `demand_` pointer? From your code it's uninitialized — Alexey Andronov, Oct 23 '18 at 09:21
But I declared it in static way - shouldn't compilator reserve memory for whole `Demand *demand_[NUMBER_OF_CORES][NUMBER_OF_SLICES];`? — mrozo, Oct 23 '18 at 09:25
In constructor I initialize whole array as NULL. Cannot I then check if `demand_[x][y]` is NULL or not? — mrozo, Oct 23 '18 at 09:28
Within `someFunction`, `tab` is a `Link*`. Try `tab = new Link(); tab.virtualPut();` — Robert Kock, Oct 23 '18 at 09:34
Are you sure you re not calling `virtualPut` after your call to `someFunction` instead of inside? Because you clearly are not allocating `tab` and returning the new object. — Matthieu Brucher, Oct 23 '18 at 09:35
@RobertKock it's still a pointer, so [0]. works as well as using ->. — Matthieu Brucher, Oct 23 '18 at 09:36
@RobertKock Yes, but I need an array of `Link`'s. During `tab = new Link[links];` a constructor is called for every object, since it is a default one — mrozo, Oct 23 '18 at 09:37
compiler allocated memory only for one uninitialized pointer(`demand_`), you need to allocate the rest NUMBER_OF_CORES*NUMBER_OF_SLICES of `Demand` pointers yourself — Alexey Andronov, Oct 23 '18 at 09:38
@mrozo Your `someFunction` is strange. You're passing a pointer to `Link` as the first argument, changing it within the function, but the caller to `someFunction` will never receive those changes due to the pointer being passed by value. Methinks there are other errors in your code that you're not showing us that is causing the issue. Please post a [mcve]. Also, since `Link` is a class, it requires that the `Link` object itself be valid for anything to work properly, including the `demand_` member. We have no idea if `Link` is valid or not. — PaulMcKenzie, Oct 23 '18 at 09:41
Has nothing to do with your problem, but the function should probably be declared as `someFunction(Link*& tab, int links)` — Robert Kock, Oct 23 '18 at 09:56
@MatthieuBrucher I was sure i tested it in that way too, but I guess I didn't recompile everything. @PaulMcKenzie - spot on! I failed with understanding pointers. But you're right, when I'm using `new()` I'm getting a new address for allocated memory, but since parameter wasn't a reference I didn't get an update for caller. [link](https://stackoverflow.com/questions/33243436/array-initialization-functions) — mrozo, Oct 23 '18 at 10:04
Emm, I'm kinda new here - should I post answer down below or just remove whole topic? — mrozo, Oct 23 '18 at 10:07
I agree that it's not clear what context you get a problem in (you mention that virtualPut() in constructor is fine, and show a function someFunction where its fine (I think?), but never show the context of a use case where it's not fine), and second the call for a MCV example. — jwimberley, Oct 23 '18 at 11:46

mgueydan · Answer 1 · 2018-10-23T11:58:26.863

The code you show us is okay. I ran it on my side with huge values, and there is no Segfault. You declared a "Demand* array of array" in your Link class, and this is a valid declaration, the memory should be allocated.

What I do suspect is that NUMBER_OF_CORES and/or NUMBER_OF_SLICES doesn't have the same value in the code where you defines the Link class and in the code where you defined the virtualPut method.

Something like :

#define NUMBER_OF_CORES 10
#define NUMBER_OF_SLICES 10
class Link
{
private:
    Demand *demand_[NUMBER_OF_CORES][NUMBER_OF_SLICES];
...
}

and

#define NUMBER_OF_CORES 5000
#define NUMBER_OF_SLICES 5000
int Link::virtualPut()
{
    for (int i = 0; i < NUMBER_OF_CORES; i++)
    {
        for (int j = 0; j < NUMBER_OF_SLICES; j++)
        {
        // here you will have buffer overflow
    ...
}

What I would do :

use std::vector
probably use a single entry array, and wrap it up
don't use #define, it's messy
don't use arrays, it generates buffer overflow

That would be something like this :

class Link
{
private:
    std::vector<Demand*> demand_;
    const int NUMBER_OF_CORES = 10;
    const int NUMBER_OF_SLICES = 50;
private:
    int getIdx(int i, int j)
    {
        return i*NUMBER_OF_SLICES + j;
    }
public:
    Link()
    {
        demand_.resize(NUMBER_OF_CORES * NUMBER_OF_SLICES);
        for (int i = 0; i < NUMBER_OF_CORES; i++)
        {
            for (int j = 0; j < NUMBER_OF_SLICES; j++)
            {
                demand_[getIdx(i,j)] = NULL;
            }
        }
    }
    int virtualPut();
};

Note : additionaly, you showed us a virtualPut() that should return an int but doesn't.

I'm not sure using vector is a good idea. I know what size of array I need. All `#defined` values are in one file, so It's also fine. My problem was solved in comments to the post, but thank you for your answer. — mrozo, Oct 23 '18 at 15:49
I think there is a missunderstanding. You should use std::vector instead of array, even if you know the array size. Just because it's much safer. If you worry about performances, it's the very same thing. You may please refer to https://stackoverflow.com/questions/381621/using-arrays-or-stdvectors-in-c-whats-the-performance-gap — mgueydan, Oct 25 '18 at 08:07

Segmentation fault - reading array of initialized pointers

1 Answers1