Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/usr.bin/xlint/lint1 lint: fix check for function calls in st...
details: https://anonhg.NetBSD.org/src/rev/4616fe132cbf
branches: trunk
changeset: 1026294:4616fe132cbf
user: rillig <rillig%NetBSD.org@localhost>
date: Tue Nov 16 21:01:05 2021 +0000
description:
lint: fix check for function calls in strict bool mode
Previously, if a function call occurred in the controlling expression,
its return type could be any scalar, not just bool. This was against
the goal of strict bool mode, which makes bool a separate and
incompabile type to all other types. For example, it would allow
controlling expressions like 'strcmp(a, b)' without the usual '!= 0',
but only if at least one of 'a' and 'b' came from a macro definition
from a system header.
The fix is that the decision of whether the type of the controlling
expression may be scalar is no longer based on the operand types but on
the main operator of the controlling expression.
diffstat:
tests/usr.bin/xlint/lint1/d_c99_bool_strict.c | 79 ++++++++-----
tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp | 4 +
usr.bin/xlint/lint1/cgram.y | 135 ++++++++++++-----------
usr.bin/xlint/lint1/ckbool.c | 13 +-
usr.bin/xlint/lint1/debug.c | 6 +-
usr.bin/xlint/lint1/externs1.h | 12 +-
usr.bin/xlint/lint1/func.c | 8 +-
usr.bin/xlint/lint1/init.c | 6 +-
usr.bin/xlint/lint1/lint1.h | 9 +-
usr.bin/xlint/lint1/mem1.c | 10 +-
usr.bin/xlint/lint1/tree.c | 133 +++++++++++------------
11 files changed, 220 insertions(+), 195 deletions(-)
diffs (truncated from 1048 to 300 lines):
diff -r 98acb16dcb16 -r 4616fe132cbf tests/usr.bin/xlint/lint1/d_c99_bool_strict.c
--- a/tests/usr.bin/xlint/lint1/d_c99_bool_strict.c Tue Nov 16 21:00:50 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/d_c99_bool_strict.c Tue Nov 16 21:01:05 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: d_c99_bool_strict.c,v 1.33 2021/11/16 18:27:04 rillig Exp $ */
+/* $NetBSD: d_c99_bool_strict.c,v 1.34 2021/11/16 21:01:06 rillig Exp $ */
# 3 "d_c99_bool_strict.c"
/*
@@ -783,8 +783,8 @@
* though they are not strictly boolean.
*
* This shouldn't apply to function call expressions though since one of the
- * goals of strict bool mode is to normalize all expressions like 'strcmp' to
- * be 'strcmp(a, b) == 0' instead of '!strcmp(a, b)'.
+ * goals of strict bool mode is to normalize all expressions calling 'strcmp'
+ * to be of the form 'strcmp(a, b) == 0' instead of '!strcmp(a, b)'.
*/
# 1 "stdio.h" 1 3 4
typedef struct stdio_file {
@@ -815,68 +815,83 @@
return;
/*
- * No warning below since the expression 'stdio_stdin' comes from a
- * system header (typically via a macro), and this property is passed
- * up to the expression 'ferror(stdio_stdin)'.
+ * Before tree.c 1.395 from 2021-11-16, the expression below didn't
+ * produce a warning since the expression 'stdio_stdin' didn't come
+ * from a system header (typically via a macro), and this property
+ * was passed up to the expression 'ferror(stdio_stdin)'.
*
- * That is wrong though since the above rule would allow a plain
- * 'strcmp' without a following '== 0', as long as one of its
- * arguments comes from a system header.
+ * That was wrong though since the type of a function call expression
+ * only depends on the function itself but not its arguments types.
+ * The old rule had allowed a raw condition 'strcmp(a, b)' without
+ * the comparison '!= 0', as long as one of its arguments came from a
+ * system header.
*
* Seen in bin/echo/echo.c, function main, call to ferror.
*/
- /*
- * TODO: In a function call expression, tn->tn_relaxed should only be
- * derived from the function itself, not from its arguments.
- */
- /* TODO: Warn about type mismatch [333]. */
+ /* expect+5: error: controlling expression must be bool, not 'int' [333] */
if (ferror(
-# 835 "d_c99_bool_strict.c" 3 4
+# 834 "d_c99_bool_strict.c" 3 4
&stdio_files[1]
-# 837 "d_c99_bool_strict.c"
+# 836 "d_c99_bool_strict.c"
))
return;
/*
* Before cgram.y 1.369 from 2021-11-16, at the end of parsing the
* name 'stdio_stdout', the parser already looked ahead to the next
- * token, to see whether it was the '(' of a function call. At that
- * point, the parser was no longer in a system header, therefore
- * 'stdio_stdout' was not tn_relaxed, and this information was pushed
- * down to the whole function call expression (which was another bug
- * at that time).
+ * token, to see whether it was the '(' of a function call.
+ *
+ * At that point, the parser was no longer in a system header,
+ * therefore 'stdio_stdout' had tn_sys == false, and this information
+ * was pushed down to the whole function call expression (which was
+ * another bug that got fixed in tree.c 1.395 from 2021-11-16).
*/
+ /* expect+5: error: controlling expression must be bool, not 'int' [333] */
if (ferror(
-# 851 "d_c99_bool_strict.c" 3 4
+# 852 "d_c99_bool_strict.c" 3 4
stdio_stdout
-# 853 "d_c99_bool_strict.c"
+# 854 "d_c99_bool_strict.c"
))
return;
/*
- * In this variant, there is a token ')' after the name
- * 'stdio_stdout', which has the effect that at the end of parsing
- * the name, the parser is still in the system header, thus setting
- * tn_relaxed to true.
+ * In this variant of the pattern, there is a token ')' after the
+ * name 'stdio_stdout', which even before tree.c 1.395 from
+ * 2021-11-16 had the effect that at the end of parsing the name, the
+ * parser was still in the system header, thus setting tn_sys (or
+ * rather tn_relaxed at that time) to true.
*/
+ /* expect+5: error: controlling expression must be bool, not 'int' [333] */
if (ferror(
-# 864 "d_c99_bool_strict.c" 3 4
+# 867 "d_c99_bool_strict.c" 3 4
(stdio_stdout)
-# 866 "d_c99_bool_strict.c"
+# 869 "d_c99_bool_strict.c"
))
return;
/*
* Before cgram.y 1.369 from 2021-11-16, the comment following
* 'stdio_stdout' did not prevent the search for '('. At the point
- * where build_name calls expr_zalloc_tnode, the parser was already
+ * where build_name called expr_zalloc_tnode, the parser was already
* in the main file again, thus treating 'stdio_stdout' as not coming
* from a system header.
+ *
+ * This has been fixed in tree.c 1.395 from 2021-11-16. Before that,
+ * an expression had come from a system header if its operands came
+ * from a system header, but that was only close to the truth. In a
+ * case where both operands come from a system header but the
+ * operator comes from the main translation unit, the main
+ * translation unit still has control over the whole expression. So
+ * the correct approach is to focus on the operator, not the
+ * operands. There are a few corner cases where the operator is
+ * invisible (for implicit conversions) or synthetic (for translating
+ * 'arr[index]' to '*(arr + index)', but these are handled as well.
*/
+ /* expect+5: error: controlling expression must be bool, not 'int' [333] */
if (ferror(
-# 878 "d_c99_bool_strict.c" 3 4
+# 893 "d_c99_bool_strict.c" 3 4
stdio_stdout /* comment */
-# 880 "d_c99_bool_strict.c"
+# 895 "d_c99_bool_strict.c"
))
return;
}
diff -r 98acb16dcb16 -r 4616fe132cbf tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp
--- a/tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp Tue Nov 16 21:00:50 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp Tue Nov 16 21:01:05 2021 +0000
@@ -171,3 +171,7 @@
d_c99_bool_strict.c(808): error: controlling expression must be bool, not 'int' [333]
d_c99_bool_strict.c(811): error: operand of '!' must be bool, not 'int' [330]
d_c99_bool_strict.c(814): error: operand of '!' must be bool, not 'int' [330]
+d_c99_bool_strict.c(836): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(854): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(869): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(895): error: controlling expression must be bool, not 'int' [333]
diff -r 98acb16dcb16 -r 4616fe132cbf usr.bin/xlint/lint1/cgram.y
--- a/usr.bin/xlint/lint1/cgram.y Tue Nov 16 21:00:50 2021 +0000
+++ b/usr.bin/xlint/lint1/cgram.y Tue Nov 16 21:01:05 2021 +0000
@@ -1,5 +1,5 @@
%{
-/* $NetBSD: cgram.y,v 1.369 2021/11/16 18:27:04 rillig Exp $ */
+/* $NetBSD: cgram.y,v 1.370 2021/11/16 21:01:05 rillig Exp $ */
/*
* Copyright (c) 1996 Christopher G. Demetriou. All Rights Reserved.
@@ -35,7 +35,7 @@
#include <sys/cdefs.h>
#if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: cgram.y,v 1.369 2021/11/16 18:27:04 rillig Exp $");
+__RCSID("$NetBSD: cgram.y,v 1.370 2021/11/16 21:01:05 rillig Exp $");
#endif
#include <limits.h>
@@ -144,6 +144,7 @@
bool y_seen_statement;
struct generic_association *y_generic;
struct array_size y_array_size;
+ bool y_in_system_header;
};
%token T_LBRACE T_RBRACE T_LBRACK T_RBRACK T_LPAREN T_RPAREN
@@ -355,6 +356,7 @@
%type <y_seen_statement> block_item
%type <y_tnode> do_while_expr
%type <y_sym> func_declarator
+%type <y_in_system_header> sys
%{
#if defined(YYDEBUG) && defined(YYBISON)
@@ -484,20 +486,20 @@
/* K&R 7.1, C90 ???, C99 6.5.2, C11 6.5.2 */
postfix_expression:
primary_expression
- | postfix_expression T_LBRACK expression T_RBRACK {
- $$ = build_unary(INDIR, build_binary($1, PLUS, $3));
+ | postfix_expression T_LBRACK sys expression T_RBRACK {
+ $$ = build_unary(INDIR, $3, build_binary($1, PLUS, $3, $4));
}
- | postfix_expression T_LPAREN T_RPAREN {
- $$ = build_function_call($1, NULL);
+ | postfix_expression T_LPAREN sys T_RPAREN {
+ $$ = build_function_call($1, $3, NULL);
}
- | postfix_expression T_LPAREN argument_expression_list T_RPAREN {
- $$ = build_function_call($1, $3);
+ | postfix_expression T_LPAREN sys argument_expression_list T_RPAREN {
+ $$ = build_function_call($1, $3, $4);
}
- | postfix_expression point_or_arrow T_NAME {
- $$ = build_member_access($1, $2, $3);
+ | postfix_expression point_or_arrow sys T_NAME {
+ $$ = build_member_access($1, $2, $3, $4);
}
- | postfix_expression T_INCDEC {
- $$ = build_unary($2 == INC ? INCAFT : DECAFT, $1);
+ | postfix_expression T_INCDEC sys {
+ $$ = build_unary($2 == INC ? INCAFT : DECAFT, $3, $1);
}
| T_LPAREN type_name T_RPAREN { /* C99 6.5.2.5 "Compound literals" */
sym_t *tmp = mktempsym($2);
@@ -589,33 +591,33 @@
/* K&R 7.2, C90 ???, C99 6.5.3, C11 6.5.3 */
unary_expression:
postfix_expression
- | T_INCDEC unary_expression {
- $$ = build_unary($1 == INC ? INCBEF : DECBEF, $2);
+ | T_INCDEC sys unary_expression {
+ $$ = build_unary($1 == INC ? INCBEF : DECBEF, $2, $3);
}
- | T_AMPER cast_expression {
- $$ = build_unary(ADDR, $2);
+ | T_AMPER sys cast_expression {
+ $$ = build_unary(ADDR, $2, $3);
}
- | T_ASTERISK cast_expression {
- $$ = build_unary(INDIR, $2);
+ | T_ASTERISK sys cast_expression {
+ $$ = build_unary(INDIR, $2, $3);
}
- | T_ADDITIVE cast_expression {
+ | T_ADDITIVE sys cast_expression {
if (tflag && $1 == PLUS) {
/* unary + is illegal in traditional C */
warning(100);
}
- $$ = build_unary($1 == PLUS ? UPLUS : UMINUS, $2);
+ $$ = build_unary($1 == PLUS ? UPLUS : UMINUS, $2, $3);
}
- | T_COMPLEMENT cast_expression {
- $$ = build_unary(COMPL, $2);
+ | T_COMPLEMENT sys cast_expression {
+ $$ = build_unary(COMPL, $2, $3);
}
- | T_LOGNOT cast_expression {
- $$ = build_unary(NOT, $2);
+ | T_LOGNOT sys cast_expression {
+ $$ = build_unary(NOT, $2, $3);
}
- | T_REAL cast_expression { /* GCC c_parser_unary_expression */
- $$ = build_unary(REAL, $2);
+ | T_REAL sys cast_expression { /* GCC c_parser_unary_expression */
+ $$ = build_unary(REAL, $2, $3);
}
- | T_IMAG cast_expression { /* GCC c_parser_unary_expression */
- $$ = build_unary(IMAG, $2);
+ | T_IMAG sys cast_expression { /* GCC c_parser_unary_expression */
+ $$ = build_unary(IMAG, $2, $3);
}
| T_EXTENSION cast_expression { /* GCC c_parser_unary_expression */
$$ = $2;
@@ -666,61 +668,62 @@
/* K&R ???, C90 ???, C99 6.5.5 to 6.5.15, C11 6.5.5 to 6.5.15 */
conditional_expression:
cast_expression
- | conditional_expression T_ASTERISK conditional_expression {
- $$ = build_binary($1, MULT, $3);
+ | conditional_expression T_ASTERISK sys conditional_expression {
+ $$ = build_binary($1, MULT, $3, $4);
}
- | conditional_expression T_MULTIPLICATIVE conditional_expression {
- $$ = build_binary($1, $2, $3);
+ | conditional_expression T_MULTIPLICATIVE sys conditional_expression {
+ $$ = build_binary($1, $2, $3, $4);
}
- | conditional_expression T_ADDITIVE conditional_expression {
- $$ = build_binary($1, $2, $3);
+ | conditional_expression T_ADDITIVE sys conditional_expression {
+ $$ = build_binary($1, $2, $3, $4);
}
- | conditional_expression T_SHIFT conditional_expression {
- $$ = build_binary($1, $2, $3);
+ | conditional_expression T_SHIFT sys conditional_expression {
+ $$ = build_binary($1, $2, $3, $4);
}
- | conditional_expression T_RELATIONAL conditional_expression {
- $$ = build_binary($1, $2, $3);
+ | conditional_expression T_RELATIONAL sys conditional_expression {
+ $$ = build_binary($1, $2, $3, $4);
}
- | conditional_expression T_EQUALITY conditional_expression {
- $$ = build_binary($1, $2, $3);
+ | conditional_expression T_EQUALITY sys conditional_expression {
+ $$ = build_binary($1, $2, $3, $4);
}
- | conditional_expression T_AMPER conditional_expression {
- $$ = build_binary($1, BITAND, $3);
+ | conditional_expression T_AMPER sys conditional_expression {
+ $$ = build_binary($1, BITAND, $3, $4);
}
- | conditional_expression T_BITXOR conditional_expression {
Home |
Main Index |
Thread Index |
Old Index