Miscompilation of do-while when the while starts with a branch #2394. Also: change do-while to make the lowering follow the execution.

releasenotes.md

@@ -46,6 +46,7 @@
 - Regression: 1 character module names would create an error.
 - Compiler segfault with struct containing list of structs with an inline member #2416
 - Occasionally when using macro method extensions on built-in types, the liveness checker would try to process them. #2398
+- Miscompilation of do-while when the while starts with a branch #2394.
 
 ### Stdlib changes
 - Add `==` to `Pair`, `Triple` and TzDateTime. Add print to `Pair` and `Triple`.

src/compiler/llvm_codegen_stmt.c

@@ -501,87 +501,8 @@ static inline LoopType loop_type_for_cond(Expr *cond, bool do_while)
 	return LOOP_NORMAL;
 }
 
-void llvm_emit_for_stmt(GenContext *c, Ast *ast)
+static void llmv_emit_for_cond(GenContext *c, Expr *cond, LLVMBasicBlockRef cond_block, LLVMBasicBlockRef loop_start_block, LLVMBasicBlockRef exit_block)
 {
-	DEBUG_PUSH_LEXICAL_SCOPE(c, ast->span);
-	// First, emit all inits.
-	if (ast->for_stmt.init)
-	{
-		llvm_emit_ignored_expr(c, exprptr(ast->for_stmt.init));
-	}
-	ExprId incr = ast->for_stmt.incr;
-
-	LLVMBasicBlockRef inc_block = incr ? llvm_basic_block_new(c, "loop.inc") : NULL;
-	Ast *body = astptr(ast->for_stmt.body);
-	LLVMBasicBlockRef body_block = ast_is_not_empty(body) ? llvm_basic_block_new(c, "loop.body") : NULL;
-	LLVMBasicBlockRef cond_block = NULL;
-
-	// Skipping first cond? This is do-while semantics
-	bool skip_first = ast->for_stmt.flow.skip_first;
-
-	ExprId cond_id = ast->for_stmt.cond;
-	Expr *cond = cond_id ? exprptr(cond_id) : NULL;
-	LoopType loop = loop_type_for_cond(cond, skip_first);
-
-	// This is the starting block to loop back to, and may either be cond, body or inc
-	LLVMBasicBlockRef loop_start_block = body_block ? body_block : inc_block;
-
-	// We only emit a cond block if we have a normal loop.
-	if (loop == LOOP_NORMAL)
-	{
-		cond_block = llvm_basic_block_new(c, "loop.cond");
-		loop_start_block = cond_block;
-	}
-
-	// In the case that *none* of the blocks exist.
-	if (!inc_block && !body_block && !cond_block)
-	{
-		if (loop == LOOP_INFINITE)
-		{
-			SourceSpan loc = ast->span;
-
-			llvm_emit_panic(c, "Infinite loop found", loc, NULL, NULL);
-			llvm_emit_block(c, llvm_basic_block_new(c, "unreachable_block"));
-			DEBUG_POP_LEXICAL_SCOPE(c);
-			return;
-		}
-		DEBUG_POP_LEXICAL_SCOPE(c);
-		return;
-	}
-
-	ASSERT(loop_start_block != NULL);
-
-	LLVMBasicBlockRef exit_block = llvm_basic_block_new(c, "loop.exit");
-
-	// Break is simple it always jumps out.
-	// For continue:
-	// 1. If there is inc, jump to the condition
-	// 2. If this is not looping, jump to the exit, otherwise go to cond/body depending on what the start is.
-	LLVMBasicBlockRef continue_block = inc_block;
-	if (!continue_block)
-	{
-		continue_block = loop == LOOP_NONE ? exit_block : loop_start_block;
-	}
-
-	ast->for_stmt.codegen.continue_block = continue_block;
-	ast->for_stmt.codegen.exit_block = exit_block;
-
-	// We have a normal loop, so we emit a cond.
-	if (loop == LOOP_NORMAL)
-	{
-		// Emit a jump for do-while semantics, to skip the initial cond.
-		if (skip_first)
-		{
-			LLVMBasicBlockRef do_while_start = body_block ? body_block : inc_block;
-			// Only jump if we have a body / inc
-			// if the case is do {} while (...) then we basically can treat this as while (...) {}
-			llvm_emit_br(c, do_while_start ? do_while_start : cond_block);
-		}
-		else
-		{
-			llvm_emit_br(c, cond_block);
-		}
-
 	// Emit the block
 	llvm_emit_block(c, cond_block);
 	BEValue be_value;
@@ -594,92 +515,158 @@ void llvm_emit_for_stmt(GenContext *c, Ast *ast)
 	{
 		llvm_emit_expr(c, &be_value, cond);
 	}
+	if (llvm_get_current_block_if_in_use(c))
+	{
 		llvm_value_rvalue(c, &be_value);
 		ASSERT(llvm_value_is_bool(&be_value));
 
-		// If we have a body, conditionally jump to it.
-		LLVMBasicBlockRef cond_success = body_block ? body_block : inc_block;
-		// If there is a while (...) { } we need to set the success to this block
-		if (!cond_success) cond_success = cond_block;
-		// Otherwise jump to inc or cond depending on what's available.
-		llvm_emit_cond_br(c, &be_value, cond_success, exit_block);
+		// Jump to start or exit
+		llvm_emit_cond_br(c, &be_value, loop_start_block, exit_block);
+	}
+}
+void llvm_emit_for_stmt(GenContext *c, Ast *ast)
+{
+	DEBUG_PUSH_LEXICAL_SCOPE(c, ast->span);
+	// First, emit all inits.
+	if (ast->for_stmt.init)
+	{
+		llvm_emit_ignored_expr(c, exprptr(ast->for_stmt.init));
 	}
 
-	// The optional cond is emitted, so emit the body
-	if (body_block)
+	// Skipping first cond? This is do-while semantics
+	bool skip_first = ast->for_stmt.flow.skip_first;
+
+	ExprId cond_id = ast->for_stmt.cond;
+	Expr *cond = cond_id ? exprptr(cond_id) : NULL;
+	LoopType loop = loop_type_for_cond(cond, skip_first);
+	Ast *body = astptr(ast->for_stmt.body);
+	ExprId incr = ast->for_stmt.incr;
+
+	if (loop == LOOP_NONE)
 	{
-		// If we have LOOP_NONE, then we don't need a new block here
-		// since we will just exit. That leaves the infinite loop.
+		// while (0) -> never entered
+		if (!skip_first) return;
+		ASSERT(!incr && "There should not be an incr in do-while");
+		// do while(0) -> emit once
+		LLVMBasicBlockRef exit_block = llvm_basic_block_new(c, "loop.exit");
+		ast->for_stmt.codegen.continue_block = exit_block;
+		ast->for_stmt.codegen.exit_block = exit_block;
+		llvm_emit_stmt(c, body);
+		if (!llvm_basic_block_is_unused(exit_block))
+		{
+			llvm_emit_br(c, exit_block);
+			llvm_emit_block(c, exit_block);
+		}
+		DEBUG_POP_LEXICAL_SCOPE(c);
+		return;
+	}
+
+	LLVMBasicBlockRef inc_block = incr ? llvm_basic_block_new(c, "loop.inc") : NULL;
+	LLVMBasicBlockRef body_block = ast_is_not_empty(body) ? llvm_basic_block_new(c, "loop.body") : NULL;
+	LLVMBasicBlockRef cond_block = NULL;
+	LLVMBasicBlockRef exit_block = llvm_basic_block_new(c, "loop.exit");
+
+	// This is the starting block to loop back to, and may either be body, inc or cond
+	LLVMBasicBlockRef loop_start_block = body_block ? body_block : inc_block;
+
+	bool start_with_cond = false;
 	switch (loop)
 	{
 		case LOOP_NORMAL:
-				// If we have LOOP_NORMAL, we already emitted a br to the body.
-				// so emit the block
-				llvm_emit_block(c, body_block);
-				break;
-			case LOOP_INFINITE:
-				// In this case we have no cond, so we need to emit the br and
-				// then the block
-				llvm_emit_br(c, body_block);
-				llvm_emit_block(c, body_block);
-			case LOOP_NONE:
-				// If there is no loop, then we will just fall through and the
-				// block is needed.
-				body_block = NULL;
+			// We only emit a cond block if we have a normal loop.
+			cond_block = llvm_basic_block_new(c, "loop.cond");
+			// Start with cond if we can't start anywhere.
+			if (!loop_start_block) loop_start_block = cond_block;
+			// Jump to the cond block on for/while
+			if (skip_first)
+			{
+				ASSERT(loop_start_block);
+				llvm_emit_br(c, loop_start_block);
 				break;
 			}
-		// Now emit the body
-		llvm_emit_stmt(c, body);
-
-		// Did we have a jump to inc yet?
-		if (inc_block && !llvm_basic_block_is_unused(inc_block))
+			// Otherwise start with cond block
+			start_with_cond = true;
+			llvm_emit_br(c, cond_block);
+			break;
+		case LOOP_INFINITE:
+			// We might have an infite loop
+			if (!loop_start_block)
 			{
-			// If so we emit the jump to the inc block.
-			llvm_emit_br(c, inc_block);
+				SourceSpan loc = ast->span;
+
+				llvm_emit_panic(c, "Infinite loop found", loc, NULL, NULL);
+				llvm_emit_block(c, llvm_basic_block_new(c, "unreachable_block"));
+				DEBUG_POP_LEXICAL_SCOPE(c);
+				return;
+			}
+			// Otherwise just jump to the start block.
+			llvm_emit_br(c, loop_start_block);
+			break;
+		case LOOP_NONE:
+			UNREACHABLE
+	}
+
+	ASSERT(inc_block || body_block || cond_block);
+	ASSERT(loop_start_block != NULL);
+
+	// Break is simple it always jumps out.
+	// For continue:
+	// 1. Jump to inc if it exists
+	// 2. Otherwise jump to cond
+	// 3. If cond doesn't exist, jump to body
+	ASSERT(loop != LOOP_NONE);
+	LLVMBasicBlockRef continue_block = inc_block ? inc_block : (cond_block ? cond_block : body_block);
+
+	ast->for_stmt.codegen.continue_block = continue_block;
+	ast->for_stmt.codegen.exit_block = exit_block;
+
+	if (start_with_cond) llmv_emit_for_cond(c, cond, cond_block, loop_start_block, exit_block);
+
+	// Emit the body
+	if (body_block)
+	{
+		// If we have do-while we jump to the body block.
+		llvm_emit_block(c, body_block);
+		llvm_emit_stmt(c, body);
+		// Was inc used by now? Otherwise we can just continue and emit the incr here.
+		if (c->current_block && inc_block && continue_block == inc_block && llvm_basic_block_is_unused(inc_block))
+		{
+			inc_block = NULL;
+			llvm_emit_ignored_expr(c, exprptr(incr));
+			// If a cond exists, jump to it, otherwise jump to the start block
+			llvm_emit_br(c, cond_block ? cond_block : loop_start_block);
 		}
 		else
 		{
-			inc_block = NULL;
+			// We have a separate inc due to use of continue or no inc
+			llvm_emit_br(c, continue_block);
 		}
 	}
 
-	if (incr)
-	{
-		// We might have neither body nor cond
-		// In that case we do a jump from the init.
-		if (loop_start_block == inc_block)
-		{
-			llvm_emit_br(c, inc_block);
-		}
+	// Emit the inc (if we didn't fold it)
 	if (inc_block)
 	{
 		// Emit the block if it exists.
 		// The inc block might also be the end of the body block.
 		llvm_emit_block(c, inc_block);
-		}
-		if (llvm_get_current_block_if_in_use(c))
-		{
-			if (incr) llvm_emit_ignored_expr(c, exprptr(incr));
-		}
+		llvm_emit_ignored_expr(c, exprptr(incr));
+		// If a cond exists, jump to it, otherwise jump to the start block
+		llvm_emit_br(c, cond_block ? cond_block : loop_start_block);
 	}
 
-	// Loop back.
-	if (loop != LOOP_NONE)
+	if (!start_with_cond && cond_block)
 	{
-		llvm_emit_br(c, loop_start_block);
+		llmv_emit_for_cond(c, cond, cond_block, loop_start_block, exit_block);
 	}
-	else
-	{
+
 	// If the exit block is unused, just skip it.
 	if (llvm_basic_block_is_unused(exit_block))
 	{
 		DEBUG_POP_LEXICAL_SCOPE(c);
 		return;
 	}
-		llvm_emit_br(c, exit_block);
-	}
 
-	// And insert exit block
+	// Emit the exit block.
 	llvm_emit_block(c, exit_block);
 	DEBUG_POP_LEXICAL_SCOPE(c);
 }

test/test_suite/statements/defer_do_while.c3t

@@ -19,20 +19,20 @@ entry:
   store i32 0, ptr %a, align 4
   br label %loop.body
 
-loop.cond:                                        ; preds = %loop.body
-  %0 = load i32, ptr %a, align 4
-  %lt = icmp slt i32 %0, 10
-  br i1 %lt, label %loop.body, label %loop.exit
-
 loop.body:                                        ; preds = %loop.cond, %entry
-  %1 = load i32, ptr %a, align 4
-  %add = add i32 %1, 1
+  %0 = load i32, ptr %a, align 4
+  %add = add i32 %0, 1
   store i32 %add, ptr %a, align 4
-  %2 = load i32, ptr %a, align 4
-  %add1 = add i32 %2, 1
+  %1 = load i32, ptr %a, align 4
+  %add1 = add i32 %1, 1
   store i32 %add1, ptr %a, align 4
   br label %loop.cond
 
+loop.cond:                                        ; preds = %loop.body
+  %2 = load i32, ptr %a, align 4
+  %lt = icmp slt i32 %2, 10
+  br i1 %lt, label %loop.body, label %loop.exit
+
 loop.exit:                                        ; preds = %loop.cond
   ret void
 }

test/test_suite/statements/do_with_no_early_use.c3t

@@ -0,0 +1,95 @@
+module test;
+import std;
+fn int main()
+{
+	int i = 10;
+	do
+	{
+		io::printfn("%s", i);
+	} while (@my_ok(nonzero(--i)));
+	return 0;
+}
+
+macro @my_ok(#z)
+{
+	if (catch #z) return false;
+	return true;
+}
+
+faultdef WAS_ZERO;
+
+fn int? nonzero(int x)
+{
+	if (x == 0) return WAS_ZERO?;
+	return x;
+}
+
+/* #expect: test.ll
+
+define i32 @main() #0 {
+entry:
+  %i = alloca i32, align 4
+  %varargslots = alloca [1 x %any], align 8
+  %retparam = alloca i64, align 8
+  %taddr = alloca %"char[]", align 8
+  %taddr1 = alloca %"any[]", align 8
+  %blockret = alloca i8, align 1
+  %temp_err = alloca i64, align 8
+  %retparam2 = alloca i32, align 4
+  store i32 10, ptr %i, align 4
+  br label %loop.body
+
+loop.body:                                        ; preds = %expr_block.exit, %entry
+  %0 = insertvalue %any undef, ptr %i, 0
+  %1 = insertvalue %any %0, i64 ptrtoint (ptr @"$ct.int" to i64), 1
+  store %any %1, ptr %varargslots, align 8
+  %2 = insertvalue %"any[]" undef, ptr %varargslots, 0
+  %"$$temp" = insertvalue %"any[]" %2, i64 1, 1
+  store %"char[]" { ptr @.str, i64 2 }, ptr %taddr, align 8
+  %3 = load [2 x i64], ptr %taddr, align 8
+  store %"any[]" %"$$temp", ptr %taddr1, align 8
+  %4 = load [2 x i64], ptr %taddr1, align 8
+  %5 = call i64 @std.io.printfn(ptr %retparam, [2 x i64] %3, [2 x i64] %4)
+  br label %loop.cond
+
+loop.cond:                                        ; preds = %loop.body
+  br label %testblock
+
+testblock:                                        ; preds = %loop.cond
+  %6 = load i32, ptr %i, align 4
+  %sub = sub i32 %6, 1
+  store i32 %sub, ptr %i, align 4
+  %7 = call i64 @test.nonzero(ptr %retparam2, i32 %sub)
+  %not_err = icmp eq i64 %7, 0
+  %8 = call i1 @llvm.expect.i1(i1 %not_err, i1 true)
+  br i1 %8, label %after_check, label %assign_optional
+
+assign_optional:                                  ; preds = %testblock
+  store i64 %7, ptr %temp_err, align 8
+  br label %end_block
+
+after_check:                                      ; preds = %testblock
+  store i64 0, ptr %temp_err, align 8
+  br label %end_block
+
+end_block:                                        ; preds = %after_check, %assign_optional
+  %9 = load i64, ptr %temp_err, align 8
+  %i2b = icmp ne i64 %9, 0
+  br i1 %i2b, label %if.then, label %if.exit
+
+if.then:                                          ; preds = %end_block
+  store i8 0, ptr %blockret, align 1
+  br label %expr_block.exit
+
+if.exit:                                          ; preds = %end_block
+  store i8 1, ptr %blockret, align 1
+  br label %expr_block.exit
+
+expr_block.exit:                                  ; preds = %if.exit, %if.then
+  %10 = load i8, ptr %blockret, align 1
+  %11 = trunc i8 %10 to i1
+  br i1 %11, label %loop.body, label %loop.exit
+
+loop.exit:                                        ; preds = %expr_block.exit
+  ret i32 0
+}

test/test_suite/statements/simple_do.c3t

@@ -54,19 +54,12 @@ entry:
   store i32 10, ptr %i, align 4
   br label %loop.body
 
-loop.cond:                                        ; preds = %if.exit
-  %0 = load i32, ptr %i, align 4
-  %sub = sub i32 %0, 1
-  store i32 %sub, ptr %i, align 4
-  %gt = icmp sgt i32 %0, 0
-  br i1 %gt, label %loop.body, label %loop.exit
-
 loop.body:                                        ; preds = %loop.cond, %entry
-  %1 = call i32 @foo.test()
-  call void (ptr, ...) @printf(ptr @.str, i32 %1)
-  %2 = load i32, ptr %i, align 4
-  %gt1 = icmp sgt i32 %2, 100
-  br i1 %gt1, label %if.then, label %if.exit
+  %0 = call i32 @foo.test()
+  call void (ptr, ...) @printf(ptr @.str, i32 %0)
+  %1 = load i32, ptr %i, align 4
+  %gt = icmp sgt i32 %1, 100
+  br i1 %gt, label %if.then, label %if.exit
 
 if.then:                                          ; preds = %loop.body
   br label %loop.exit
@@ -74,33 +67,40 @@ if.then:                                          ; preds = %loop.body
 if.exit:                                          ; preds = %loop.body
   br label %loop.cond
 
-loop.exit:                                        ; preds = %if.then, %loop.cond
+loop.cond:                                        ; preds = %if.exit
+  %2 = load i32, ptr %i, align 4
+  %sub = sub i32 %2, 1
+  store i32 %sub, ptr %i, align 4
+  %gt1 = icmp sgt i32 %2, 0
+  br i1 %gt1, label %loop.body, label %loop.exit
+
+loop.exit:                                        ; preds = %loop.cond, %if.then
   store i32 1, ptr %i, align 4
-  br label %loop.body3
+  br label %loop.body2
 
-loop.cond2:                                       ; preds = %if.exit5
-  %3 = load i32, ptr %i, align 4
-  %add = add i32 %3, 1
-  store i32 %add, ptr %i, align 4
-  %lt = icmp slt i32 %3, 100
-  br i1 %lt, label %loop.body3, label %loop.exit6
-
-loop.body3:                                       ; preds = %loop.cond2, %loop.exit
-  %4 = call i32 @foo.test()
+loop.body2:                                       ; preds = %loop.cond5, %loop.exit
+  %3 = call i32 @foo.test()
+  %4 = load i32, ptr %i, align 4
+  call void (ptr, ...) @printf(ptr @.str.1, i32 %4, i32 %3)
   %5 = load i32, ptr %i, align 4
-  call void (ptr, ...) @printf(ptr @.str.1, i32 %5, i32 %4)
-  %6 = load i32, ptr %i, align 4
-  %smod = srem i32 %6, 3
+  %smod = srem i32 %5, 3
   %eq = icmp eq i32 %smod, 0
-  br i1 %eq, label %if.then4, label %if.exit5
+  br i1 %eq, label %if.then3, label %if.exit4
 
-if.then4:                                         ; preds = %loop.body3
+if.then3:                                         ; preds = %loop.body2
   br label %loop.exit6
 
-if.exit5:                                         ; preds = %loop.body3
-  br label %loop.cond2
+if.exit4:                                         ; preds = %loop.body2
+  br label %loop.cond5
 
-loop.exit6:                                       ; preds = %if.then4, %loop.cond2
+loop.cond5:                                       ; preds = %if.exit4
+  %6 = load i32, ptr %i, align 4
+  %add = add i32 %6, 1
+  store i32 %add, ptr %i, align 4
+  %lt = icmp slt i32 %6, 100
+  br i1 %lt, label %loop.body2, label %loop.exit6
+
+loop.exit6:                                       ; preds = %loop.cond5, %if.then3
   ret void
 }