-1

I'm dealing with someone else's code as a part of my assignment and have ran into trouble. Instead of running smoothly, the given code throws out the mentioned error in the following function:

template <typename T>
inline T ***Create3DArray(int d1, int d2, int d3) {
   T ***retval;

   retval = (T***)malloc((sizeof(T**)+(sizeof(T*)+sizeof(T)*d3)*d2)*d1);

   T **ptr = (T**)(retval+d1);
   T *ptr2 = (T*)(ptr+d1*d2);
   for(int i = 0; i < d1; i++, ptr += d2) {
      retval[i] = ptr; // this line triggers the CXX0030 error
      for(int j = 0; j < d2; j++, ptr2 += d3) {
        retval[i][j] = ptr2;
        if(j == 0) {
          for(int k = 0; k < d3; k++) 
             retval[i][j][k] = 0;
        } else
          memcpy(retval[i][j], retval[i][0], sizeof(T)*d3);
        }
      }
   return retval;
}

Any clues as to why this happens? I doubt that someone would publish their code if it couldn't even be ran. Is this maybe a Visual Studio specific issue?

Edit: I stress the fact that I'm not the one who has written the code, and have very little insight regarding the big picture (although the problem seems localized). Here's some more info, the line which calls the function Create3DArray is:

  float ***pts = Create3DArray<float>(classes->NumParts(), numObjects*m, part_width*part_width);

The arguments are 15, 11988 and 3136, meaning that over 1GB of memory gets allocated.

The link to the project's website is here. The file which I'm currently trying to use can be found under Examples->import_birds200.cpp. Do note that the whole thing is pretty big and uses some 1GB of data.

MrLinjak
  • 65
  • 7
  • I'm pretty sure this is not an issue of Visual Studio. – Matt Dec 23 '14 at 18:54
  • 1
    _@MrLinjak_ `T ***retval;` Congratulations, you're a 3 star template programmer now. (Do we have a hat for this?) – πάντα ῥεῖ Dec 23 '14 at 18:55
  • @πάνταῥεῖ I'd take some stars back because o the malloc() in c++ code, and the savage memcpy() to copy a row... – Christophe Dec 23 '14 at 19:07
  • @Christophe Meh! You're an ole grinch! :-( No hat for them?? – πάντα ῥεῖ Dec 23 '14 at 19:10
  • 1
    @Matt I'll take it that the error is pretty easy to figure out, judging by all those snide remarks. Like I said, the code is not mine, and I'm a pretty bad at c++ myself, so one serious answer would be much appreciated. – MrLinjak Dec 23 '14 at 19:21
  • @Christophe Could you please clarify what's wrong with the code? – MrLinjak Dec 23 '14 at 20:18
  • 1
    @MrLinjak I've just created an answer in which I try to explain. I've tried the corrected code version extensively, without getting any error. If the error persist in your code, please show us the code that calls the function (and the value of the three dimensions). – Christophe Dec 23 '14 at 20:26
  • 1
    @MrLinjak: Perhaps the person who wrote this code should be given the assignment instead. – Lightness Races in Orbit Dec 23 '14 at 21:04
  • @LightnessRacesinOrbit I have added the link to the project website. – MrLinjak Dec 23 '14 at 21:14

1 Answers1

1

1.You use of c functions that do not ensure c++ object lifecycle / semantics:

You use malloc() in c++. This is not a great practice, because malloc() doesn't initialize the objects in the allocated area. When you later assign an object, like in retval[i][j][k] = 0; your compiler assumes that retval[i][j][k] already contains an object which is in a stable state.

THis isn't the direct cause of your error. But from the second iteration onwards, depending on how T operator= is implemented, you could have corrupted memory.

If you want to proceed with malloc(), you have to use placement new to initialize the objects properly: new (&retval[i][j][k])T(); // placement creation

You later use memcpy(). This will performe a byte clone of your object, without ensuring the semantic of a copy. For example, if your type T would have member pointing to a memory region allocated during its construction, both the clone and the original would then point to the same region. The first who gets deleted will free the memory. The second will attempt to free again : memory issues guaranteed !

Prefer std::copy() over memcpy(). But this requires an existing object to first be constructed. So in your context, no way arround a placement new : get rid of the special case using the mmemcpy().

2. Memory allocation issues:

It may sound trivial, but allocation could fail. So it would be preferable to put an assert to verify that you didn't get a NULL in return !

I'm suggesting this because that's one of the probable cause of Cxx0030

Christophe
  • 68,716
  • 7
  • 72
  • 138
  • I have edited my question with some more info. Could you please share your version of the code? – MrLinjak Dec 23 '14 at 20:54
  • Please link to English language documentation. – Lightness Races in Orbit Dec 23 '14 at 21:05
  • @LightnessRacesinOrbit Corrected ! Sorry, but I didn't notice that the french address was kept even If I asked MS to display the original language ! – Christophe Dec 23 '14 at 21:31
  • 1
    @MrLinjak With a simple float you won't in practice encounter any of the issue I menstionned in 1. However, with 15*11988*3136 + the pointers, you try to allocate 2 256 381 420 bytes which is above the 2GB memory barrier. malloc() returns hence a null pointer. See also here: http://stackoverflow.com/questions/639540/how-much-memory-can-a-32-bit-process-access-on-a-64-bit-operating-system – Christophe Dec 23 '14 at 21:49
  • 1
    @Christophe The problem was big memory allocation, like you said. After switching to x64 configuration the thing was as slow as it gets, but managed to finish properly. Sadly, next piece of code is already throwing its' own exceptions at me, but the problem in the OP was solved. Thanks a lot. – MrLinjak Dec 24 '14 at 17:06