r202912 - [-Wunreachable-code] Don't warn about dead code guarded by a "configuration value".
Ted Kremenek
kremenek at apple.com
Tue Mar 4 16:01:17 PST 2014
Author: kremenek
Date: Tue Mar 4 18:01:17 2014
New Revision: 202912
URL: http://llvm.org/viewvc/llvm-project?rev=202912&view=rev
Log:
[-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.
Modified:
cfe/trunk/lib/Analysis/ReachableCode.cpp
cfe/trunk/test/Sema/warn-unreachable.c
Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=202912&r1=202911&r2=202912&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp Tue Mar 4 18:01:17 2014
@@ -351,6 +351,56 @@ namespace clang { namespace reachable_co
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 CF
// 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 CF
// 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]) {
Modified: cfe/trunk/test/Sema/warn-unreachable.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unreachable.c?rev=202912&r1=202911&r2=202912&view=diff
==============================================================================
--- cfe/trunk/test/Sema/warn-unreachable.c (original)
+++ cfe/trunk/test/Sema/warn-unreachable.c Tue Mar 4 18:01:17 2014
@@ -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}}
+}
+
More information about the cfe-commits
mailing list