Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mono][interp] Throw invalid program if stack state is invalid #73215

Merged
merged 1 commit into from
Dec 12, 2022
Merged

[mono][interp] Throw invalid program if stack state is invalid #73215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 43 additions & 19 deletions src/mono/mono/mini/interp/transform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,33 @@ interp_prev_ins (InterpInst *ins)
return ins;
}

static gboolean
check_stack_helper (TransformData *td, int n)
{
int stack_size = GPTRDIFF_TO_INT (td->sp - td->stack);
if (stack_size < n) {
td->has_invalid_code = TRUE;
return FALSE;
}
return TRUE;
}

#define CHECK_STACK(td, n) \
do { \
guint stack_size = GPTRDIFF_TO_UINT ((td)->sp - (td)->stack); \
if (stack_size < (n)) \
g_warning ("%s.%s: not enough values (%d < %d) on stack at %04x", \
m_class_get_name ((td)->method->klass), (td)->method->name, \
stack_size, n, (td)->ip - (td)->il_code); \
if (!check_stack_helper (td, n)) \
goto exit; \
} while (0)

#define CHECK_STACK_RET_VOID(td, n) \
do { \
if (!check_stack_helper (td, n)) \
return; \
} while (0)

#define CHECK_STACK_RET(td, n, ret) \
do { \
if (!check_stack_helper (td, n)) \
return ret; \
} while (0)

#define ENSURE_STACK_SIZE(td, size) \
Expand Down Expand Up @@ -872,9 +892,9 @@ handle_branch (TransformData *td, int long_op, int offset)
static void
one_arg_branch(TransformData *td, int mint_op, int offset, int inst_size)
{
CHECK_STACK_RET_VOID(td, 1);
int type = td->sp [-1].type == STACK_TYPE_O || td->sp [-1].type == STACK_TYPE_MP ? STACK_TYPE_I : td->sp [-1].type;
int long_op = mint_op + type - STACK_TYPE_I4;
CHECK_STACK(td, 1);
--td->sp;
if (offset) {
handle_branch (td, long_op, offset + inst_size);
Expand All @@ -901,9 +921,9 @@ interp_add_conv (TransformData *td, StackInfo *sp, InterpInst *prev_ins, int typ
static void
two_arg_branch(TransformData *td, int mint_op, int offset, int inst_size)
{
CHECK_STACK_RET_VOID(td, 2);
int type1 = td->sp [-1].type == STACK_TYPE_O || td->sp [-1].type == STACK_TYPE_MP ? STACK_TYPE_I : td->sp [-1].type;
int type2 = td->sp [-2].type == STACK_TYPE_O || td->sp [-2].type == STACK_TYPE_MP ? STACK_TYPE_I : td->sp [-2].type;
CHECK_STACK(td, 2);

if (type1 == STACK_TYPE_I4 && type2 == STACK_TYPE_I8) {
// The il instruction starts with the actual branch, and not with the conversion opcodes
Expand Down Expand Up @@ -935,8 +955,8 @@ two_arg_branch(TransformData *td, int mint_op, int offset, int inst_size)
static void
unary_arith_op(TransformData *td, int mint_op)
{
CHECK_STACK_RET_VOID(td, 1);
int op = mint_op + td->sp [-1].type - STACK_TYPE_I4;
CHECK_STACK(td, 1);
td->sp--;
interp_add_ins (td, op);
interp_ins_set_sreg (td->last_ins, td->sp [0].local);
Expand All @@ -947,6 +967,7 @@ unary_arith_op(TransformData *td, int mint_op)
static void
binary_arith_op(TransformData *td, int mint_op)
{
CHECK_STACK_RET_VOID(td, 2);
int type1 = td->sp [-2].type;
int type2 = td->sp [-1].type;
int op;
Expand Down Expand Up @@ -978,7 +999,6 @@ binary_arith_op(TransformData *td, int mint_op)
td->ip - td->il_code, mono_interp_opname (mint_op), type1, type2);
}
op = mint_op + type1 - STACK_TYPE_I4;
CHECK_STACK(td, 2);
td->sp -= 2;
interp_add_ins (td, op);
interp_ins_set_sregs2 (td->last_ins, td->sp [0].local, td->sp [1].local);
Expand All @@ -989,8 +1009,8 @@ binary_arith_op(TransformData *td, int mint_op)
static void
shift_op(TransformData *td, int mint_op)
{
CHECK_STACK_RET_VOID(td, 2);
int op = mint_op + td->sp [-2].type - STACK_TYPE_I4;
CHECK_STACK(td, 2);
if (td->sp [-1].type != STACK_TYPE_I4) {
g_warning("%s.%s: shift type mismatch %d",
m_class_get_name (td->method->klass), td->method->name,
Expand Down Expand Up @@ -1079,7 +1099,7 @@ store_arg(TransformData *td, int n)
{
gint32 size = 0;
int mt;
CHECK_STACK (td, 1);
CHECK_STACK_RET_VOID (td, 1);
MonoType *type;

type = get_arg_type_exact (td, n, &mt);
Expand Down Expand Up @@ -1127,7 +1147,7 @@ static void
store_local (TransformData *td, int local)
{
int mt = td->locals [local].mt;
CHECK_STACK (td, 1);
CHECK_STACK_RET_VOID (td, 1);
#if SIZEOF_VOID_P == 8
// nint and int32 can be used interchangeably. Add implicit conversions.
if (td->sp [-1].type == STACK_TYPE_I4 && stack_type [mt] == STACK_TYPE_I8)
Expand Down Expand Up @@ -3132,7 +3152,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target

if (target_method == NULL) {
if (calli) {
CHECK_STACK(td, 1);
CHECK_STACK_RET(td, 1, FALSE);
if (method->wrapper_type != MONO_WRAPPER_NONE)
csignature = (MonoMethodSignature *)mono_method_get_wrapper_data (method, token);
else {
Expand Down Expand Up @@ -3300,7 +3320,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
need_null_check = TRUE;
}

CHECK_STACK (td, csignature->param_count + csignature->hasthis);
CHECK_STACK_RET (td, csignature->param_count + csignature->hasthis, FALSE);
if (tailcall && !td->gen_sdb_seq_points && !calli && op == -1 &&
(target_method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) == 0 &&
(target_method->iflags & METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL) == 0 &&
Expand Down Expand Up @@ -4323,7 +4343,7 @@ initialize_clause_bblocks (TransformData *td)
static void
handle_ldind (TransformData *td, int op, int type, gboolean *volatile_)
{
CHECK_STACK (td, 1);
CHECK_STACK_RET_VOID (td, 1);
interp_add_ins (td, op);
td->sp--;
interp_ins_set_sreg (td->last_ins, td->sp [0].local);
Expand All @@ -4340,7 +4360,7 @@ handle_ldind (TransformData *td, int op, int type, gboolean *volatile_)
static void
handle_stind (TransformData *td, int op, gboolean *volatile_)
{
CHECK_STACK (td, 2);
CHECK_STACK_RET_VOID (td, 2);
if (*volatile_) {
interp_emit_memory_barrier (td, MONO_MEMORY_BARRIER_REL);
*volatile_ = FALSE;
Expand All @@ -4355,7 +4375,7 @@ handle_stind (TransformData *td, int op, gboolean *volatile_)
static void
handle_ldelem (TransformData *td, int op, int type)
{
CHECK_STACK (td, 2);
CHECK_STACK_RET_VOID (td, 2);
ENSURE_I4 (td, 1);
interp_add_ins (td, op);
td->sp -= 2;
Expand All @@ -4368,7 +4388,7 @@ handle_ldelem (TransformData *td, int op, int type)
static void
handle_stelem (TransformData *td, int op)
{
CHECK_STACK (td, 3);
CHECK_STACK_RET_VOID (td, 3);
ENSURE_I4 (td, 2);
interp_add_ins (td, op);
td->sp -= 3;
Expand Down Expand Up @@ -4589,7 +4609,9 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,

td->dont_inline = g_list_prepend (td->dont_inline, method);
while (td->ip < end) {
g_assert (td->sp >= td->stack);
// Check here for every opcode to avoid code bloat
if (td->has_invalid_code)
goto exit;
in_offset = GPTRDIFF_TO_INT (td->ip - header->code);
if (!inlining)
td->current_il_offset = in_offset;
Expand Down Expand Up @@ -7759,6 +7781,8 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
return ret;
exit:
ret = FALSE;
if (td->has_invalid_code)
mono_error_set_generic_error (error, "System", "InvalidProgramException", "");
goto exit_ret;
}

Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/interp/transform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ typedef struct
// bail out of inlining when having to generate certain opcodes (like call, throw).
int aggressive_inlining : 1;
int optimized : 1;
int has_invalid_code : 1;
} TransformData;

#define STACK_TYPE_I4 0
Expand Down
3 changes: 0 additions & 3 deletions src/tests/issues.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2207,9 +2207,6 @@
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Methodical/ELEMENT_TYPE_IU/u_fld_il_d/**">
<Issue>needs triage</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/JIT/jit64/localloc/ehverify/eh07_large/**">
<Issue>https://github.com/dotnet/runtime/issues/54395</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/JIT/jit64/verif/sniff/fg/ver_fg_13/**">
<Issue>https://github.com/dotnet/runtime/issues/54396</Issue>
</ExcludeList>
Expand Down