[PATCH] D109940: Fixed bug with clang where a fallthrough switch statement wasn't getting proper debug information

[Shubham Sandeep Rastogi via Phabricator via llvm-commits]

rastogishubham added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:1523
+  // by a default statement.
+  if (CurCase->getSubStmt()->getStmtClass() == Stmt::DefaultStmtClass) {
+    EmitStopPoint(CurCase);
----------------
aprantl wrote:
> Is the default statement special in a meaningful way here, or could a more general patch also work for a switch statement like:
> ```
>   switch (num) {
>   case 0:
>     break;
>   case 10: // break here
>   case 11:
>      break;
>   default:
> }
> ```
> 
> In other words, should wee just unconditionally emit a stop point?
So if you have a fallthrough to another case statement, clang doesn't generate a branch just like it does for the default, for example:

void func(int num) {
    switch (num) {
    case 0:
        break;
    case 10: // break here
    case 11:
        break;
    default:
        break;
    }
}

dasm:

tmp0:
	.loc	1 2 13 prologue_end             ## switchFallthruCaseStmt.c:2:13
	movl	-4(%rbp), %eax
	.loc	1 2 5 is_stmt 0                 ## switchFallthruCaseStmt.c:2:5
	testl	%eax, %eax
	movl	%eax, -8(%rbp)                  ## 4-byte Spill
	je	LBB0_1
	jmp	LBB0_5
LBB0_5:
	.loc	1 0 5                           ## switchFallthruCaseStmt.c:0:5
	movl	-8(%rbp), %eax                  ## 4-byte Reload
	.loc	1 2 5                           ## switchFallthruCaseStmt.c:2:5
	addl	$-10, %eax
	subl	$2, %eax
	jb	LBB0_2
	jmp	LBB0_3
LBB0_1:
Ltmp1:
	.loc	1 4 9 is_stmt 1                 ## switchFallthruCaseStmt.c:4:9
	jmp	LBB0_4
LBB0_2:
	.loc	1 7 9                           ## switchFallthruCaseStmt.c:7:9
	jmp	LBB0_4
LBB0_3:
	.loc	1 9 9                           ## switchFallthruCaseStmt.c:9:9
	jmp	LBB0_4
Ltmp2:
LBB0_4:
	.loc	1 11 1                          ## switchFallthruCaseStmt.c:11:1
	popq	%rbp
	retq

If you notice, clang has basically mashed the case 10 and 11 together and not generated a jump from case 10 to case 11. So I think it only happens when there is a case statement followed by a default.


================
Comment at: clang/test/CodeGen/switch-fallthrough.c:1
+// RUN:  %clang_cc1 -triple x86_64-apple-macosx11.0.0 -debug-info-kind=standalone -emit-llvm %s -o - | FileCheck %s -v
+// CHECK: ], !dbg !{{[0-9]+}}
----------------
aprantl wrote:
> Is the -v necessary?
The -v is not necessary, I had added that for debugging, I was supposed to remove it. Do I just paste another diff? 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109940/new/

https://reviews.llvm.org/D109940