Module Name: src
Committed By: rillig
Date: Tue Mar 23 18:40:50 UTC 2021
Modified Files:
src/tests/usr.bin/xlint/lint1: msg_171.c msg_171.exp
src/usr.bin/xlint/lint1: cgram.y decl.c externs1.h init.c
Log Message:
lint: fix wrong error message about type mismatch in compound literal
Now that the code contains explicit markers for starting and ending an
initialization, and having the guarantee that an assertion fails
whenever some code accesses the state of the "current initialization"
even though there is no ongoing initialization gives me much more
confidence in the correctness of the code. The calls to
begin_initialization and end_initialization always appear in pairs,
enclosing the minimal amount of code necessary for initialization.
In a nutshell, global modifiable state is error-prone and hard to
understand.
A nice side effect is that the grammar no longer needs a special rule
for the outermost initializer since the functions for the debug logging
are now called explicitly.
The code that misuses the initialization state just because it needs to
temporarily store a sym_t somewhere is now clearly marked as such. A
GCC statement expression can appear anywhere and is therefore
independent of the initialization. Most probably the code can simply
refer to the local variable in the grammar rule itself, or this variable
needs to be encoded in the grammar %union. For sure there is a better
way to handle this.
There is no longer a need that the function 'declare' initializes the
initialization state, it was just the wrong place to do this.
To generate a diff of this commit:
cvs rdiff -u -r1.5 -r1.6 src/tests/usr.bin/xlint/lint1/msg_171.c \
src/tests/usr.bin/xlint/lint1/msg_171.exp
cvs rdiff -u -r1.197 -r1.198 src/usr.bin/xlint/lint1/cgram.y
cvs rdiff -u -r1.158 -r1.159 src/usr.bin/xlint/lint1/decl.c
cvs rdiff -u -r1.83 -r1.84 src/usr.bin/xlint/lint1/externs1.h
cvs rdiff -u -r1.110 -r1.111 src/usr.bin/xlint/lint1/init.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/tests/usr.bin/xlint/lint1/msg_171.c
diff -u src/tests/usr.bin/xlint/lint1/msg_171.c:1.5 src/tests/usr.bin/xlint/lint1/msg_171.c:1.6
--- src/tests/usr.bin/xlint/lint1/msg_171.c:1.5 Mon Mar 22 16:51:24 2021
+++ src/tests/usr.bin/xlint/lint1/msg_171.c Tue Mar 23 18:40:50 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: msg_171.c,v 1.5 2021/03/22 16:51:24 rillig Exp $ */
+/* $NetBSD: msg_171.c,v 1.6 2021/03/23 18:40:50 rillig Exp $ */
# 3 "msg_171.c"
// Test for message: cannot assign to '%s' from '%s' [171]
@@ -23,6 +23,11 @@ example(int i, void *vp, struct s *s)
* with automatic storage duration, like any normal named object. It is an
* lvalue, which means that it is possible to take the address of the object.
* Seen in external/mpl/bind/dist/lib/dns/rbtdb.c, update_rrsetstats.
+ *
+ * Before init.c 1.111 from 2021-03-23, lint could not handle these nested
+ * initializations (the outer one for the variable 'p', the inner one for the
+ * compound literal) and wrongly complained about a type mismatch between
+ * 'struct point' and 'pointer to struct point'.
*/
void
pointer_to_compound_literal(void)
@@ -33,21 +38,5 @@ pointer_to_compound_literal(void)
};
struct point *p = &(struct point){
12, 5,
- }; /* expect: 171 *//*FIXME*/
- /*
- * FIXME: The type mismatch in the above line occurs because lint
- * wrongly assumes that there is only ever a single initialization
- * going on, which takes place in initstk.
- *
- * In the debug log, this is marked by the two calls to initstack_init
- * that are happily intermixed with 'begin initialization' and 'end
- * initialization'. This was not planned for, and it worked well
- * before C99, since compound literals are a new feature from C99.
- *
- * The proper fix, as for so many similar problems is to not use
- * global variables for things that have a limited lifetime, but
- * instead let the grammar determine the lifetime and scope of these
- * objects, which makes them only accessible when they can actually be
- * used.
- */
+ };
}
Index: src/tests/usr.bin/xlint/lint1/msg_171.exp
diff -u src/tests/usr.bin/xlint/lint1/msg_171.exp:1.5 src/tests/usr.bin/xlint/lint1/msg_171.exp:1.6
--- src/tests/usr.bin/xlint/lint1/msg_171.exp:1.5 Mon Mar 22 15:29:43 2021
+++ src/tests/usr.bin/xlint/lint1/msg_171.exp Tue Mar 23 18:40:50 2021
@@ -2,4 +2,3 @@ msg_171.c(14): error: cannot assign to '
msg_171.c(15): error: cannot assign to 'struct s' from 'int' [171]
msg_171.c(17): error: cannot assign to 'pointer to void' from 'struct s' [171]
msg_171.c(18): error: cannot assign to 'struct s' from 'pointer to void' [171]
-msg_171.c(36): error: cannot assign to 'struct point' from 'pointer to struct point' [171]
Index: src/usr.bin/xlint/lint1/cgram.y
diff -u src/usr.bin/xlint/lint1/cgram.y:1.197 src/usr.bin/xlint/lint1/cgram.y:1.198
--- src/usr.bin/xlint/lint1/cgram.y:1.197 Tue Mar 23 17:36:55 2021
+++ src/usr.bin/xlint/lint1/cgram.y Tue Mar 23 18:40:50 2021
@@ -1,5 +1,5 @@
%{
-/* $NetBSD: cgram.y,v 1.197 2021/03/23 17:36:55 rillig Exp $ */
+/* $NetBSD: cgram.y,v 1.198 2021/03/23 18:40:50 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.197 2021/03/23 17:36:55 rillig Exp $");
+__RCSID("$NetBSD: cgram.y,v 1.198 2021/03/23 18:40:50 rillig Exp $");
#endif
#include <limits.h>
@@ -1015,9 +1015,11 @@ notype_init_decl:
check_size($1);
}
| notype_decl opt_asm_or_symbolrename {
+ begin_initialization($1);
cgram_declare($1, true, $2);
- } T_ASSIGN outermost_initializer {
+ } T_ASSIGN initializer {
check_size($1);
+ end_initialization();
}
;
@@ -1027,9 +1029,11 @@ type_init_decl:
check_size($1);
}
| type_decl opt_asm_or_symbolrename {
+ begin_initialization($1);
cgram_declare($1, true, $2);
- } T_ASSIGN outermost_initializer {
+ } T_ASSIGN initializer {
check_size($1);
+ end_initialization();
}
;
@@ -1331,14 +1335,6 @@ opt_asm_or_symbolrename: /* expect only
}
;
-outermost_initializer:
- {
- cgram_debug("begin initialization");
- } initializer {
- cgram_debug("end initialization");
- }
- ;
-
initializer: /* C99 6.7.8 "Initialization" */
expr %prec T_COMMA {
init_using_expr($1);
@@ -1885,24 +1881,28 @@ term:
expr_statement_list {
block_level--;
mem_block_level--;
- *current_initsym() = mktempsym(duptyp($4->tn_type));
+ /* XXX: probably does not need the full initialization code */
+ begin_initialization(mktempsym(duptyp($4->tn_type)));
mem_block_level++;
block_level++;
/* ({ }) is a GCC extension */
gnuism(320);
} compound_statement_rbrace T_RPAREN {
$$ = new_name_node(*current_initsym(), 0);
+ end_initialization();
}
| T_LPAREN compound_statement_lbrace expr_statement_list {
block_level--;
mem_block_level--;
- *current_initsym() = mktempsym($3->tn_type);
+ /* XXX: probably does not need the full initialization code */
+ begin_initialization(mktempsym($3->tn_type));
mem_block_level++;
block_level++;
/* ({ }) is a GCC extension */
gnuism(320);
} compound_statement_rbrace T_RPAREN {
$$ = new_name_node(*current_initsym(), 0);
+ end_initialization();
}
| term T_INCDEC {
$$ = build($2 == INC ? INCAFT : DECAFT, $1, NULL);
@@ -1990,12 +1990,14 @@ term:
}
| T_LPAREN type_name T_RPAREN %prec T_UNARY {
sym_t *tmp = mktempsym($2);
+ begin_initialization(tmp);
cgram_declare(tmp, true, NULL);
} init_lbrace initializer_list comma_opt init_rbrace {
if (!Sflag)
/* compound literals are a C9X/GCC extension */
gnuism(319);
$$ = new_name_node(*current_initsym(), 0);
+ end_initialization();
}
;
Index: src/usr.bin/xlint/lint1/decl.c
diff -u src/usr.bin/xlint/lint1/decl.c:1.158 src/usr.bin/xlint/lint1/decl.c:1.159
--- src/usr.bin/xlint/lint1/decl.c:1.158 Tue Mar 23 17:36:56 2021
+++ src/usr.bin/xlint/lint1/decl.c Tue Mar 23 18:40:50 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: decl.c,v 1.158 2021/03/23 17:36:56 rillig Exp $ */
+/* $NetBSD: decl.c,v 1.159 2021/03/23 18:40:50 rillig Exp $ */
/*
* Copyright (c) 1996 Christopher G. Demetriou. All Rights Reserved.
@@ -38,7 +38,7 @@
#include <sys/cdefs.h>
#if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: decl.c,v 1.158 2021/03/23 17:36:56 rillig Exp $");
+__RCSID("$NetBSD: decl.c,v 1.159 2021/03/23 18:40:50 rillig Exp $");
#endif
#include <sys/param.h>
@@ -1896,9 +1896,6 @@ declare(sym_t *decl, bool initflg, sbuf_
{
char *s;
- *current_initerr() = false;
- *current_initsym() = decl;
-
switch (dcs->d_ctx) {
case EXTERN:
if (renaming != NULL) {
Index: src/usr.bin/xlint/lint1/externs1.h
diff -u src/usr.bin/xlint/lint1/externs1.h:1.83 src/usr.bin/xlint/lint1/externs1.h:1.84
--- src/usr.bin/xlint/lint1/externs1.h:1.83 Tue Mar 23 17:36:56 2021
+++ src/usr.bin/xlint/lint1/externs1.h Tue Mar 23 18:40:50 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: externs1.h,v 1.83 2021/03/23 17:36:56 rillig Exp $ */
+/* $NetBSD: externs1.h,v 1.84 2021/03/23 18:40:50 rillig Exp $ */
/*
* Copyright (c) 1994, 1995 Jochen Pohl
@@ -292,6 +292,8 @@ extern void bitfieldtype(int);
/*
* init.c
*/
+extern void begin_initialization(sym_t *);
+extern void end_initialization(void);
extern bool *current_initerr(void);
extern sym_t **current_initsym(void);
Index: src/usr.bin/xlint/lint1/init.c
diff -u src/usr.bin/xlint/lint1/init.c:1.110 src/usr.bin/xlint/lint1/init.c:1.111
--- src/usr.bin/xlint/lint1/init.c:1.110 Tue Mar 23 17:36:56 2021
+++ src/usr.bin/xlint/lint1/init.c Tue Mar 23 18:40:50 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: init.c,v 1.110 2021/03/23 17:36:56 rillig Exp $ */
+/* $NetBSD: init.c,v 1.111 2021/03/23 18:40:50 rillig Exp $ */
/*
* Copyright (c) 1994, 1995 Jochen Pohl
@@ -37,7 +37,7 @@
#include <sys/cdefs.h>
#if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: init.c,v 1.110 2021/03/23 17:36:56 rillig Exp $");
+__RCSID("$NetBSD: init.c,v 1.111 2021/03/23 18:40:50 rillig Exp $");
#endif
#include <stdlib.h>
@@ -201,9 +201,10 @@ typedef struct namlist {
struct initialization {
/*
- * initerr is set as soon as a fatal error occurred in an initialization.
- * The effect is that the rest of the initialization is ignored (parsed
- * by yacc, expression trees built, but no initialization takes place).
+ * is set as soon as a fatal error occurred in the initialization.
+ * The effect is that the rest of the initialization is ignored
+ * (parsed by yacc, expression trees built, but no initialization
+ * takes place).
*/
bool initerr;
@@ -219,7 +220,7 @@ struct initialization {
struct initialization *next;
};
-static struct initialization init;
+static struct initialization *init;
static bool init_array_using_string(tnode_t *);
@@ -227,25 +228,29 @@ static bool init_array_using_string(tnod
bool *
current_initerr(void)
{
- return &init.initerr;
+ lint_assert(init != NULL);
+ return &init->initerr;
}
sym_t **
current_initsym(void)
{
- return &init.initsym;
+ lint_assert(init != NULL);
+ return &init->initsym;
}
static namlist_t **
current_namedmem(void)
{
- return &init.namedmem;
+ lint_assert(init != NULL);
+ return &init->namedmem;
}
static initstack_element **
current_initstk(void)
{
- return &init.initstk;
+ lint_assert(init != NULL);
+ return &init->initstk;
}
#define initerr (*current_initerr())
@@ -371,6 +376,30 @@ debug_initstack(void)
#endif
+
+void
+begin_initialization(sym_t *sym)
+{
+ struct initialization *curr_init;
+
+ debug_step("begin initialization");
+ curr_init = xcalloc(1, sizeof *curr_init);
+ curr_init->next = init;
+ init = curr_init;
+ initsym = sym;
+}
+
+void
+end_initialization(void)
+{
+ struct initialization *curr_init;
+
+ curr_init = init;
+ init = init->next;
+ free(curr_init);
+ debug_step("end initialization");
+}
+
void
designator_push_name(sbuf_t *sb)
{
