Add null checks for field load/stores

[morganbr]

Loading or storing a field on a null object should throw a NullReferenceException (or trap). #6383 introduces a pattern for doing null checks that we should be able to use similarly for field operations. The operations that should do checks are:

• ldfld (implemented in ImportLoadField)
• ldflda (implemented in ImportAddressOfField)
• stfld (implemented in ImportStoreField)

[hippiehunter]

Something went wrong when I enabled null checking for field operations. I think its a real issue, but i don't really know whats going on in this stack trace.

Starting
Hello from C#!
basic block stack entry Test: Ok.
Inline assign byte Test: Ok.
dup test: Ok.
Instance method call test: Ok.
Virtual Slot Test: Ok
Virtual Slot Test: Ok If second
Virtual Slot Test 2: Ok
value type int field test: Ok.
static int field test: Ok.
thread static int field test: Ok.
BeforeFieldInit test: Ok.
NonBeforeFieldInit test: Ok.
box test: Ok.
Boxed Stub Test: Ok.
Subtraction Test: Ok.
Division Test: Ok.
not test: Ok.
negInt test: Ok.
shiftLeft test: Ok.
shiftRight test: Ok.
unsignedShift test: Ok.
SwitchOp0 test: Ok.
SwitchOp1 test: Ok.
SwitchOpDefault test: Ok.
CpObj test: Ok.
Static delegate test: Ok.
Instance delegate test: Ok.
Virtual Delegate Test: Ok
Hello
Array
Test
Array load/store test: Ok.
dup ref test: Ok.
Large array load/store test: Ok.
Small array load/store test: Ok.
Newobj value type test: Ok.
Stackalloc test: Ok.
Int to String Test: Ok if next line says 42.
42
interface call test: Ok 1
Type casting with isinst & castclass to class test: Ok.
Type casting with isinst & castclass to interface test: Ok.
Array
Cast
Test
warning: build with  -s DEMANGLE_SUPPORT=1  to link in libcxxabi demangling
exception thrown: abort() at jsStackTrace@http://localhost:6931/HelloWasm.js:1049:13
stackTrace@http://localhost:6931/HelloWasm.js:1066:12
abort@http://localhost:6931/HelloWasm.js:7175:44
_abort@http://localhost:6931/HelloWasm.js:5725:7
__Z6AssertPKcS0_jS0_@http://localhost:6931/HelloWasm.wasm:wasm-function[19573]:0xa14795
__ZN6Thread22SetupHackPInvokeTunnelEv@http://localhost:6931/HelloWasm.wasm:wasm-function[21127]:0xb0a1c5
_RhpGetCurrentThreadStackTrace@http://localhost:6931/HelloWasm.wasm:wasm-function[21270]:0xb11f8b
_S_P_CoreLib_System_Runtime_RuntimeExports__RhpGetCurrentThreadStackTrace@http://localhost:6931/HelloWasm.wasm:wasm-function[258]:0xa96eb
_S_P_CoreLib_System_Runtime_RuntimeExports__RhGetCurrentThreadStackTrace@http://localhost:6931/HelloWasm.wasm:wasm-function[257]:0xa962c
_RhGetCurrentThreadStackTrace@http://localhost:6931/HelloWasm.wasm:wasm-function[259]:0xa976c
_S_P_CoreLib_System_Diagnostics_StackTrace__InitializeForThreadFrameIndex@http://localhost:6931/HelloWasm.wasm:wasm-function[976]:0xf3ee0
_S_P_CoreLib_System_Diagnostics_StackTrace___ctor_2@http://localhost:6931/HelloWasm.wasm:wasm-function[506]:0xc3c99
_S_P_CoreLib_System_Diagnostics_Debug__Assert_1@http://localhost:6931/HelloWasm.wasm:wasm-function[443]:0xbbb61
_S_P_CoreLib_System_Diagnostics_Debug__Assert_0@http://localhost:6931/HelloWasm.wasm:wasm-function[185]:0xa4db8
_S_P_CoreLib_System_Collections_Concurrent_ConcurrentUnifierW_2_IntPtr__S_P_CoreLib_System_Type___GetOrAdd@http://localhost:6931/HelloWasm.wasm:wasm-function[846]:0xe6fab
_S_P_CoreLib_Internal_Reflection_Core_NonPortable_RuntimeTypeUnifier__GetTypeForRuntimeTypeHandle@http://localhost:6931/HelloWasm.wasm:wasm-function[1318]:0x1192ba
_S_P_CoreLib_System_Type__GetTypeFromHandle@http://localhost:6931/HelloWasm.wasm:wasm-function[702]:0xdac02
_S_P_Reflection_Execution_Internal_Reflection_Execution_ReflectionExecutionDomainCallbacksImplementation__GetBetterDiagnosticInfoIfAvailable@http://localhost:6931/HelloWasm.wasm:wasm-function[8203]:0x4ac04c
_S_P_CoreLib_System_RuntimeTypeHandle__get_LastResortToString@http://localhost:6931/HelloWasm.wasm:wasm-function[6798]:0x3dc986
_S_P_CoreLib_Internal_Runtime_Augments_RuntimeAugments__GetLastResortString@http://localhost:6931/HelloWasm.wasm:wasm-function[6091]:0x3580bc
_S_P_Reflection_Execution_Internal_Reflection_Execution_ExecutionEnvironmentImplementation__GetLastResortString@http://localhost:6931/HelloWasm.wasm:wasm-function[6090]:0x35803c
_S_P_Reflection_Core_System_Reflection_Runtime_General_Helpers__LastResortString@http://localhost:6931/HelloWasm.wasm:wasm-function[4304]:0x2869e3
_S_P_Reflection_Core_System_Reflection_Runtime_TypeInfos_RuntimeBlockedTypeInfo__ToString@http://localhost:6931/HelloWasm.wasm:wasm-function[4323]:0x287c8a
_S_P_Reflection_Core_System_Reflection_Runtime_TypeInfos_RuntimeTypeInfo__EstablishDebugName@http://localhost:6931/HelloWasm.wasm:wasm-function[2835]:0x1cc6f4
_S_P_Reflection_Core_System_Reflection_Runtime_TypeInfos_RuntimeBlockedTypeInfo__GetRuntimeBlockedTypeInfo@http://localhost:6931/HelloWasm.wasm:wasm-function[2382]:0x179044
_S_P_Reflection_Core_Internal_Reflection_Core_Execution_ExecutionDomain__GetNamedTypeForHandle@http://localhost:6931/HelloWasm.wasm:wasm-function[1941]:0x15674b
_S_P_Reflection_Execution_Internal_Reflection_Execution_ReflectionExecutionDomainCallbacksImplementation__GetNamedTypeForHandle@http://localhost:6931/HelloWasm.wasm:wasm-function[1940]:0x15638a
_S_P_CoreLib_Internal_Reflection_Core_NonPortable_RuntimeTypeUnifier_RuntimeTypeHandleToTypeCache__Factory@http://localhost:6931/HelloWasm.wasm:wasm-function[1548]:0x12c311
_S_P_CoreLib_System_Collections_Concurrent_ConcurrentUnifierW_2_IntPtr__S_P_CoreLib_System_Type___GetOrAdd@http://localhost:6931/HelloWasm.wasm:wasm-function[846]:0xe7489
_S_P_CoreLib_Internal_Reflection_Core_NonPortable_RuntimeTypeUnifier__GetTypeForRuntimeTypeHandle@http://localhost:6931/HelloWasm.wasm:wasm-function[1318]:0x1192ba
_S_P_CoreLib_System_Type__GetTypeFromHandle@http://localhost:6931/HelloWasm.wasm:wasm-function[702]:0xdac02
_S_P_CoreLib_System_SR___cctor@http://localhost:6931/HelloWasm.wasm:wasm-function[1711]:0x13d7e4
_S_P_CoreLib_System_Runtime_CompilerServices_ClassConstructorRunner__Call@http://localhost:6931/HelloWasm.wasm:wasm-function[368]:0xb5bd6
_S_P_CoreLib_System_Runtime_CompilerServices_ClassConstructorRunner__EnsureClassConstructorRun@http://localhost:6931/HelloWasm.wasm:wasm-function[325]:0xaefb3
_S_P_CoreLib_System_SR__InternalGetResourceString@http://localhost:6931/HelloWasm.wasm:wasm-function[980]:0xf4313
_S_P_CoreLib_System_SR__GetResourceString_0@http://localhost:6931/HelloWasm.wasm:wasm-function[503]:0xc34cc
_S_P_CoreLib_System_SR__get_Arg_NullReferenceException@http://localhost:6931/HelloWasm.wasm:wasm-function[449]:0xbc1fc
_S_P_CoreLib_System_NullReferenceException___ctor@http://localhost:6931/HelloWasm.wasm:wasm-function[248]:0xa8ed4
_corert_throwifnull@http://localhost:6931/HelloWasm.wasm:wasm-function[246]:0xa8ba6
_S_P_CoreLib_Internal_Runtime_EEType__get_Kind@http://localhost:6931/HelloWasm.wasm:wasm-function[461]:0xbd2ac
_S_P_CoreLib_Internal_Runtime_EEType__get_IsCloned@http://localhost:6931/HelloWasm.wasm:wasm-function[212]:0xa6b2a
_S_P_CoreLib_System_Runtime_TypeCast__AreTypesEquivalentInternal@http://localhost:6931/HelloWasm.wasm:wasm-function[182]:0xa49a6
_S_P_CoreLib_System_Runtime_TypeCast__AreTypesAssignableInternal@http://localhost:6931/HelloWasm.wasm:wasm-function[488]:0xc0f96
_S_P_CoreLib_System_Runtime_TypeCast_CastCache__CacheMiss@http://localhost:6931/HelloWasm.wasm:wasm-function[466]:0xbf247
_S_P_CoreLib_System_Runtime_TypeCast_CastCache__AreTypesAssignableInternal@http://localhost:6931/HelloWasm.wasm:wasm-function[193]:0xa531b
_S_P_CoreLib_System_Runtime_TypeCast__IsInstanceOfArray@http://localhost:6931/HelloWasm.wasm:wasm-function[176]:0xa36de
_HelloWasm_Program__Main@http://localhost:6931/HelloWasm.wasm:wasm-function[343]:0xb255f
_HelloWasm__Module___MainMethodWrapper@http://localhost:6931/HelloWasm.wasm:wasm-function[337]:0xaf501
_StartupCodeMain@http://localhost:6931/HelloWasm.wasm:wasm-function[326]:0xaf090
___managed__Main@http://localhost:6931/HelloWasm.wasm:wasm-function[19571]:0xa14748
_main@http://localhost:6931/HelloWasm.wasm:wasm-function[21430]:0xb19879
Module._main@http://localhost:6931/HelloWasm.js:6739:10
callMain@http://localhost:6931/HelloWasm.js:7048:15
doRun@http://localhost:6931/HelloWasm.js:7106:42
run/<@http://localhost:6931/HelloWasm.js:7117:7
setTimeout handler*run@http://localhost:6931/HelloWasm.js:7113:5
runCaller@http://localhost:6931/HelloWasm.js:7025:29
removeRunDependency@http://localhost:6931/HelloWasm.js:1514:7
receiveInstance@http://localhost:6931/HelloWasm.js:1675:7
receiveInstantiatedSource@http://localhost:6931/HelloWasm.js:1700:7
promise callback*doNativeWasm@http://localhost:6931/HelloWasm.js:1716:10
integrateWasmJS/Module.asm@http://localhost:6931/HelloWasm.js:1805:15
@http://localhost:6931/HelloWasm.js:6529:10

[yowl]

That looks like a reflection stack trace that I've seen a few times with #5842 . Reflection is not enabled, yet, for wasm. This might just go away when I get #5842 finished. At the moment I'm waiting for emscripten-core/emscripten-fastcomp#238.

[MichalStrehovsky]

Null dereference in EEType__get_Kind does sound like something is broken. Nothing in the stack pops out to me - the only option is to step through this and see if the EEType data structure is corrupt - from the stack it kind of looks like the array element type is a null pointer.

[yowl]

You could try building with WASM=0 and posting the js for IsCloned and Kind. Perhaps the shadow stack is setup wrong.

[hippiehunter]

ok, So i did a little bit more digging and this works

object tempArrayThing = new CastingTestClass[1];
if(tempArrayThing is CastingTestClass[])
{
   PrintLine("blap");
}

but this fails

object arrayCastingTest = new BoxStubTest[] {}
if (!(arrayCastingTest is CastingTestClass[]))
{   
    PrintLine("Type casting with isinst & castclass to array test: Ok.");
}

Nothing looks wrong inside EEType__get_Kind/EEType__get_IsCloned, both seem fine. I cant tell if the issue is the EEType symbol for the array itself or if the failure is inside the CastCache when the types aren't actually compatible.

[yowl]

Both these tests pass in my local branch where I have reflection enabled. Do you have the changes mentioned here #5842 (comment) from #6467? Looks like this was merged 2 days ago.

[hippiehunter]

I rebased with what is currently in master but I still get the same stack trace

[yowl]

Is there a branch to look at?