Back to Coding Horrors

How does this ever work.

Coding Horrors Community

Started by ApochPiQ December 06, 2016 11:13 PM

13 comments, last by fastcall22 6 years, 8 months ago

ApochPiQ

23,138

Author

December 06, 2016 11:13 PM

This is my favorite (read: least favorite) thing from the past couple of weeks.

It occurs only when running under Application Verifier, which is annoying, because it's masking another bug that I want to catch using AppVerifier, but I can't, because this crash starts happening first.

Here's the lovely code responsible (only slightly paraphrased):

ImportantHandle::ImportantHandle (void * rawData, unsigned sizeOfData) {
    if (sizeOfData) {
        this->ImportantPointer = rawData;
        // Plus some inconsequential stuff
    }
}

Spot the bug!

That's right: if you pass in a zero-size "blob", you get an undefined (aka. garbage) value in your ImportantPointer. What could go wrong!!!

The saddest part is, this is just one of many bugs in the program in question. Sometimes, it will just freeze in an infinite loop trying to reacquire a lost DirectX device, failing, and trying again without changing any state.

Other times, it'll crash in random areas with heap corruption - the bug I'm trying to chase - but not reliably, without AppVerifier. And of course AV causes it to barf in a dozen other creative ways instead.

It really is a minor miracle that I'm not yet bald.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

Nypyren

12,313

December 06, 2016 11:43 PM

It really is a minor miracle that I'm not yet bald.

My theory is that our hair has evolved to use blinding rage as a growth medium.

ApochPiQ

23,138

Author

December 07, 2016 12:55 AM

This code is a total gold mine.

Just found a place where we allocate N bytes, then placement-new an object of < N bytes into the buffer. No big deal, except we then access the buffer as if it were an object of... N bytes.

If you run with the debug allocator, the N bytes are zero-initialized. If you run under appverifier (or in Release, you know, just like you would for any software that's to be... released) the space between the small overlay object and the remainder of the N-byte buffer is garbage.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

Servant of the Lord

33,739

December 07, 2016 01:30 AM

by 'if(sizeOfData)', did you mean 'if(rawData)'?

i.e. you were checking that the pointer wasn't null, but not that it was of non-zero size?

ApochPiQ

23,138

Author

December 07, 2016 01:31 AM

No.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

conquestor3

1,592

December 07, 2016 03:20 PM

FWIW I've worked 4 years in a very high stress position and went bald in my mid/late 20's. It's not so bad.

Sounds like you need to re-implement whatever was being tried there.

Zaoshi Kaba

8,470

December 07, 2016 08:10 PM

FWIW I've worked 4 years in a very high stress position and went bald in my mid/late 20's. It's not so bad.

That's why I'm not looking forward to any promotions. I just want to remain yet-another-programmer.

ApochPiQ

23,138

Author

December 08, 2016 12:32 AM

Here's a juicy tidbit for posterity:

When running under Application Verifier, calls to HeapAlloc with the HEAP_ZERO_MEMORY flag will lie to you silently and return you NON-ZEROED pages.

This means that if you have any behavior relying on HEAP_ZERO_MEMORY, it will start blowing up in spectacular ways under AppVerifier. For extra fun, make sure the expected-to-be-zero data is used as a bitfield, preferably one indicating what array members are valid.

Days like this make me wish I had the discipline necessary to learn a completely different profession.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

samoth

9,833

December 08, 2016 12:06 PM

Have you considered using Dr.Memory? This is basically a comprehensive (read as: no WTF) alternative to Valgrind with some extras which also works under Windows. I've found it to be immensely, stunningly helpful.

Although strange "dunno why" crashes where I cannot immediately pinpoint the reason are kinda rare for me (luckily, not having to deal with someone else's code of such a kind as the one you posted!), whenever they do happen, Dr.Memory's output looks something like:


15 possible errors (but probably false positive), see possible-blahblah.log
1 possible leak (but probably harmless), see possible-blahblah.log
2 leaks, blah blah, see blah. log
1 error in line source.cpp, line 12345 : blah blah
 
crash in blah.exe at 12345678 : blubb.cpp line 456789, accessing blah, 2 bytes
 
5 system handles not closed, blah blah, see blah.log

... which basically means 2 minutes spent fixing it, not 2 days. No PhD needed to decipher the output either.

Sik_the_hedgehog

3,003

December 09, 2016 08:09 PM

by 'if(sizeOfData)', did you mean 'if(rawData)'?

i.e. you were checking that the pointer wasn't null, but not that it was of non-zero size?

I think the check was that if the size is zero, then the data doesn't exist (hence no pointer is needed).

Don't pay much attention to "the hedgehog" in my nick, it's just because "Sik" was already taken =/ By the way, Sik is pronounced like seek, not like sick.

How does this ever work.

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

How does this ever work.

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines