11

In my physically based renderer, I'm getting a memory corruption bug (the program crashes, and the debugger gives a bogus stack trace that's worthless). I traced it down to this SSCCE. The line with the constructor seems to be what triggers the error:

#include <cstdint>

class Foo final {
    public:
        uint8_t packed;

    public:
        inline Foo(void) : packed(0xFF) {} //causes error
        inline ~Foo(void) = default;
};
static_assert(sizeof(Foo)==sizeof(uint8_t),"Implementation error!");

int main(int /*argc*/, char* /*argv*/[]) {
    Foo* arr = new Foo[4]; //Tried a bunch of different sizes.  All fail.
    delete [] arr;

    return 0;
}

The problem does not occur for MSVC or GCC, only Intel Compiler (the version of which is 16.0). But, since this is a memory corruption bug, that doesn't really mean anything. Before I submit a bug report, can someone confirm this is not me misusing C++?

Here is a premade solution demonstrating the problem. Here is the assembly.

geometrian
  • 14,775
  • 10
  • 56
  • 132
  • 1
    Can you provide more detail on the memory corruption - is it every value, is there any kind of pattern to the corruption, does it crash your application or just provide weird data, etc? I don't see anything that should cause memory corruption, but I am not an expert in bit field declarations in C++ either. As long as your static_assert is true, then your class should be packing the bits correctly. Also, can you convert the bit-packed struct to non-packed ints and verify that it doesn't error? – Matt Jordan Mar 17 '16 at 16:02
  • Do you also get corruption if you allocate less the 32767 elements, maybe just 10 or 20 ? I suppose you get the memory corruption error during the `delete`?. – Jabberwocky Mar 17 '16 at 16:11
  • @MattJordan I simplified the example drastically. Unfortunately, the memory corruption is just a crash. It's very difficult to debug because the debugger says nothing useful. – geometrian Mar 17 '16 at 16:30
  • @MichaelWalz I tried a bunch of different sizes, but none worked. In particular, I tried 1, 2, 3, and 4. Indeed, the problem manifests on the delete[]. – geometrian Mar 17 '16 at 16:31
  • @imallett very interesting. Indeed it could hardly be simpler. Is stepping through the assembly code an option? Try to remove the `inline` keyword (just a wild guess). If only everybody on SO would strip down their faulty down as you did.... – Jabberwocky Mar 17 '16 at 16:35
  • Did you try different compiler flags (optimized, debug build, etc.)? – Jabberwocky Mar 17 '16 at 16:37
  • @MichaelWalz removing `inline` had no effect. [Here](http://pastebin.com/ypVJpC6p)'s the preprocessed source; it's a lot more complex that I want to mentally decompile. Problem occurs with all of {x86,x86-64}*{debug,release}. – geometrian Mar 17 '16 at 16:55
  • 1
    The code as posted is perfectly valid C++ code (it has redundant but harmless `inline`s in it and `void` as function arguments, but this is not a problem). If icc-generated executable crashes here, it is a terrible bug in icc or a terribly broken icc installation. But are you really sure it crashes with a given code? – SergeyA Mar 17 '16 at 16:55
  • @SergeyA Yeah; this seems like such a simple/obvious/serious bug that I wanted a sanity check. If anyone has the compiler, [here's a premade solution](http://www.mediafire.com/download/nh9162ubfgrdxoo/union_init_bug.zip) I would appreciate them testing. – geometrian Mar 17 '16 at 17:01
  • @imallett, sorry, but I just don't see any reason to pay for the compiler anymore. This puts icc out of my reach once and for all. – SergeyA Mar 17 '16 at 17:03
  • @SergeyA [sidenote] Me too. But as a grad student, Intel provides it for free. – geometrian Mar 17 '16 at 17:06
  • 1
    Just a wild guess: rename the variable `packed` –  Mar 17 '16 at 17:18
  • Can you ask the compiler to generate assembly code? If the compiler generates buggy code, we should be able to spot it right away in such a simple program. – Sergei Tachenov Mar 17 '16 at 17:28
  • @SergeyTachenov I gave the assembly previously, above. I have now added to the question that link and another containing a test solution, so they are more obvious. – geometrian Mar 17 '16 at 17:47
  • You should probably mention the word “assembly”. I saw that “preprocessed source” and thought it just the code that went through the C++ preprocessor. Which isn't of any use here, obviously, so I didn't even looked at it. – Sergei Tachenov Mar 17 '16 at 17:49
  • @SergeyTachenov Ah blech you're right. I understand the difference, obviously, but for some reason I fairly consistently write "preprocessed" instead of "assembled". Fixed. – geometrian Mar 17 '16 at 17:51
  • Does the assembly correspond to exactly this code, with `Foo[4]` and not some other size? Just to confirm. – Sergei Tachenov Mar 17 '16 at 18:16
  • The assembly code doesn't seem to be valid - in main, it is subtracting 64 from esp (stack pointer) to allocate local storage, but then subtracting 76 from a copy of the previous esp (ebp) as the base address to initialize; it looks to me like it is messing with stack that will be used by any functions it calls, although I haven't dug enough to tell what it is doing with locations esp-76 to esp-64, but that seems suspect. – Matt Jordan Mar 17 '16 at 18:26
  • @SergeyTachenov I'm not sure; it might have been from a slightly earlier test. I have regenerated the assembly ([same link](http://pastebin.com/ypVJpC6p)) so now I'm sure they are in 1:1 correspondence. Sorry if I screwed up! – geometrian Mar 17 '16 at 18:33
  • Ah, now it sort of makes sense. Now it passes 4 to the array constructor, not 3. Not that it helps to figure out the problem, though. – Sergei Tachenov Mar 17 '16 at 18:36
  • I don't understand one thing in the assembly: it allocates 8 bytes (7 bytes in the previous version), calls the constructor for the last 4 bytes (no clue what the first 4 bytes are for). Then it calls `delete` for the address starting with the 4th byte, not the address that was returned by `new`. Maybe it's just something i don't know, though, as I'm no expert. – Sergei Tachenov Mar 17 '16 at 19:10

1 Answers1

4

As established in the comments, over a series of increasingly simpler examples (and corresponding edits), this is perfectly valid C++ code.

I posted a bug report on Intel's developer forum, and it has been officially confirmed as such.

geometrian
  • 14,775
  • 10
  • 56
  • 132