Apply proper source location to fallthrough switch cases

[rastogishubham]

Summary

This fixes a bug in clang where, when clang sees a switch with a fallthrough to a default like this:

static void funcA(void) {}
static void funcB(void) {}

int main(int argc, char **argv) {

switch (argc) {
    case 0:
        funcA();
        break;
    case 10:
    default:
        funcB();
        break;
}

}

It does not add a proper debug location for that switch case, such as case 10: above.

The IR output looks like this:
Function Attrs: noinline nounwind optnone ssp uwtable
define i32 @main(i32 %0, i8** %1) #0 !dbg !9 {

%3 = alloca i32, align 4
%4 = alloca i32, align 4
%5 = alloca i8**, align 8
store i32 0, i32* %3, align 4
store i32 %0, i32* %4, align 4
call void @llvm.dbg.declare(metadata i32* %4, metadata !16, metadata !DIExpression()), !dbg !17
store i8** %1, i8*** %5, align 8
call void @llvm.dbg.declare(metadata i8*** %5, metadata !18, metadata !DIExpression()), !dbg !19
%6 = load i32, i32* %4, align 4, !dbg !20
switch i32 %6, label %9 [
  i32 0, label %7
  i32 10, label %8
], !dbg !21

7: ; preds = %2

call void @funcA(), !dbg !22
br label %10, !dbg !24

8: ; preds = %2

br label %9, !dbg !24

9: ; preds = %2, %8

call void @funcB(), !dbg !25
br label %10, !dbg !26

10: ; preds = %9, %7

%11 = load i32, i32* %3, align 4, !dbg !27
ret i32 %11, !dbg !27

}

label 7 and label 8 both have branches but label 8's branch uses the same debug location as label 7's branch. This resolves to disassembly such as:

LBB0_5:
.loc 1 0 5 Documents/test.c:0:5 movl -20(%rbp), %eax 4-byte Reload
.loc 1 5 5 Documents/test.c:5:5 subl $10, %eax je LBB0_2 jmp LBB0_3 LBB0_1: Ltmp1: .loc 1 7 13 is_stmt 1 Documents/test.c:7:13
callq _funcA
.loc 1 8 13 Documents/test.c:8:13 jmp LBB0_4 LBB0_2: jmp LBB0_3 LBB0_3: .loc 1 11 13 Documents/test.c:11:13
callq _funcB

LBB0_2 does not get a .loc in this case

[aprantl]

Thanks! I have one question inline.

clang/lib/CodeGen/CGStmt.cpp

1523

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?

clang/test/CodeGen/switch-fallthrough.c

1 ↗

(On Diff #373120)

Is the -v necessary?

[rastogishubham]

The -v on the lit test has been removed

[rastogishubham]

clang/lib/CodeGen/CGStmt.cpp

1523

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.

clang/test/CodeGen/switch-fallthrough.c

1 ↗

(On Diff #373120)

The -v is not necessary, I had added that for debugging, I was supposed to remove it. Do I just paste another diff?

[aprantl]

clang/lib/CodeGen/CGStmt.cpp

1523

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:

Interesting. Thanks!

clang/lib/CodeGen/CGStmt.cpp

while (NextCase && NextCase->getRHS() == nullptr) {
     // Since this loop is only executed when the CaseStmt has no attributes
     // use a hard-coded value.
     if (SwitchLikelihood)
       SwitchLikelihood->push_back(Stmt::LH_None);
 
     SwitchInsn->addCase(CaseVal, CaseDest);
     NextCase = dyn_cast<CaseStmt>(CurCase->getSubStmt());
   }
 
+  // Generate a stop point for debug info if the case statement is
+  // followed by a default statement. A fallthrough case before a
+  // default case gets its own branch target.
+  if (CurCase->getSubStmt()->getStmtClass() == Stmt::DefaultStmtClass)
+    EmitStopPoint(CurCase);
+
   // Normal default recursion for non-cases.
   EmitStmt(CurCase->getSubStmt());
 }
 
 void CodeGenFunction::EmitDefaultStmt(const DefaultStmt &S,
                                       ArrayRef<const Attr *> Attrs) {
   // If there is no enclosing switch instance that we're aware of, then this
   // default statement can be elided. This situation only happens when we've

clang/test/CodeGen/debug-info-switch-fallthrough.c

+// RUN:  %clang_cc1 -triple x86_64-apple-macosx11.0.0 -debug-info-kind=standalone -emit-llvm %s -o - | FileCheck %s
+// CHECK: ], !dbg !{{[0-9]+}}
+// CHECK-EMPTY:
+// CHECK-NEXT: {{.+}}
+// CHECK-NEXT: br {{.+}}, !dbg !{{[0-9+]}}
+// CHECK-EMPTY:
+// CHECK-NEXT: {{.+}}
+// CHECK-NEXT: br {{.+}}, !dbg ![[LOC:[0-9]+]]
+void test(int num) {
+  switch (num) {
+  case 0:
+    break;
+  case 10: // CHECK: ![[LOC]] = !DILocation(line: [[@LINE]], column:{{.+}}, scope: {{.+}})
+  default:
+    break;
+  }
+}