dolphinsmalltalk / Dolphin

Dolphin Smalltalk Core Image
MIT License
304 stars 58 forks source link

Calling a C-runtime library function with an invalid parameter crashes Dolphin #59

Closed blairmcg closed 8 years ago

blairmcg commented 8 years ago

From MSDN. "The behavior of the C Runtime when an invalid parameter is found is to call the currently assigned invalid parameter handler. The default invalid parameter invokes Watson crash reporting, which causes the application to crash and asks the user if they want to load the crash dump to Microsoft for analysis." This is inconvenient, since a small mistake crashes the dev image or app, and inconsistent with the usual behavior when calling other external APIs from Dolphin, which is generally that a the error is detected and a Smalltalk exception gets raised. The parameter validation errors should be translated to a Smalltalk exception (perhaps a CRTError), so that the issue can be handled in Smalltalk in an appropriate way. The simplest fix for this would be to replace the standard parameter validation function with one that does nothing, which will cause the called CRT function to perform old-style C-runtime error reporting (typically set errno to EINVAL, and return whatever form of error return value it uses). This could be done in the VM, or the image. This fix will require that any call sites detect and handle the error appropriately, although they need to do this anyway for other potential error conditions. A better fix might be to fail the external library call and override #invalidCall on CRTLibrary so as to translate the invalid parameter information into an appropriate Smalltalk exception in a standard way. the advantage of this would be consistent and automatic behaviour for invalid parameter errors. This will require both VM and image side modifications.

blairmcg commented 8 years ago

As it's always fun to show off Dolphin's powerful external interfacing capabilities, here is a demo of how this can be fixed on the Smalltalk side alone. First file in this:

!CRTLibrary methodsFor!

_set_invalid_parameter_handler: anExternalCallback 
    <cdecl: void _set_invalid_parameter_handler ExternalCallback*>
    ^self invalidCall! !
Smalltalk at: #CrtInvalidParameterHandler put: (ExternalCallback block: [:exp :func :file :line :rsvd | CRTError signal] argumentTypes: 'LPWSTR LPWSTR LPWSTR sdword uintptr')!
CRTLibrary default _set_invalid_parameter_handler: CrtInvalidParameterHandler asParameter!

Now test it out by evaluating this:

CRTLibrary default _close: -1

You should get a "Bad File Descriptor" walkback. Essentially the above has registered a Smalltalk block as the error handler for invalid parameters, where the block just throws a CRTError pulling the errno that the CRT _close function has just set to EBADF. You can set a breakpoint in the callback block, but note that all the diagnostic params will be null unless you are running on a debug CRT (i.e. with a debug VM build).

However, on further investigation I think the best fix for this is to handle it like access violation faults, and to register an invalid parameter handler early on in the VMs startup, and then to send an interrupt from the VM into Smalltalk when the handler is called. This will leave the bad call on the stack, without a load of extra callback frames on top to confuse matters.

blairmcg commented 8 years ago

Fixed as of e89c445