Bug 904 – Bad code generated for local _assert routine

Given the code: extern (C) void _d_assert( char[] file, uint line ); void call_assert( char[] file, uint line ) { _d_assert( file, line ); } void main() { call_assert( "a", 1 ); assert( false ); } The code generation for call_assert and the automatically generated _assert routine should be identical, but they aren't: _D4test11call_assertFAakZv comdat assume CS:_D4test11call_assertFAakZv L0: push EAX push dword ptr 0Ch[ESP] push dword ptr 0Ch[ESP] call near ptr __d_assert add ESP,0Ch ret 8 _D4test11call_assertFAakZv ends _D4test8__assertFiZv comdat assume CS:_D4test8__assertFiZv L0: push EAX push dword ptr FLAT:_DATA[01Ch] push dword ptr FLAT:_DATA[018h] call near ptr __d_assert ret _D4test8__assertFiZv ends You'll notice that the implicitly generated _assert routine doesn't clean up its stack on exit. This is fine for the normal case where an exception is thrown from assert(), but any attempt to override _d_assert to behave otherwise will resunt in an access violation.

The _d_assert() should never return, so there is no need to clean up after it. It's a compiler support routine, and should not be overridden with something that returns.

But surely that is no reason to generate invalid code. What if the user wants to halt the debugger on an assert via int 3, and optionally continue afterwords? Or report assertion failures during unit testing without littering the code with try/catch blocks? I am concerned because this change broke code that has been in place and working for two years, and there seems no rational explanation for the change.

It also seems to contradict: http://www.digitalmars.com/techtips/unittests.html "Provide a custom implementation of: extern (C) void _d_assert(char[] filename, uint line); to do the logging. _d_assert is the function called when an assert trips. By providing your own, it prevents the Phobos library version from being linked in."

It's not invalid code. The function is never supposed to return, therefore, it doesn't need any cleanup code. Replacing it with a function that does return will break use of asserts that assume that failed asserts don't continue. Changing behavior globally is often a problem because of 3rd party code linked in that assumes the defined behavior. Inserting your own logging code doesn't change this, it should log and then not return. The optimizer assumes tripped asserts don't return, so having it return will introduce some subtle bugs. The local assert function has always been like this, it was probably a fluke that it appeared to work in the past. Logging errors and continuing is not what assert is for, so something else should be used for that purpose.

[email protected] wrote: > > It's not invalid code. The function is never supposed to return, therefore, it > doesn't need any cleanup code. Replacing it with a function that does return > will break use of asserts that assume that failed asserts don't continue. > Changing behavior globally is often a problem because of 3rd party code linked > in that assumes the defined behavior. This is a valid point. I do think there is some value in providing a returning assert handler for debugging purposes, but I won't press the matter. However, I remain somewhat concerned that limiting the behavior of the assert handler in this manner will inspire the use of custom error signaling routines and reduce the overall utility of assert, but perhaps this is unwarranted. > The optimizer assumes tripped asserts don't return, so having it return will > introduce some subtle bugs. Valid as well. I suppose I'll leave the ability to supply a custom assert handler in place and simply document that it must either throw or terminate the program.