Skip to content

Commit a483d90

Browse files
authored
codegen: Unify printing of and add some more internal IR validity errors (#58429)
What should codegen do when it detects invalid IR? There are a few reasonable options. One is to just assert - this is relatively straightforward but also not very friendly because it immediately takes down your session, so you can't inspect what values may have caused the issue. Additionally, often allow invalid IR in dead code (because otherwise transformation passes would have to check whether the transformation that they're doing makes some code dead, which can be expensive or not necessarily visible). Thus we generally try to keep codegen going, trying to bail back to valid code as quickly as possible. In a few places, we additionally insert an unmodeled `emit_error("Internal Error")`. These are a bit weird because the earlier stages of the compiler do not know that they can exist, but in practice they mostly work fine (although they can cause additional crashes if there are try/catches in the way). If we don't, then we generally just stop codegening, so if execution ever were to reach there, it'll just run into whatever code comes after, likely crashing fairly soon. Because of this consideration, the `emit_error` is generally preferred. However, the tradeoff is that it increases the size of the code and LLVM can no longer optimize under the assumption that a particular code branch doesn't happen. We thus need to be judicious in where we use it. The general guidance is that it's fine to use in situations where the IR itself is obviously invalid, but should not be arbitrarily added to all instances of `unreachable` (putting behind NDEBUG is fine). This PR cleans this up a bit by changing all these error locations to print `INTERNAL ERROR - IR Validity`, as well as adding a new one to `emit_invoke` when there's a manifest mismatch in type of the arguments and the signature. This new case is particularly useful after the recent addition of ABIOverride, as that makes it more likely that external AbstractInterpreters are doing ABI shenanigans.
1 parent ee7e814 commit a483d90

File tree

1 file changed

+8
-6
lines changed

1 file changed

+8
-6
lines changed

src/codegen.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4990,8 +4990,10 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, bool is_opaque_clos
49904990
}
49914991
jl_value_t *jt = jl_nth_slot_type(specTypes, i);
49924992
jl_cgval_t arg = update_julia_type(ctx, argv[i], jt);
4993-
if (arg.typ == jl_bottom_type)
4993+
if (arg.typ == jl_bottom_type) {
4994+
emit_error(ctx, "(INTERNAL ERROR - IR Validity): Argument type mismatch in Expr(:invoke)");
49944995
return jl_cgval_t();
4996+
}
49954997
if (is_uniquerep_Type(jt)) {
49964998
continue;
49974999
}
@@ -5028,7 +5030,7 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, bool is_opaque_clos
50285030
Value *val = emit_unbox(ctx, et, arg, jt);
50295031
if (!val) {
50305032
// There was a type mismatch of some sort - exit early
5031-
CreateTrap(ctx.builder);
5033+
emit_error(ctx, "(INTERNAL ERROR - IR Validity): Argument type mismatch in Expr(:invoke)");
50325034
return jl_cgval_t();
50335035
}
50345036
argvals[idx] = val;
@@ -5295,7 +5297,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, ArrayR
52955297
}
52965298
if (result.typ == jl_bottom_type) {
52975299
#ifndef JL_NDEBUG
5298-
emit_error(ctx, "(Internal Error - IR Validity): Returned from function we expected not to.");
5300+
emit_error(ctx, "(INTERNAL ERROR - IR Validity): Returned from function we expected not to.");
52995301
#endif
53005302
CreateTrap(ctx.builder);
53015303
}
@@ -5647,7 +5649,7 @@ static jl_cgval_t emit_local(jl_codectx_t &ctx, jl_value_t *slotload)
56475649
if (sym == jl_unused_sym) {
56485650
// This shouldn't happen in well-formed input, but let's be robust,
56495651
// since we otherwise cause undefined behavior here.
5650-
emit_error(ctx, "(INTERNAL ERROR): Tried to use `#undef#` argument.");
5652+
emit_error(ctx, "(INTERNAL ERROR - IR Validity): Tried to use `#undef#` argument.");
56515653
return jl_cgval_t();
56525654
}
56535655
return emit_varinfo(ctx, vi, sym);
@@ -6494,7 +6496,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
64946496
if (source.constant == NULL) {
64956497
// For now, we require non-constant source to be handled by using
64966498
// eval. This should probably be a verifier error and an abort here.
6497-
emit_error(ctx, "(internal error) invalid IR: opaque closure source must be constant");
6499+
emit_error(ctx, "(INTERNAL ERROR - IR Validity): opaque closure source must be constant");
64986500
return jl_cgval_t();
64996501
}
65006502
bool can_optimize = argt.constant != NULL && lb.constant != NULL && ub.constant != NULL &&
@@ -9380,7 +9382,7 @@ static jl_llvm_functions_t
93809382
// Probably dead code, but let's be loud about it in case it isn't, so we fail
93819383
// at the point of the miscompile, rather than later when something attempts to
93829384
// read the scope.
9383-
emit_error(ctx, "(INTERNAL ERROR): Attempted to execute EnterNode with bad scope");
9385+
emit_error(ctx, "(INTERNAL ERROR - IR Validity): Attempted to execute EnterNode with bad scope");
93849386
find_next_stmt(-1);
93859387
continue;
93869388
}

0 commit comments

Comments
 (0)