From c74ab84ea23f497ac83501473220cd9cfefe81e8 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis@chromium.org>
Date: Mon, 11 Oct 2021 14:19:21 -0400
Subject: [PATCH] [clang] Omit most AttributedStatements from the CFG

`[[clang::fallthrough]]` has meaning for the CFG, but all other
StmtAttrs we currently have don't. So omit them, as AttributedStatements
with children cause several issues and there's no benefit in including
them.

Fixes PR52103 and PR49454. See PR52103 for details.

Differential Revision: https://reviews.llvm.org/D111568
---
 clang/lib/Analysis/CFG.cpp                         | 32 ++++++++++++++++++++--
 clang/test/SemaCXX/switch-implicit-fallthrough.cpp |  4 +++
 clang/test/SemaCXX/unreachable-code.cpp            | 22 +++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 50adc8c..69b745c 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@ private:
   // Visitors to walk an AST and construct the CFG.
   CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
+  CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
   CFGBlock *VisitCallExpr(CallExpr *C, AddStmtChoice asc);
@@ -2149,6 +2150,9 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
     case Stmt::InitListExprClass:
       return VisitInitListExpr(cast<InitListExpr>(S), asc);
 
+    case Stmt::AttributedStmtClass:
+      return VisitAttributedStmt(cast<AttributedStmt>(S), asc);
+
     case Stmt::AddrLabelExprClass:
       return VisitAddrLabelExpr(cast<AddrLabelExpr>(S), asc);
 
@@ -2398,8 +2402,32 @@ CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
   return Block;
 }
 
-CFGBlock *CFGBuilder::VisitUnaryOperator(UnaryOperator *U,
-           AddStmtChoice asc) {
+static bool isFallthroughStatement(const AttributedStmt *A) {
+  bool isFallthrough = hasSpecificAttr<FallThroughAttr>(A->getAttrs());
+  assert((!isFallthrough || isa<NullStmt>(A->getSubStmt())) &&
+         "expected fallthrough not to have children");
+  return isFallthrough;
+}
+
+CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *A,
+                                          AddStmtChoice asc) {
+  // AttributedStmts for [[likely]] can have arbitrary statements as children,
+  // and the current visitation order here would add the AttributedStmts
+  // for [[likely]] after the child nodes, which is undesirable: For example,
+  // if the child contains an unconditional return, the [[likely]] would be
+  // considered unreachable.
+  // So only add the AttributedStmt for FallThrough, which has CFG effects and
+  // also no children, and omit the others. None of the other current StmtAttrs
+  // have semantic meaning for the CFG.
+  if (isFallthroughStatement(A) && asc.alwaysAdd(*this, A)) {
+    autoCreateBlock();
+    appendStmt(Block, A);
+  }
+
+  return VisitChildren(A);
+}
+
+CFGBlock *CFGBuilder::VisitUnaryOperator(UnaryOperator *U, AddStmtChoice asc) {
   if (asc.alwaysAdd(*this, U)) {
     autoCreateBlock();
     appendStmt(Block, U);
diff --git a/clang/test/SemaCXX/switch-implicit-fallthrough.cpp b/clang/test/SemaCXX/switch-implicit-fallthrough.cpp
index 0b79081..bb39d63 100644
--- a/clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ b/clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -50,6 +50,8 @@ label_default:
       break;
   }
   switch (n / 20) {
+    [[likely]] case 6:
+      [[clang::fallthrough]];
     case 7:
       n += 400;
       [[clang::fallthrough]];
@@ -73,6 +75,8 @@ label_default:
       n += 800;
   }
   switch (n / 30) {
+    case 6:
+      [[unlikely, clang::fallthrough]];
     case 11:
     case 12:  // no warning here, intended fall-through, no statement between labels
       n += 1600;
diff --git a/clang/test/SemaCXX/unreachable-code.cpp b/clang/test/SemaCXX/unreachable-code.cpp
index 0dfc3d5..6a95f76 100644
--- a/clang/test/SemaCXX/unreachable-code.cpp
+++ b/clang/test/SemaCXX/unreachable-code.cpp
@@ -77,3 +77,25 @@ void weak_redecl() {
     return;
   bar(); // no-warning
 }
+
+namespace pr52103 {
+
+void g(int a);
+
+void f(int a) {
+  if (a > 4) [[ likely ]] { // no-warning
+    return;
+  }
+
+  if (a > 4) [[ unlikely ]] { // no-warning
+    return;
+
+    return; // expected-warning {{will never be executed}}
+  }
+
+  [[clang::musttail]] return g(a); // no-warning
+
+  [[clang::musttail]] return g(a); // expected-warning {{will never be executed}}
+}
+
+}
-- 
2.7.4