[-Wunreachable-code] Don't warn about dead code guarded by a "configuration value".

Some unreachable code is only "sometimes unreachable" because it
is guarded by a configuration value that is determined at compile
time and is always constant.  Sometimes those represent real bugs,
but often they do not.  This patch causes the reachability analysis
to cover such branches even if they are technically unreachable
in the CFG itself.  There are some conservative heuristics at
play here to determine a "configuration value"; these are intended
to be refined over time.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@202912 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/ReachableCode.cpp b/lib/Analysis/ReachableCode.cpp
index b0e92504260b4cbdf05a1d5ea9df5b2a5474e4e1..5baaa006777f02b4fa08860cfc2441d93a9e5b1e 100644 (file)
--- a/lib/Analysis/ReachableCode.cpp
+++ b/lib/Analysis/ReachableCode.cpp
@@ -351,6 +351,56 @@ namespace clang { namespace reachable_code {
 
 void Callback::anchor() { }  
 
+/// Returns true if the statement is expanded from a configuration macro.
+static bool isExpandedFromConfigurationMacro(const Stmt *S) {
+  // FIXME: This is not very precise.  Here we just check to see if the
+  // value comes from a macro, but we can do much better.  This is likely
+  // to be over conservative.  This logic is factored into a separate function
+  // so that we can refine it later.
+  SourceLocation L = S->getLocStart();
+  return L.isMacroID();
+}
+
+/// Returns true if the statement represents a configuration value.
+///
+/// A configuration value is something usually determined at compile-time
+/// to conditionally always execute some branch.  Such guards are for
+/// "sometimes unreachable" code.  Such code is usually not interesting
+/// to report as unreachable, and may mask truly unreachable code within
+/// those blocks.
+static bool isConfigurationValue(const Stmt *S) {
+  if (!S)
+    return false;
+
+  if (const Expr *Ex = dyn_cast<Expr>(S))
+    S = Ex->IgnoreParenCasts();
+
+  switch (S->getStmtClass()) {
+    case Stmt::IntegerLiteralClass:
+      return isExpandedFromConfigurationMacro(S);
+    case Stmt::UnaryExprOrTypeTraitExprClass:
+      return true;
+    case Stmt::BinaryOperatorClass: {
+      const BinaryOperator *B = cast<BinaryOperator>(S);
+      return (B->isLogicalOp() || B->isRelationalOp()) &&
+             (isConfigurationValue(B->getLHS()) ||
+              isConfigurationValue(B->getRHS()));
+    }
+    case Stmt::UnaryOperatorClass: {
+      const UnaryOperator *UO = cast<UnaryOperator>(S);
+      return UO->getOpcode() == UO_LNot &&
+             isConfigurationValue(UO->getSubExpr());
+    }
+    default:
+      return false;
+  }
+}
+
+/// Returns true if we should always explore all successors of a block.
+static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B) {
+  return isConfigurationValue(B->getTerminatorCondition());
+}
+
 unsigned ScanReachableFromBlock(const CFGBlock *Start,
                                 llvm::BitVector &Reachable) {
   unsigned count = 0;
@@ -370,13 +420,28 @@ unsigned ScanReachableFromBlock(const CFGBlock *Start,
   // Find the reachable blocks from 'Start'.
   while (!WL.empty()) {
     const CFGBlock *item = WL.pop_back_val();
-    
+
+    // There are cases where we want to treat all successors as reachable.
+    // The idea is that some "sometimes unreachable" code is not interesting,
+    // and that we should forge ahead and explore those branches anyway.
+    // This allows us to potentially uncover some "always unreachable" code
+    // within the "sometimes unreachable" code.
+    bool TreatAllSuccessorsAsReachable = shouldTreatSuccessorsAsReachable(item);
+
     // Look at the successors and mark then reachable.
     for (CFGBlock::const_succ_iterator I = item->succ_begin(), 
          E = item->succ_end(); I != E; ++I) {
       const CFGBlock *B = *I;
-      if (!B) {
-        //
+      if (!B) do {
+        const CFGBlock *UB = I->getPossiblyUnreachableBlock();
+        if (!UB)
+          break;
+
+        if (TreatAllSuccessorsAsReachable) {
+          B = UB;
+          break;
+        }
+
         // For switch statements, treat all cases as being reachable.
         // There are many cases where a switch can contain values that
         // are not in an enumeration but they are still reachable because
@@ -391,13 +456,12 @@ unsigned ScanReachableFromBlock(const CFGBlock *Start,
         // this we can either put more heuristics here, or possibly retain
         // that information in the CFG itself.
         //
-        if (const CFGBlock *UB = I->getPossiblyUnreachableBlock()) {
-          const Stmt *Label = UB->getLabel();
-          if (Label && isa<SwitchCase>(Label)) {
-            B = UB;
-          }
-        }
+        const Stmt *Label = UB->getLabel();
+        if (Label && isa<SwitchCase>(Label))
+          B = UB;
       }
+      while (false);
+
       if (B) {
         unsigned blockID = B->getBlockID();
         if (!Reachable[blockID]) {

diff --git a/test/Sema/warn-unreachable.c b/test/Sema/warn-unreachable.c
index 630b623b58ab9cd73a81c388a28f957a2e5762c0..b2448e987835a038f891f79c4c783ad7a1df7ed5 100644 (file)
--- a/test/Sema/warn-unreachable.c
+++ b/test/Sema/warn-unreachable.c
@@ -136,7 +136,7 @@ void PR9774(int *s) {
 
 // Test case for <rdar://problem/11005770>.  We should treat code guarded
 // by 'x & 0' and 'x * 0' as unreachable.
-void calledFun();
+int calledFun();
 void test_mul_and_zero(int x) {
   if (x & 0) calledFun(); // expected-warning {{will never be executed}}
   if (0 & x) calledFun(); // expected-warning {{will never be executed}}
@@ -203,3 +203,26 @@ MyEnum trival_dead_return_enum() {
   return Value1; // no-warning
 }
 
+// Test unreachable code depending on configuration values
+#define CONFIG_CONSTANT 1
+int test_config_constant(int x) {
+  if (!CONFIG_CONSTANT) {
+    calledFun(); // no-warning
+    return 1;
+  }
+  if (!1) {
+    calledFun(); // expected-warning {{will never be executed}}
+    return 1;
+  }
+  if (sizeof(int) > sizeof(char)) {
+    calledFun(); // no-warning
+    return 1;
+  }
+  if (x > 10)
+    return CONFIG_CONSTANT ? calledFun() : calledFun(); // no-warning
+  else
+    return 1 ?
+      calledFun() :
+      calledFun(); // expected-warning {{will never be executed}}
+}
+