[analyzer] Add BranchCondition callback to 'switch' (llvm#182058)

Previously the condition of a 'switch' statement did not trigger a
`BranchCondition` callback. This commit resolves this old FIXME and e.g.
lets the checker `core.uninitialzed.Branch` report code where the
condition of a `switch` statement is undefined.

This commit also contains a very small unrelated change that removes a
short fragment of dead code from `processBranch`.

Author: NagyDonat

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -2872,9 +2872,6 @@ void ExprEngine::processBranch(
 
   BranchNodeBuilder Builder(Dst, BldCtx, DstT, DstF);
   for (ExplodedNode *PredN : CheckersOutSet) {
-    if (PredN->isSink())
-      continue;
-
     ProgramStateRef PrevState = PredN->getState();
 
     ProgramStateRef StTrue = PrevState, StFalse = PrevState;
@@ -3106,70 +3103,83 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
 ///  nodes by processing the 'effects' of a switch statement.
 void ExprEngine::processSwitch(NodeBuilderContext &BC, const SwitchStmt *Switch,
                                ExplodedNode *Pred, ExplodedNodeSet &Dst) {
+  currBldrCtx = &BC;
   const Expr *Condition = Switch->getCond();
 
   SwitchNodeBuilder Builder(Dst, BC);
+  ExplodedNodeSet CheckersOutSet;
 
-  ProgramStateRef State = Pred->getState();
-  SVal CondV = State->getSVal(Condition, Pred->getLocationContext());
-
-  if (CondV.isUndef()) {
-    // FIXME: Emit warnings when the switch condition is undefined.
-    return;
-  }
+  getCheckerManager().runCheckersForBranchCondition(
+      Condition->IgnoreParens(), CheckersOutSet, Pred, *this);
 
-  std::optional<NonLoc> CondNL = CondV.getAs<NonLoc>();
+  for (ExplodedNode *Node : CheckersOutSet) {
+    ProgramStateRef State = Node->getState();
 
-  for (const CFGBlock *Block : Builder) {
-    // Successor may be pruned out during CFG construction.
-    if (!Block)
+    SVal CondV = State->getSVal(Condition, Node->getLocationContext());
+    if (CondV.isUndef()) {
+      // This can only happen if core.uninitialized.Branch is disabled.
       continue;
+    }
 
-    const CaseStmt *Case = cast<CaseStmt>(Block->getLabel());
+    std::optional<NonLoc> CondNL = CondV.getAs<NonLoc>();
 
-    // Evaluate the LHS of the case value.
-    llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(getContext());
-    assert(V1.getBitWidth() == getContext().getIntWidth(Condition->getType()));
+    for (const CFGBlock *Block : Builder) {
+      // Successor may be pruned out during CFG construction.
+      if (!Block)
+        continue;
 
-    // Get the RHS of the case, if it exists.
-    llvm::APSInt V2;
-    if (const Expr *E = Case->getRHS())
-      V2 = E->EvaluateKnownConstInt(getContext());
-    else
-      V2 = V1;
+      const CaseStmt *Case = cast<CaseStmt>(Block->getLabel());
 
-    ProgramStateRef StateMatching;
-    if (CondNL) {
-      // Split the state: this "case:" matches / does not match.
-      std::tie(StateMatching, State) =
-          State->assumeInclusiveRange(*CondNL, V1, V2);
-    } else {
-      // The switch condition is UnknownVal, so we enter each "case:" without
-      // any state update.
-      StateMatching = State;
-    }
+      // Evaluate the LHS of the case value.
+      llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(getContext());
+      assert(V1.getBitWidth() ==
+             getContext().getIntWidth(Condition->getType()));
 
-    if (StateMatching)
-      Builder.generateCaseStmtNode(Block, StateMatching, Pred);
+      // Get the RHS of the case, if it exists.
+      llvm::APSInt V2;
+      if (const Expr *E = Case->getRHS())
+        V2 = E->EvaluateKnownConstInt(getContext());
+      else
+        V2 = V1;
 
-    // If _not_ entering the current case is infeasible, we are done with
-    // processing this branch.
+      ProgramStateRef StateMatching;
+      if (CondNL) {
+        // Split the state: this "case:" matches / does not match.
+        std::tie(StateMatching, State) =
+            State->assumeInclusiveRange(*CondNL, V1, V2);
+      } else {
+        // The switch condition is UnknownVal, so we enter each "case:" without
+        // any state update.
+        StateMatching = State;
+      }
+
+      if (StateMatching)
+        Builder.generateCaseStmtNode(Block, StateMatching, Node);
+
+      // If _not_ entering the current case is infeasible, then we are done
+      // with processing the paths through the current Node.
+      if (!State)
+        break;
+    }
     if (!State)
-      return;
-  }
-  // If we have switch(enum value), the default branch is not
-  // feasible if all of the enum constants not covered by 'case:' statements
-  // are not feasible values for the switch condition.
-  //
-  // Note that this isn't as accurate as it could be.  Even if there isn't
-  // a case for a particular enum value as long as that enum value isn't
-  // feasible then it shouldn't be considered for making 'default:' reachable.
-  if (Condition->IgnoreParenImpCasts()->getType()->isEnumeralType()) {
-    if (Switch->isAllEnumCasesCovered())
-      return;
+      continue;
+
+    // If we have switch(enum value), the default branch is not
+    // feasible if all of the enum constants not covered by 'case:' statements
+    // are not feasible values for the switch condition.
+    //
+    // Note that this isn't as accurate as it could be.  Even if there isn't
+    // a case for a particular enum value as long as that enum value isn't
+    // feasible then it shouldn't be considered for making 'default:' reachable.
+    if (Condition->IgnoreParenImpCasts()->getType()->isEnumeralType()) {
+      if (Switch->isAllEnumCasesCovered())
+        continue;
+    }
+
+    Builder.generateDefaultCaseNode(State, Node);
   }
 
-  Builder.generateDefaultCaseNode(State, Pred);
+  currBldrCtx = nullptr;
 }
 
 //===----------------------------------------------------------------------===//

clang/test/Analysis/uninitialized-branch.c

@@ -0,0 +1,61 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -Wno-uninitialized -verify %s
+
+int if_cond(void) {
+  int foo;
+  if (foo) //expected-warning {{Branch condition evaluates to a garbage value}}
+    return 1;
+  return 2;
+}
+
+int logical_op_and_if_cond(void) {
+  int foo, bar;
+  if (foo && bar) //expected-warning {{Branch condition evaluates to a garbage value}}
+    return 1;
+  return 2;
+}
+
+int logical_op_cond(int arg) {
+  int foo;
+  if (foo && arg) //expected-warning {{Branch condition evaluates to a garbage value}}
+    return 1;
+  return 2;
+}
+
+int if_cond_after_logical_op(int arg) {
+  int foo;
+  if (arg && foo) //expected-warning {{Branch condition evaluates to a garbage value}}
+    return 1;
+  return 2;
+}
+
+int ternary_cond(void) {
+  int foo;
+  return foo ? 1 : 2; //expected-warning {{Branch condition evaluates to a garbage value}}
+}
+
+int while_cond(void) {
+  int foo;
+  while (foo) //expected-warning {{Branch condition evaluates to a garbage value}}
+    return 1;
+  return 2;
+}
+
+int do_while_cond(void) {
+  int foo, bar;
+  do {
+    foo = 43;
+  } while (bar); //expected-warning {{Branch condition evaluates to a garbage value}}
+  return foo;
+}
+
+int switch_cond(void) {
+  int foo;
+  switch (foo) { //expected-warning {{Branch condition evaluates to a garbage value}}
+    case 1:
+      return 3;
+    case 2:
+      return 440;
+    default:
+      return 6772;
+  }
+}

clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp

@@ -362,6 +362,7 @@ TEST(BlockEntranceTester, BlockEntranceVSBranchCondition) {
                 "Within 'top' B5 -> B3",
                 "Within 'top' B6 -> B4",
                 "Within 'top': branch condition 'x == 6'",
+                "Within 'top': branch condition 'x'",
             }),
             Diags);
 }