From 746e9c89cfd0ea856c084ebe498382540d9d841d Mon Sep 17 00:00:00 2001 From: rillig Date: Fri, 19 Feb 2021 12:28:56 +0000 Subject: [PATCH] lint: warn about mismatch in getopt handling --- distrib/sets/lists/tests/mi | 6 +- tests/usr.bin/xlint/lint1/Makefile | 4 +- tests/usr.bin/xlint/lint1/msg_338.c | 45 ++++++ tests/usr.bin/xlint/lint1/msg_338.exp | 4 + tests/usr.bin/xlint/lint1/msg_339.c | 45 ++++++ tests/usr.bin/xlint/lint1/msg_339.exp | 4 + tests/usr.bin/xlint/lint1/t_integration.sh | 4 +- usr.bin/xlint/lint1/Makefile | 6 +- usr.bin/xlint/lint1/ckgetopt.c | 178 +++++++++++++++++++++ usr.bin/xlint/lint1/err.c | 6 +- usr.bin/xlint/lint1/externs1.h | 11 +- usr.bin/xlint/lint1/func.c | 13 +- 12 files changed, 312 insertions(+), 14 deletions(-) create mode 100644 tests/usr.bin/xlint/lint1/msg_338.c create mode 100644 tests/usr.bin/xlint/lint1/msg_338.exp create mode 100644 tests/usr.bin/xlint/lint1/msg_339.c create mode 100644 tests/usr.bin/xlint/lint1/msg_339.exp create mode 100644 usr.bin/xlint/lint1/ckgetopt.c diff --git a/distrib/sets/lists/tests/mi b/distrib/sets/lists/tests/mi index e05521dc4803..cc0379c2a186 100644 --- a/distrib/sets/lists/tests/mi +++ b/distrib/sets/lists/tests/mi @@ -1,4 +1,4 @@ -# $NetBSD: mi,v 1.1019 2021/02/16 09:46:24 kre Exp $ +# $NetBSD: mi,v 1.1020 2021/02/19 12:28:56 rillig Exp $ # # Note: don't delete entries from here - mark them as "obsolete" instead. # @@ -6511,6 +6511,10 @@ ./usr/tests/usr.bin/xlint/lint1/msg_336.exp tests-usr.bin-tests compattestfile,atf ./usr/tests/usr.bin/xlint/lint1/msg_337.c tests-usr.bin-tests compattestfile,atf ./usr/tests/usr.bin/xlint/lint1/msg_337.exp tests-usr.bin-tests compattestfile,atf +./usr/tests/usr.bin/xlint/lint1/msg_338.c tests-usr.bin-tests compattestfile,atf +./usr/tests/usr.bin/xlint/lint1/msg_338.exp tests-usr.bin-tests compattestfile,atf +./usr/tests/usr.bin/xlint/lint1/msg_339.c tests-usr.bin-tests compattestfile,atf +./usr/tests/usr.bin/xlint/lint1/msg_339.exp tests-usr.bin-tests compattestfile,atf ./usr/tests/usr.bin/xlint/lint1/t_integration tests-usr.bin-tests compattestfile,atf ./usr/tests/usr.bin/ztest tests-usr.bin-tests compattestfile,atf ./usr/tests/usr.bin/ztest/Atffile tests-usr.bin-tests compattestfile,atf diff --git a/tests/usr.bin/xlint/lint1/Makefile b/tests/usr.bin/xlint/lint1/Makefile index c1a8456e3868..a13133877209 100644 --- a/tests/usr.bin/xlint/lint1/Makefile +++ b/tests/usr.bin/xlint/lint1/Makefile @@ -1,4 +1,4 @@ -# $NetBSD: Makefile,v 1.30 2021/01/17 23:00:41 rillig Exp $ +# $NetBSD: Makefile,v 1.31 2021/02/19 12:28:56 rillig Exp $ NOMAN= # defined @@ -91,7 +91,7 @@ FILES+= d_type_question_colon.c FILES+= d_typefun.c FILES+= d_typename_as_var.c FILES+= d_zero_sized_arrays.c -FILES+= ${:U0 ${:U:range=337}:C,^.$,0&,:C,^..$,0&,:@msg@msg_${msg}.c msg_${msg}.exp@} +FILES+= ${:U0 ${:U:range=339}:C,^.$,0&,:C,^..$,0&,:@msg@msg_${msg}.c msg_${msg}.exp@} # Note: only works for adding tests. # To remove a test, the $$mi file must be edited manually. diff --git a/tests/usr.bin/xlint/lint1/msg_338.c b/tests/usr.bin/xlint/lint1/msg_338.c new file mode 100644 index 000000000000..14cff179fdfe --- /dev/null +++ b/tests/usr.bin/xlint/lint1/msg_338.c @@ -0,0 +1,45 @@ +/* $NetBSD: msg_338.c,v 1.1 2021/02/19 12:28:56 rillig Exp $ */ +# 3 "msg_338.c" + +// Test for message: option '%c' should be handled in the switch [338] + +int getopt(int, char *const *, const char *); +extern char *optarg; + +int +main(int argc, char **argv) +{ + int o; + + while ((o = getopt(argc, argv, "a:bc:d")) != -1) { /* expect: 338, 338 */ + switch (o) { + case 'a': + break; + case 'b': + /* + * The following while loop must not finish the check + * for the getopt options. + */ + while (optarg[0] != '\0') + optarg++; + break; + case 'e': /* expect: option 'e' should be listed */ + break; + case 'f': /* expect: option 'f' should be listed */ + /* + * The case labels in nested switch statements are + * ignored by the check for getopt options. + */ + switch (optarg[0]) { + case 'X': + break; + } + break; + case '?': + default: + break; + } + } + + return 0; +} diff --git a/tests/usr.bin/xlint/lint1/msg_338.exp b/tests/usr.bin/xlint/lint1/msg_338.exp new file mode 100644 index 000000000000..5889bb9ad60d --- /dev/null +++ b/tests/usr.bin/xlint/lint1/msg_338.exp @@ -0,0 +1,4 @@ +msg_338.c(26): warning: option 'e' should be listed in the options string [339] +msg_338.c(28): warning: option 'f' should be listed in the options string [339] +msg_338.c(14): warning: option 'c' should be handled in the switch [338] +msg_338.c(14): warning: option 'd' should be handled in the switch [338] diff --git a/tests/usr.bin/xlint/lint1/msg_339.c b/tests/usr.bin/xlint/lint1/msg_339.c new file mode 100644 index 000000000000..c9ad87def195 --- /dev/null +++ b/tests/usr.bin/xlint/lint1/msg_339.c @@ -0,0 +1,45 @@ +/* $NetBSD: msg_339.c,v 1.1 2021/02/19 12:28:56 rillig Exp $ */ +# 3 "msg_339.c" + +// Test for message: option '%c' should be listed in the options string [339] + +int getopt(int, char *const *, const char *); +extern char *optarg; + +int +main(int argc, char **argv) +{ + int o; + + while ((o = getopt(argc, argv, "a:bc:d")) != -1) { /* expect: 338, 338 */ + switch (o) { + case 'a': + break; + case 'b': + /* + * The following while loop must not finish the check + * for the getopt options. + */ + while (optarg[0] != '\0') + optarg++; + break; + case 'e': /* expect: option 'e' should be listed */ + break; + case 'f': /* expect: option 'f' should be listed */ + /* + * The case labels in nested switch statements are + * ignored by the check for getopt options. + */ + switch (optarg[0]) { + case 'X': + break; + } + break; + case '?': + default: + break; + } + } + + return 0; +} diff --git a/tests/usr.bin/xlint/lint1/msg_339.exp b/tests/usr.bin/xlint/lint1/msg_339.exp new file mode 100644 index 000000000000..4f8e54c4cc03 --- /dev/null +++ b/tests/usr.bin/xlint/lint1/msg_339.exp @@ -0,0 +1,4 @@ +msg_339.c(26): warning: option 'e' should be listed in the options string [339] +msg_339.c(28): warning: option 'f' should be listed in the options string [339] +msg_339.c(14): warning: option 'c' should be handled in the switch [338] +msg_339.c(14): warning: option 'd' should be handled in the switch [338] diff --git a/tests/usr.bin/xlint/lint1/t_integration.sh b/tests/usr.bin/xlint/lint1/t_integration.sh index 30d57642d9f5..3fb074754aa9 100644 --- a/tests/usr.bin/xlint/lint1/t_integration.sh +++ b/tests/usr.bin/xlint/lint1/t_integration.sh @@ -1,4 +1,4 @@ -# $NetBSD: t_integration.sh,v 1.28 2021/01/18 20:02:34 rillig Exp $ +# $NetBSD: t_integration.sh,v 1.29 2021/02/19 12:28:56 rillig Exp $ # # Copyright (c) 2008, 2010 The NetBSD Foundation, Inc. # All rights reserved. @@ -184,7 +184,7 @@ all_messages_body() srcdir="$(atf_get_srcdir)" ok="true" - for msg in $(seq 0 337); do + for msg in $(seq 0 339); do base="$(printf '%s/msg_%03d' "${srcdir}" "${msg}")" flags="$(extract_flags "${base}.c")" diff --git a/usr.bin/xlint/lint1/Makefile b/usr.bin/xlint/lint1/Makefile index 58ae8c06fab2..865cc432e7ea 100644 --- a/usr.bin/xlint/lint1/Makefile +++ b/usr.bin/xlint/lint1/Makefile @@ -1,10 +1,10 @@ -# $NetBSD: Makefile,v 1.59 2021/01/23 17:58:03 rillig Exp $ +# $NetBSD: Makefile,v 1.60 2021/02/19 12:28:56 rillig Exp $ .include PROG= lint1 -SRCS= cgram.y decl.c emit.c emit1.c err.c func.c init.c inittyp.c \ - lex.c \ +SRCS= cgram.y ckgetopt.c decl.c emit.c emit1.c err.c \ + func.c init.c inittyp.c lex.c \ main1.c mem.c mem1.c oper.c print.c scan.l tree.c tyname.c MAN= lint.7 diff --git a/usr.bin/xlint/lint1/ckgetopt.c b/usr.bin/xlint/lint1/ckgetopt.c new file mode 100644 index 000000000000..8fb927fba248 --- /dev/null +++ b/usr.bin/xlint/lint1/ckgetopt.c @@ -0,0 +1,178 @@ +/* $NetBSD: ckgetopt.c,v 1.1 2021/02/19 12:28:56 rillig Exp $ */ + +/*- + * Copyright (c) 2021 The NetBSD Foundation, Inc. + * All rights reserved. + * + * This code is derived from software contributed to The NetBSD Foundation + * by Roland Illig . + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#if defined(__RCSID) && !defined(lint) +__RCSID("$NetBSD: ckgetopt.c,v 1.1 2021/02/19 12:28:56 rillig Exp $"); +#endif + +#include +#include +#include + +#include "lint1.h" + +/* + * In a typical while loop for parsing getopt options, ensure that each + * option from the options string is handled, and that each handled option + * is listed in the options string. + */ + +struct { + pos_t options_pos; + char *options; + char *unhandled_options; + int while_level; + int switch_level; +} ck; + + +static bool +is_getopt_call(const tnode_t *tn, char **out_options) +{ + if (tn == NULL) + return false; + if (tn->tn_op != NE) + return false; + if (tn->tn_left->tn_op != ASSIGN) + return false; + + const tnode_t *call = tn->tn_left->tn_right; + if (call->tn_op != CALL) + return false; + if (call->tn_left->tn_op != ADDR) + return false; + if (call->tn_left->tn_left->tn_op != NAME) + return false; + if (strcmp(call->tn_left->tn_left->tn_sym->s_name, "getopt") != 0) + return false; + + if (call->tn_right->tn_op != PUSH) + return false; + + const tnode_t *last_arg = call->tn_right->tn_left; + if (last_arg->tn_op != CVT) + return false; + if (last_arg->tn_left->tn_op != ADDR) + return false; + if (last_arg->tn_left->tn_left->tn_op != STRING) + return false; + if (last_arg->tn_left->tn_left->tn_string->st_tspec != CHAR) + return false; + + *out_options = xstrdup( + (const char *)last_arg->tn_left->tn_left->tn_string->st_cp); + return true; +} + +static void +check_unlisted_option(char opt) +{ + if (opt == '?') + return; + + const char *optptr = strchr(ck.options, opt); + if (optptr != NULL) + ck.unhandled_options[optptr - ck.options] = ' '; + else { + /* option '%c' should be listed in the options string */ + warning(339, opt); + return; + } +} + +static void +check_unhandled_option(void) +{ + for (const char *opt = ck.unhandled_options; *opt != '\0'; opt++) { + if (*opt == ' ' || *opt == ':') + continue; + + pos_t prev_pos = curr_pos; + curr_pos = ck.options_pos; + /* option '%c' should be handled in the switch */ + warning(338, *opt); + curr_pos = prev_pos; + } +} + + +void +check_getopt_begin_while(const tnode_t *tn) +{ + if (ck.while_level == 0 && is_getopt_call(tn, &ck.options)) { + ck.unhandled_options = xstrdup(ck.options); + ck.options_pos = curr_pos; + } + ck.while_level++; +} + +void +check_getopt_begin_switch(void) +{ + if (ck.while_level > 0) + ck.switch_level++; +} + + +void +check_getopt_case_label(int64_t value) +{ + if (ck.switch_level == 1 && value == (char)value) + check_unlisted_option((char)value); +} + +void +check_getopt_end_switch(void) +{ + if (ck.switch_level == 0) + return; + + ck.switch_level--; + if (ck.switch_level == 0) + check_unhandled_option(); +} + +void +check_getopt_end_while(void) +{ + if (ck.while_level == 0) + return; + + ck.while_level--; + if (ck.while_level != 0) + return; + + free(ck.options); + free(ck.unhandled_options); + ck.options = NULL; + ck.unhandled_options = NULL; +} diff --git a/usr.bin/xlint/lint1/err.c b/usr.bin/xlint/lint1/err.c index a90ae78afe44..42a720353f60 100644 --- a/usr.bin/xlint/lint1/err.c +++ b/usr.bin/xlint/lint1/err.c @@ -1,4 +1,4 @@ -/* $NetBSD: err.c,v 1.78 2021/02/04 06:54:59 rillig Exp $ */ +/* $NetBSD: err.c,v 1.79 2021/02/19 12:28:56 rillig Exp $ */ /* * Copyright (c) 1994, 1995 Jochen Pohl @@ -37,7 +37,7 @@ #include #if defined(__RCSID) && !defined(lint) -__RCSID("$NetBSD: err.c,v 1.78 2021/02/04 06:54:59 rillig Exp $"); +__RCSID("$NetBSD: err.c,v 1.79 2021/02/19 12:28:56 rillig Exp $"); #endif #include @@ -397,6 +397,8 @@ const char *msgs[] = { "operand of '%s' must not be bool", /* 335 */ "left operand of '%s' must not be bool", /* 336 */ "right operand of '%s' must not be bool", /* 337 */ + "option '%c' should be handled in the switch", /* 338 */ + "option '%c' should be listed in the options string", /* 339 */ }; /* diff --git a/usr.bin/xlint/lint1/externs1.h b/usr.bin/xlint/lint1/externs1.h index 5eeba4d49852..5bcbca1e6a8d 100644 --- a/usr.bin/xlint/lint1/externs1.h +++ b/usr.bin/xlint/lint1/externs1.h @@ -1,4 +1,4 @@ -/* $NetBSD: externs1.h,v 1.65 2021/01/31 12:44:34 rillig Exp $ */ +/* $NetBSD: externs1.h,v 1.66 2021/02/19 12:28:56 rillig Exp $ */ /* * Copyright (c) 1994, 1995 Jochen Pohl @@ -328,3 +328,12 @@ extern int lex_input(void); * print.c */ extern char *print_tnode(char *, size_t, const tnode_t *); + +/* + * ckgetopt.c + */ +extern void check_getopt_begin_while(const tnode_t *); +extern void check_getopt_begin_switch(void); +extern void check_getopt_case_label(int64_t); +extern void check_getopt_end_switch(void); +extern void check_getopt_end_while(void); diff --git a/usr.bin/xlint/lint1/func.c b/usr.bin/xlint/lint1/func.c index 86edeace5726..d409ac8a052a 100644 --- a/usr.bin/xlint/lint1/func.c +++ b/usr.bin/xlint/lint1/func.c @@ -1,4 +1,4 @@ -/* $NetBSD: func.c,v 1.67 2021/01/31 12:44:34 rillig Exp $ */ +/* $NetBSD: func.c,v 1.68 2021/02/19 12:28:56 rillig Exp $ */ /* * Copyright (c) 1994, 1995 Jochen Pohl @@ -37,7 +37,7 @@ #include #if defined(__RCSID) && !defined(lint) -__RCSID("$NetBSD: func.c,v 1.67 2021/01/31 12:44:34 rillig Exp $"); +__RCSID("$NetBSD: func.c,v 1.68 2021/02/19 12:28:56 rillig Exp $"); #endif #include @@ -478,6 +478,8 @@ check_case_label(tnode_t *tn, cstk_t *ci) /* duplicate case in switch: %ld */ error(199, (long)nv.v_quad); } else { + check_getopt_case_label(nv.v_quad); + /* * append the value to the list of * case values @@ -642,6 +644,7 @@ switch1(tnode_t *tn) tp->t_tspec = INT; } + check_getopt_begin_switch(); expr(tn, true, false, true, false); pushctrl(T_SWITCH); @@ -684,9 +687,11 @@ switch2(void) } } + check_getopt_end_switch(); + if (cstmt->c_break) { /* - * end of switch alway reached (c_break is only set if the + * end of switch always reached (c_break is only set if the * break statement can be reached). */ reached = true; @@ -726,6 +731,7 @@ while1(tnode_t *tn) if (tn != NULL && tn->tn_op == CON) cstmt->c_infinite = is_nonzero(tn); + check_getopt_begin_while(tn); expr(tn, false, true, true, false); } @@ -744,6 +750,7 @@ while2(void) reached = !cstmt->c_infinite || cstmt->c_break; rchflg = false; + check_getopt_end_while(); popctrl(T_WHILE); }