Hey,

I had some time again to play around with 3D noise.
And while doing so, I had multiple crashes in PolyVox.

Or, rather, I WISH I had crashes, so I could jump in with gdb and debug them. Instead, the application just quits. No chance to find out where the error actually came from.


Is something that IMO should never, never, never, never, never be used in a library. It is assuming that the user wants to quit the application if there is an error. Something which is very dangerous (as it removes the possibility to do proper debugging) in your own application, and even more hazardous in an external library such as PolyVox.

A crash is far more helpful than an application quitting silently.

On some occasions, I could find that something like "NaN after normalize" (don't remember exactly) was printed to the console, together with the file in which it happened, but no trace, variable values, etc.
But it did not really help me figuring out where the actual problem was.

So, if PolyVox knows that something will not work out, printing that knowledge is fine and helpful, but silently quitting the application instead of letting it crash is something I actually consider very harmful.

_________________
My site! - Have a look
Also on Twitter - with more ketchup

Yep, I agree with you here. The code in question is in ErrorHandling.h:



As you can see it does something sensible on Windows (goes into the debugger so you can examine the callstack, etc) but I think I just didn't know the equivalent for Linux. However a quick bit of Googling has led me to this:

http://stackoverflow.com/a/173656

So probably on Linux it should use '__builtin_trap()' instead of std::exit(EXIT_FAILURE). If you can test this and let me know if it works then I'll commit it to PolyVox. Failing that it could just use a divide-by-zero of null pointer dereference to force a crash.

As for the root cause of your problem I have seen similar crashes when running the MarchingCubesSurfaceExtractor. It seems it calls 'normalise()' on a vector of length zero so I need to fix this.

If it continues to be a problem you can comment out the relevant POLYVOX_ASSERT() (it should print a filename and line number when it crashes/exits), or you can also disable the asserts completely in the following file:

https://bitbucket.org/volumesoffun/poly ... elop#cl-27

Added an issue here: https://bitbucket.org/volumesoffun/poly ... debugbreak

Is the problem, I guess.
I am using MinGW, which naturally does not define _MSC_VER. Getting rid of MSVC is one of the main points of using MinGW
_WIN32 would be a more fitting flag, I think.

I don't know if MinGW has __debugbreak(), though.
It certainly has DebugBreak().

Don't know about Linux, though, sorry
Also don't know about clang.
So it is possible that this whole thing should depend not on the platform, but on the compiler.

_________________
My site! - Have a look
Also on Twitter - with more ketchup

It is possible, but I think some testing would be needed of the various configurations to really find out. I don't really want to do this (some platforms I don't use myself) so I think the safest option is to fall back on a crash. Then if people want to test better options for per-platform or per-compiler then they can do this, and submit a patch.

Ok, it now looks like this:



Hopefully that is ok for most situations, but if people want to test and submit platform/compiler specific solutions then we can also include them.

Having a quick look around, it seems many people suggest I mostly got this from a thread on SO. When run outside a debugger, it will just crash but inside GDB it will act like a breakpoint. I've not tested it at all but it's likely to be better than a bare segfault.

_________________
Matt Williams
Linux/CMake guy

why don't you simply use an exception?
an exception only introduces overhead when thrown and it already has all the features you want (like breakpoint & crash with useful text in release)
+ it can be caught.

Yep, this one is also possible. But I guess it's Linux/Unix specific (rather than being GCC specific) which means it's dependant on a runtime test to check which platform we are on, rather than a compiler check.

But you're welcome to add some better platform/compiler specific solutions as long as the fallback is in place. Actually the fallback is also sub-optimal as some platforms actually allow the write to address '0'. I tried doing a divide by zero instead but Visual Studio catches it at compile time.



Hmmm.... this is actually an interesting suggestion. The code we are discussing above is called by the POLYVOX_ASSERT macro (our alternative to assert() which also works in release builds) and both asserts and exceptions have a valid place in an error handling system.

If you are saying that in some places we should throw an exception rather than asserting then I think you are right, and attempting to normalise a zero-length vector is a good example of this. There are places in PolyVox where we throw exceptions but overall the usage is not consistent and we do need to fix this.

However, I think what you are actually saying is that we could implement the POLYVOX_ASSERT macro by having it throw an exception, and this would then cause the crash into the debugger. I hadn't thought about this and it seems a bit unconventional, but I see the idea discussed here so it could be worth investigating: http://stackoverflow.com/a/37474

One additional point on the subject of exceptions is that they are not supported on all platforms. At least my Android NDK did not support them but I think they are available in newer versions. For now, exception throwing is wrapped in the POLYVOX_THROW macro so that it can be disabled.

I've just committed some changes to the way errors are handled in PolyVox. Basically I've replaced a number of asserts with exceptions so that the application doesn't need to stop in these situations.

You should also read this addition to the user manual: https://bitbucket.org/volumesoffun/poly ... at=develop