From c91095489745c77c9f1c02ac4cfdef65e2fc9321 Mon Sep 17 00:00:00 2001 From: rillig Date: Sat, 15 May 2021 19:12:14 +0000 Subject: [PATCH] lint: warn about unreachable case labels for '&&' See octeon_gmxreg.h 1.2 from 2020-06-18 for an example, where RXN_RX_INBND_SPEED was cleaned up without adjusting the corresponding code in octeon_gmx.c. --- tests/usr.bin/xlint/lint1/Makefile | 3 +- tests/usr.bin/xlint/lint1/expr_range.c | 38 +++++++++++++++----------- usr.bin/xlint/lint1/func.c | 30 ++++++++++++++++++-- usr.bin/xlint/lint1/lint1.h | 3 +- 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/tests/usr.bin/xlint/lint1/Makefile b/tests/usr.bin/xlint/lint1/Makefile index f225807b3f2a..60d7ca488e11 100644 --- a/tests/usr.bin/xlint/lint1/Makefile +++ b/tests/usr.bin/xlint/lint1/Makefile @@ -1,4 +1,4 @@ -# $NetBSD: Makefile,v 1.56 2021/05/14 21:14:55 rillig Exp $ +# $NetBSD: Makefile,v 1.57 2021/05/15 19:12:14 rillig Exp $ NOMAN= # defined MAX_MESSAGE= 343 # see lint1/err.c @@ -104,6 +104,7 @@ FILES+= emit.c FILES+= emit.exp FILES+= emit.ln FILES+= expr_range.c +FILES+= expr_range.exp FILES+= feat_stacktrace.c FILES+= feat_stacktrace.exp FILES+= gcc_attribute.c diff --git a/tests/usr.bin/xlint/lint1/expr_range.c b/tests/usr.bin/xlint/lint1/expr_range.c index 79e98201d3b1..0feed7a26154 100644 --- a/tests/usr.bin/xlint/lint1/expr_range.c +++ b/tests/usr.bin/xlint/lint1/expr_range.c @@ -1,37 +1,43 @@ -/* $NetBSD: expr_range.c,v 1.1 2021/05/14 21:14:55 rillig Exp $ */ +/* $NetBSD: expr_range.c,v 1.2 2021/05/15 19:12:14 rillig Exp $ */ # 3 "expr_range.c" /* - * Test whether lint can detect switch branches that are impossible to reach. - * As of 2021-05-14, it cannot. To do this, it would need to keep track of - * the possible values of each variable or expression. To do this reliably, - * it would also need accurate control flow analysis, which as of 2021-05-14 - * works only for functions without 'goto'. + * In a switch statement that has (expr & constant) as the controlling + * expression, complain if one of the case branches is unreachable because + * the case label does can never match the controlling expression. * - * GCC 10 does not complain the unreachable branch. It knows that the branch - * is unreachable though since it doesn't generate any code for it. GCC once - * had the option -Wunreachable-code, but that option was made a no-op in - * 2011. + * GCC 10 does not complain about the unreachable branch. It knows that the + * branch is unreachable though since it doesn't generate any code for it. + * GCC once had the option -Wunreachable-code, but that option was made a + * no-op in 2011. * * Clang 10 does not complain about this either, and just like GCC it doesn't * generate any code for this branch. The code for tracking an expression's * possible values may be related to RangeConstraintManager, just guessing. - * - * There should be at least one static analysis tool that warns about this. */ /* lint1-extra-flags: -chap */ +void println(const char *); + void example(unsigned x) { switch (x & 6) { - case 1: - /* This branch is unreachable. */ - printf("one\n"); + case 0: + println("0 is reachable"); + break; + case 1: /* expect: statement not reached */ + println("1 is not reachable"); break; case 2: - printf("two\n"); + println("2 is reachable"); + break; + case 6: + println("6 is reachable"); + break; + case 7: /* expect: statement not reached */ + println("7 is not reachable"); break; } } diff --git a/usr.bin/xlint/lint1/func.c b/usr.bin/xlint/lint1/func.c index 7ca01e34966e..1ae3261d2c04 100644 --- a/usr.bin/xlint/lint1/func.c +++ b/usr.bin/xlint/lint1/func.c @@ -1,4 +1,4 @@ -/* $NetBSD: func.c,v 1.107 2021/05/03 07:08:54 rillig Exp $ */ +/* $NetBSD: func.c,v 1.108 2021/05/15 19:12:14 rillig Exp $ */ /* * Copyright (c) 1994, 1995 Jochen Pohl @@ -37,7 +37,7 @@ #include #if defined(__RCSID) && !defined(lint) -__RCSID("$NetBSD: func.c,v 1.107 2021/05/03 07:08:54 rillig Exp $"); +__RCSID("$NetBSD: func.c,v 1.108 2021/05/15 19:12:14 rillig Exp $"); #endif #include @@ -439,6 +439,25 @@ named_label(sym_t *sym) set_reached(true); } +static void +check_case_label_bitand(const tnode_t *case_expr, const tnode_t *switch_expr) +{ + uint64_t case_value, mask; + + if (switch_expr->tn_op != BITAND || + switch_expr->tn_right->tn_op != CON) + return; + + lint_assert(case_expr->tn_op == CON); + case_value = case_expr->tn_val->v_quad; + mask = switch_expr->tn_right->tn_val->v_quad; + + if ((case_value & ~mask) != 0) { + /* statement not reached */ + warning(193); + } +} + static void check_case_label_enum(const tnode_t *tn, const cstk_t *ci) { @@ -483,6 +502,7 @@ check_case_label(tnode_t *tn, cstk_t *ci) return; } + check_case_label_bitand(tn, ci->c_switch_expr); check_case_label_enum(tn, ci); lint_assert(ci->c_switch_type != NULL); @@ -694,12 +714,16 @@ switch1(tnode_t *tn) tp->t_tspec = INT; } + /* leak the memory, for check_case_label_bitand */ + expr_save_memory(); + check_getopt_begin_switch(); - expr(tn, true, false, true, false); + expr(tn, true, false, false, false); begin_control_statement(CS_SWITCH); cstmt->c_switch = true; cstmt->c_switch_type = tp; + cstmt->c_switch_expr = tn; set_reached(false); seen_fallthrough = true; diff --git a/usr.bin/xlint/lint1/lint1.h b/usr.bin/xlint/lint1/lint1.h index 9f00562366dd..b82070561c84 100644 --- a/usr.bin/xlint/lint1/lint1.h +++ b/usr.bin/xlint/lint1/lint1.h @@ -1,4 +1,4 @@ -/* $NetBSD: lint1.h,v 1.100 2021/04/18 17:47:32 rillig Exp $ */ +/* $NetBSD: lint1.h,v 1.101 2021/05/15 19:12:14 rillig Exp $ */ /* * Copyright (c) 1996 Christopher G. Demetriou. All Rights Reserved. @@ -416,6 +416,7 @@ typedef struct control_statement { bool c_had_return_value : 1; /* had "return expr;" */ type_t *c_switch_type; /* type of switch expression */ + tnode_t *c_switch_expr; case_label_t *c_case_labels; /* list of case values */ struct memory_block *c_for_expr3_mem; /* saved memory for end of loop