authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2019-09-20 17:48:24-04:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2019-09-20 17:48:24-04:00
log257fded70cc456f0c4c2a56f845cf7f21e738674
treebb2ab8d0f241b7c37ef2d99ff5b798dde4edfa48
parentf663bcd6b2ff8186e13b6826318fa3c8e72a9ff9
signaturelock-open Commit is signed but in an unrecognized format.

avoid setting `tail` for `@panic`

Currently, slices are passed via reference, even though it would be better to pass the ptr and len as separate arguments (#561). This means that any function call with a slice parameter cannot be a tail call, because according to LLVM spec: > Both [tail,musttail] markers imply that the callee does not access > allocas from the caller There was one other place we were setting `tail` and I made that conditional on whether or not the argument referenced allocas in the caller. This was causing undefined behavior in the compiler when it hit asserts, causing it to print garbage memory to the terminal. See #3262 for example.

1 files changed, 47 insertions(+), 17 deletions(-)

src/codegen.cpp+47-17
...@@ -964,7 +964,9 @@ static ZigType *ptr_to_stack_trace_type(CodeGen *g) {...@@ -964,7 +964,9 @@ static ZigType *ptr_to_stack_trace_type(CodeGen *g) {
964 return get_pointer_to_type(g, get_stack_trace_type(g), false);964 return get_pointer_to_type(g, get_stack_trace_type(g), false);
965}965}
966966
967static void gen_panic(CodeGen *g, LLVMValueRef msg_arg, LLVMValueRef stack_trace_arg) {967static void gen_panic(CodeGen *g, LLVMValueRef msg_arg, LLVMValueRef stack_trace_arg,
968 bool stack_trace_is_llvm_alloca)
969{
968 assert(g->panic_fn != nullptr);970 assert(g->panic_fn != nullptr);
969 LLVMValueRef fn_val = fn_llvm_value(g, g->panic_fn);971 LLVMValueRef fn_val = fn_llvm_value(g, g->panic_fn);
970 LLVMCallConv llvm_cc = get_llvm_cc(g, g->panic_fn->type_entry->data.fn.fn_type_id.cc);972 LLVMCallConv llvm_cc = get_llvm_cc(g, g->panic_fn->type_entry->data.fn.fn_type_id.cc);
...@@ -975,14 +977,20 @@ static void gen_panic(CodeGen *g, LLVMValueRef msg_arg, LLVMValueRef stack_trace...@@ -975,14 +977,20 @@ static void gen_panic(CodeGen *g, LLVMValueRef msg_arg, LLVMValueRef stack_trace
975 msg_arg,977 msg_arg,
976 stack_trace_arg,978 stack_trace_arg,
977 };979 };
978 LLVMValueRef call_instruction = ZigLLVMBuildCall(g->builder, fn_val, args, 2, llvm_cc, ZigLLVM_FnInlineAuto, "");980 ZigLLVMBuildCall(g->builder, fn_val, args, 2, llvm_cc, ZigLLVM_FnInlineAuto, "");
979 LLVMSetTailCall(call_instruction, true);981 if (!stack_trace_is_llvm_alloca) {
982 // The stack trace argument is not in the stack of the caller, so
983 // we'd like to set tail call here, but because slices (the type of msg_arg) are
984 // still passed as pointers (see https://github.com/ziglang/zig/issues/561) we still
985 // cannot make this a tail call.
986 //LLVMSetTailCall(call_instruction, true);
987 }
980 LLVMBuildUnreachable(g->builder);988 LLVMBuildUnreachable(g->builder);
981}989}
982990
983// TODO update most callsites to call gen_assertion instead of this991// TODO update most callsites to call gen_assertion instead of this
984static void gen_safety_crash(CodeGen *g, PanicMsgId msg_id) {992static void gen_safety_crash(CodeGen *g, PanicMsgId msg_id) {
985 gen_panic(g, get_panic_msg_ptr_val(g, msg_id), nullptr);993 gen_panic(g, get_panic_msg_ptr_val(g, msg_id), nullptr, false);
986}994}
987995
988static void gen_assertion_scope(CodeGen *g, PanicMsgId msg_id, Scope *source_scope) {996static void gen_assertion_scope(CodeGen *g, PanicMsgId msg_id, Scope *source_scope) {
...@@ -1320,7 +1328,7 @@ static LLVMValueRef get_safety_crash_err_fn(CodeGen *g) {...@@ -1320,7 +1328,7 @@ static LLVMValueRef get_safety_crash_err_fn(CodeGen *g) {
1320 gen_store_untyped(g, slice_len, msg_slice_len_field_ptr, 0, false);1328 gen_store_untyped(g, slice_len, msg_slice_len_field_ptr, 0, false);
13211329
1322 // Call panic()1330 // Call panic()
1323 gen_panic(g, msg_slice, err_ret_trace_arg);1331 gen_panic(g, msg_slice, err_ret_trace_arg, false);
13241332
1325 LLVMPositionBuilderAtEnd(g->builder, prev_block);1333 LLVMPositionBuilderAtEnd(g->builder, prev_block);
1326 if (!g->strip_debug_symbols) {1334 if (!g->strip_debug_symbols) {
...@@ -1331,21 +1339,25 @@ static LLVMValueRef get_safety_crash_err_fn(CodeGen *g) {...@@ -1331,21 +1339,25 @@ static LLVMValueRef get_safety_crash_err_fn(CodeGen *g) {
1331 return fn_val;1339 return fn_val;
1332}1340}
13331341
1334static LLVMValueRef get_cur_err_ret_trace_val(CodeGen *g, Scope *scope) {1342static LLVMValueRef get_cur_err_ret_trace_val(CodeGen *g, Scope *scope, bool *is_llvm_alloca) {
1335 if (!g->have_err_ret_tracing) {1343 if (!g->have_err_ret_tracing) {
1344 *is_llvm_alloca = false;
1336 return nullptr;1345 return nullptr;
1337 }1346 }
1338 if (g->cur_err_ret_trace_val_stack != nullptr) {1347 if (g->cur_err_ret_trace_val_stack != nullptr) {
1348 *is_llvm_alloca = !fn_is_async(g->cur_fn);
1339 return g->cur_err_ret_trace_val_stack;1349 return g->cur_err_ret_trace_val_stack;
1340 }1350 }
1351 *is_llvm_alloca = false;
1341 return g->cur_err_ret_trace_val_arg;1352 return g->cur_err_ret_trace_val_arg;
1342}1353}
13431354
1344static void gen_safety_crash_for_err(CodeGen *g, LLVMValueRef err_val, Scope *scope) {1355static void gen_safety_crash_for_err(CodeGen *g, LLVMValueRef err_val, Scope *scope) {
1345 LLVMValueRef safety_crash_err_fn = get_safety_crash_err_fn(g);1356 LLVMValueRef safety_crash_err_fn = get_safety_crash_err_fn(g);
1346 LLVMValueRef call_instruction;1357 LLVMValueRef call_instruction;
1358 bool is_llvm_alloca = false;
1347 if (g->have_err_ret_tracing) {1359 if (g->have_err_ret_tracing) {
1348 LLVMValueRef err_ret_trace_val = get_cur_err_ret_trace_val(g, scope);1360 LLVMValueRef err_ret_trace_val = get_cur_err_ret_trace_val(g, scope, &is_llvm_alloca);
1349 if (err_ret_trace_val == nullptr) {1361 if (err_ret_trace_val == nullptr) {
1350 err_ret_trace_val = LLVMConstNull(get_llvm_type(g, ptr_to_stack_trace_type(g)));1362 err_ret_trace_val = LLVMConstNull(get_llvm_type(g, ptr_to_stack_trace_type(g)));
1351 }1363 }
...@@ -1362,7 +1374,9 @@ static void gen_safety_crash_for_err(CodeGen *g, LLVMValueRef err_val, Scope *sc...@@ -1362,7 +1374,9 @@ static void gen_safety_crash_for_err(CodeGen *g, LLVMValueRef err_val, Scope *sc
1362 call_instruction = ZigLLVMBuildCall(g->builder, safety_crash_err_fn, args, 1,1374 call_instruction = ZigLLVMBuildCall(g->builder, safety_crash_err_fn, args, 1,
1363 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");1375 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");
1364 }1376 }
1365 LLVMSetTailCall(call_instruction, true);1377 if (!is_llvm_alloca) {
1378 LLVMSetTailCall(call_instruction, true);
1379 }
1366 LLVMBuildUnreachable(g->builder);1380 LLVMBuildUnreachable(g->builder);
1367}1381}
13681382
...@@ -2202,7 +2216,9 @@ static LLVMValueRef ir_render_save_err_ret_addr(CodeGen *g, IrExecutable *execut...@@ -2202,7 +2216,9 @@ static LLVMValueRef ir_render_save_err_ret_addr(CodeGen *g, IrExecutable *execut
2202 assert(g->have_err_ret_tracing);2216 assert(g->have_err_ret_tracing);
22032217
2204 LLVMValueRef return_err_fn = get_return_err_fn(g);2218 LLVMValueRef return_err_fn = get_return_err_fn(g);
2205 LLVMValueRef my_err_trace_val = get_cur_err_ret_trace_val(g, save_err_ret_addr_instruction->base.scope);2219 bool is_llvm_alloca;
2220 LLVMValueRef my_err_trace_val = get_cur_err_ret_trace_val(g, save_err_ret_addr_instruction->base.scope,
2221 &is_llvm_alloca);
2206 ZigLLVMBuildCall(g->builder, return_err_fn, &my_err_trace_val, 1,2222 ZigLLVMBuildCall(g->builder, return_err_fn, &my_err_trace_val, 1,
2207 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");2223 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");
22082224
...@@ -2256,6 +2272,10 @@ static LLVMBasicBlockRef gen_suspend_begin(CodeGen *g, const char *name_hint) {...@@ -2256,6 +2272,10 @@ static LLVMBasicBlockRef gen_suspend_begin(CodeGen *g, const char *name_hint) {
2256 return resume_bb;2272 return resume_bb;
2257}2273}
22582274
2275// Be careful setting tail call. According to LLVM lang ref,
2276// tail and musttail imply that the callee does not access allocas from the caller.
2277// This works for async functions since the locals are spilled.
2278// http://llvm.org/docs/LangRef.html#id320
2259static void set_tail_call_if_appropriate(CodeGen *g, LLVMValueRef call_inst) {2279static void set_tail_call_if_appropriate(CodeGen *g, LLVMValueRef call_inst) {
2260 LLVMSetTailCall(call_inst, true);2280 LLVMSetTailCall(call_inst, true);
2261}2281}
...@@ -2361,7 +2381,8 @@ static void gen_async_return(CodeGen *g, IrInstructionReturn *instruction) {...@@ -2361,7 +2381,8 @@ static void gen_async_return(CodeGen *g, IrInstructionReturn *instruction) {
2361 LLVMValueRef awaiter_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, g->cur_frame_ptr,2381 LLVMValueRef awaiter_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, g->cur_frame_ptr,
2362 frame_index_trace_arg(g, ret_type) + 1, "");2382 frame_index_trace_arg(g, ret_type) + 1, "");
2363 LLVMValueRef dest_trace_ptr = LLVMBuildLoad(g->builder, awaiter_trace_ptr_ptr, "");2383 LLVMValueRef dest_trace_ptr = LLVMBuildLoad(g->builder, awaiter_trace_ptr_ptr, "");
2364 LLVMValueRef my_err_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope);2384 bool is_llvm_alloca;
2385 LLVMValueRef my_err_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope, &is_llvm_alloca);
2365 LLVMValueRef args[] = { dest_trace_ptr, my_err_trace_val };2386 LLVMValueRef args[] = { dest_trace_ptr, my_err_trace_val };
2366 ZigLLVMBuildCall(g->builder, get_merge_err_ret_traces_fn_val(g), args, 2,2387 ZigLLVMBuildCall(g->builder, get_merge_err_ret_traces_fn_val(g), args, 2,
2367 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");2388 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");
...@@ -3967,7 +3988,9 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr...@@ -3967,7 +3988,9 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr
3967 if (prefix_arg_err_ret_stack) {3988 if (prefix_arg_err_ret_stack) {
3968 LLVMValueRef err_ret_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, frame_result_loc,3989 LLVMValueRef err_ret_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, frame_result_loc,
3969 frame_index_trace_arg(g, src_return_type) + 1, "");3990 frame_index_trace_arg(g, src_return_type) + 1, "");
3970 LLVMValueRef my_err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope);3991 bool is_llvm_alloca;
3992 LLVMValueRef my_err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope,
3993 &is_llvm_alloca);
3971 LLVMBuildStore(g->builder, my_err_ret_trace_val, err_ret_trace_ptr_ptr);3994 LLVMBuildStore(g->builder, my_err_ret_trace_val, err_ret_trace_ptr_ptr);
3972 }3995 }
3973 }3996 }
...@@ -4024,7 +4047,8 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr...@@ -4024,7 +4047,8 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr
40244047
4025 gen_init_stack_trace(g, trace_field_ptr, addrs_field_ptr);4048 gen_init_stack_trace(g, trace_field_ptr, addrs_field_ptr);
40264049
4027 gen_param_values.append(get_cur_err_ret_trace_val(g, instruction->base.scope));4050 bool is_llvm_alloca;
4051 gen_param_values.append(get_cur_err_ret_trace_val(g, instruction->base.scope, &is_llvm_alloca));
4028 }4052 }
4029 }4053 }
4030 } else {4054 } else {
...@@ -4032,7 +4056,8 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr...@@ -4032,7 +4056,8 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr
4032 gen_param_values.append(result_loc);4056 gen_param_values.append(result_loc);
4033 }4057 }
4034 if (prefix_arg_err_ret_stack) {4058 if (prefix_arg_err_ret_stack) {
4035 gen_param_values.append(get_cur_err_ret_trace_val(g, instruction->base.scope));4059 bool is_llvm_alloca;
4060 gen_param_values.append(get_cur_err_ret_trace_val(g, instruction->base.scope, &is_llvm_alloca));
4036 }4061 }
4037 }4062 }
4038 FnWalk fn_walk = {};4063 FnWalk fn_walk = {};
...@@ -4912,7 +4937,8 @@ static LLVMValueRef ir_render_align_cast(CodeGen *g, IrExecutable *executable, I...@@ -4912,7 +4937,8 @@ static LLVMValueRef ir_render_align_cast(CodeGen *g, IrExecutable *executable, I
4912static LLVMValueRef ir_render_error_return_trace(CodeGen *g, IrExecutable *executable,4937static LLVMValueRef ir_render_error_return_trace(CodeGen *g, IrExecutable *executable,
4913 IrInstructionErrorReturnTrace *instruction)4938 IrInstructionErrorReturnTrace *instruction)
4914{4939{
4915 LLVMValueRef cur_err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope);4940 bool is_llvm_alloca;
4941 LLVMValueRef cur_err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope, &is_llvm_alloca);
4916 if (cur_err_ret_trace_val == nullptr) {4942 if (cur_err_ret_trace_val == nullptr) {
4917 return LLVMConstNull(get_llvm_type(g, ptr_to_stack_trace_type(g)));4943 return LLVMConstNull(get_llvm_type(g, ptr_to_stack_trace_type(g)));
4918 }4944 }
...@@ -5496,7 +5522,9 @@ static LLVMValueRef ir_render_union_tag(CodeGen *g, IrExecutable *executable, Ir...@@ -5496,7 +5522,9 @@ static LLVMValueRef ir_render_union_tag(CodeGen *g, IrExecutable *executable, Ir
5496}5522}
54975523
5498static LLVMValueRef ir_render_panic(CodeGen *g, IrExecutable *executable, IrInstructionPanic *instruction) {5524static LLVMValueRef ir_render_panic(CodeGen *g, IrExecutable *executable, IrInstructionPanic *instruction) {
5499 gen_panic(g, ir_llvm_value(g, instruction->msg), get_cur_err_ret_trace_val(g, instruction->base.scope));5525 bool is_llvm_alloca;
5526 LLVMValueRef err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope, &is_llvm_alloca);
5527 gen_panic(g, ir_llvm_value(g, instruction->msg), err_ret_trace_val, is_llvm_alloca);
5500 return nullptr;5528 return nullptr;
5501}5529}
55025530
...@@ -5753,7 +5781,8 @@ static LLVMValueRef gen_await_early_return(CodeGen *g, IrInstruction *source_ins...@@ -5753,7 +5781,8 @@ static LLVMValueRef gen_await_early_return(CodeGen *g, IrInstruction *source_ins
5753 LLVMValueRef their_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, target_frame_ptr,5781 LLVMValueRef their_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, target_frame_ptr,
5754 frame_index_trace_arg(g, result_type), "");5782 frame_index_trace_arg(g, result_type), "");
5755 LLVMValueRef src_trace_ptr = LLVMBuildLoad(g->builder, their_trace_ptr_ptr, "");5783 LLVMValueRef src_trace_ptr = LLVMBuildLoad(g->builder, their_trace_ptr_ptr, "");
5756 LLVMValueRef dest_trace_ptr = get_cur_err_ret_trace_val(g, source_instr->scope);5784 bool is_llvm_alloca;
5785 LLVMValueRef dest_trace_ptr = get_cur_err_ret_trace_val(g, source_instr->scope, &is_llvm_alloca);
5757 LLVMValueRef args[] = { dest_trace_ptr, src_trace_ptr };5786 LLVMValueRef args[] = { dest_trace_ptr, src_trace_ptr };
5758 ZigLLVMBuildCall(g->builder, get_merge_err_ret_traces_fn_val(g), args, 2,5787 ZigLLVMBuildCall(g->builder, get_merge_err_ret_traces_fn_val(g), args, 2,
5759 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");5788 get_llvm_cc(g, CallingConventionUnspecified), ZigLLVM_FnInlineAuto, "");
...@@ -5802,7 +5831,8 @@ static LLVMValueRef ir_render_await(CodeGen *g, IrExecutable *executable, IrInst...@@ -5802,7 +5831,8 @@ static LLVMValueRef ir_render_await(CodeGen *g, IrExecutable *executable, IrInst
58025831
5803 // supply the error return trace pointer5832 // supply the error return trace pointer
5804 if (codegen_fn_has_err_ret_tracing_arg(g, result_type)) {5833 if (codegen_fn_has_err_ret_tracing_arg(g, result_type)) {
5805 LLVMValueRef my_err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope);5834 bool is_llvm_alloca;
5835 LLVMValueRef my_err_ret_trace_val = get_cur_err_ret_trace_val(g, instruction->base.scope, &is_llvm_alloca);
5806 assert(my_err_ret_trace_val != nullptr);5836 assert(my_err_ret_trace_val != nullptr);
5807 LLVMValueRef err_ret_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, target_frame_ptr,5837 LLVMValueRef err_ret_trace_ptr_ptr = LLVMBuildStructGEP(g->builder, target_frame_ptr,
5808 frame_index_trace_arg(g, result_type) + 1, "");5838 frame_index_trace_arg(g, result_type) + 1, "");