From e5c0994ddfcf64bbc7e65ebb68890e98141b84d9 Mon Sep 17 00:00:00 2001
From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Thu, 29 Aug 2019 20:37:28 +0000
Subject: [PATCH] [CFG] Fix CFG for statement-expressions in return values.

We're building the CFG from bottom to top, so when the return-value expression
has a non-trivial CFG on its own, we need to continue building from the entry
to the return-value expression CFG rather than from the block to which
we've just appended the return statement.

Fixes a false positive warning "control may reach end of non-void function".

llvm-svn: 370406
---
 clang/lib/Analysis/CFG.cpp  |  5 ++--
 clang/test/Analysis/cfg.cpp | 49 ++++++++++++++++++++++++++++++++++++-
 clang/test/Sema/return.c    | 11 +++++++++
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index e4ed0f86b918a..fb835d43680a0 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2994,9 +2994,8 @@ CFGBlock *CFGBuilder::VisitReturnStmt(Stmt *S) {
 
   // Visit children
   if (ReturnStmt *RS = dyn_cast<ReturnStmt>(S)) {
-    Expr *O = RS->getRetValue();
-    if (O)
-      Visit(O, AddStmtChoice::AlwaysAdd, /*ExternallyDestructed=*/true);
+    if (Expr *O = RS->getRetValue())
+      return Visit(O, AddStmtChoice::AlwaysAdd, /*ExternallyDestructed=*/true);
     return Block;
   } else { // co_return
     return VisitChildren(S);
diff --git a/clang/test/Analysis/cfg.cpp b/clang/test/Analysis/cfg.cpp
index 74f4d50f25dfc..9b0203e99efe9 100644
--- a/clang/test/Analysis/cfg.cpp
+++ b/clang/test/Analysis/cfg.cpp
@@ -499,6 +499,54 @@ void foo() {
 } // end namespace pr37688_deleted_union_destructor
 
 
+namespace return_statement_expression {
+int unknown();
+
+// CHECK-LABEL: int foo()
+// CHECK:       [B6 (ENTRY)]
+// CHECK-NEXT:    Succs (1): B5
+// CHECK:       [B1]
+// CHECK-NEXT:    1: 0
+// CHECK-NEXT:    2: return [B1.1];
+// CHECK-NEXT:    Preds (1): B5
+// CHECK-NEXT:    Succs (1): B0
+// CHECK:       [B2]
+// CHECK-NEXT:    1: 0
+// CHECK-NEXT:    2: ({ ... ; [B2.1] })
+// CHECK-NEXT:    3: return [B2.2];
+// CHECK-NEXT:    Preds (1): B4
+// CHECK-NEXT:    Succs (1): B0
+// FIXME: Why do we have [B3] at all?
+// CHECK:       [B3]
+// CHECK-NEXT:    Succs (1): B4
+// CHECK:       [B4]
+// CHECK-NEXT:    1: 0
+// CHECK-NEXT:    2: [B4.1] (ImplicitCastExpr, IntegralToBoolean, _Bool)
+// CHECK-NEXT:    T: while [B4.2]
+// CHECK-NEXT:    Preds (2): B3 B5
+// CHECK-NEXT:    Succs (2): NULL B2
+// CHECK:       [B5]
+// CHECK-NEXT:    1: unknown
+// CHECK-NEXT:    2: [B5.1] (ImplicitCastExpr, FunctionToPointerDecay, int (*)(void))
+// CHECK-NEXT:    3: [B5.2]()
+// CHECK-NEXT:    4: [B5.3] (ImplicitCastExpr, IntegralToBoolean, _Bool)
+// CHECK-NEXT:    T: if [B5.4]
+// CHECK-NEXT:    Preds (1): B6
+// CHECK-NEXT:    Succs (2): B4 B1
+// CHECK:       [B0 (EXIT)]
+// CHECK-NEXT:    Preds (2): B1 B2
+int foo() {
+  if (unknown())
+    return ({
+      while (0)
+        ;
+      0;
+    });
+  else
+    return 0;
+}
+} // namespace statement_expression_in_return
+
 // CHECK-LABEL: template<> int *PR18472<int>()
 // CHECK: [B2 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B1
@@ -522,4 +570,3 @@ template <class T> T *PR18472() {
 void PR18472_helper() {
   PR18472<int>();
 }
-
diff --git a/clang/test/Sema/return.c b/clang/test/Sema/return.c
index debf5ab55f5b5..68c2251c46330 100644
--- a/clang/test/Sema/return.c
+++ b/clang/test/Sema/return.c
@@ -328,3 +328,14 @@ int sizeof_long() {
   if (sizeof(long) == 8)
     return 2;
 } // no-warning
+
+int return_statement_expression() {
+  if (unknown())
+    return ({
+      while (0)
+        ;
+      0;
+    });
+  else
+    return 0;
+} // no-warning (used to be "control may reach end of non-void function")