Description
Currently mono_inst_name
is defined like this:
const char*
mono_inst_name (int op) {
#ifndef DISABLE_LOGGING
if (op >= OP_LOAD && op <= OP_LAST)
return (const char*)&opstr + opidx [op - OP_LOAD];
if (op < OP_LOAD)
return mono_opcode_name (op);
g_error ("unknown opcode name for %d", op);
return NULL;
#else
g_error ("unknown opcode name for %d", op);
g_assert_not_reached ();
#endif
}
The problem is that when logging is disabled, this function is uncallable (the #else
branch, above). Any call to it will crash the runtime.
On Android, iOS and WASM we build the runtime with DISABLE_LOGGING
defined. Below is Android, but wasm and ios are similar:
<_MonoCMakeArgs Include="-DENABLE_MINIMAL=ssa,logging" />
Why this is bad:
If someone incorrectly leaves in a call to mono_inst_name
in a branch that isn't guarded by #ifndef DISABLE_LOGGING
, the runtime will crash. We tend to use mono_inst_name
in code that is trying to provide additional diagnostics. So something bad has already happened, we try to gather more info, and instead we crash the runtime.
What we should do instead:
Wrap both the declaration and definition of mono_inst_name
in #ifndef DISABLE_LOGGING
. Turn any use of mono_inst_name
that isn't protected by #ifndef DISABLE_LOGGING
into a compile-time error that will fail to build the runtime.
That way, we don't get random crashes during unrelated investigations just because we called some diagnostic functionality