From 73167563d5f1d05ddb690038420c4b23d1d1e1dd Mon Sep 17 00:00:00 2001 From: Andrew Andrianov Date: Sun, 8 Oct 2017 14:18:32 +0300 Subject: [PATCH 1/6] ivlpp: Warn about macro redefinition Verilog spec has a very nasty system of macros jumping from file to file, resulting in a global macro scope. We abosolutely MUST track macro redefinitions and warn user about them. Signed-off-by: Andrew Andrianov --- ivlpp/lexor.lex | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ivlpp/lexor.lex b/ivlpp/lexor.lex index c81af7397..a0aa71b42 100644 --- a/ivlpp/lexor.lex +++ b/ivlpp/lexor.lex @@ -858,6 +858,19 @@ void define_macro(const char* name, const char* value, int keyword, int argc) { int idx; struct define_t* def; + struct define_t* prev; + + /* Verilog spec has a very nasty system of macros jumping from + * file to file, resulting in a global macro scope. We abosolutely + * MUST track macro redefinitions and warn user about them. + */ + + prev = def_lookup(name); + if (prev) { + emit_pathline(istack); + fprintf(stderr, "warning: redefinition of macro %s\n", + name); + } def = malloc(sizeof(struct define_t)); def->name = strdup(name); From 0c84413347a9fc76f46e1c8d41e66033d7e1634e Mon Sep 17 00:00:00 2001 From: Andrew Andrianov Date: Wed, 18 Oct 2017 19:18:14 +0300 Subject: [PATCH 2/6] ivlpp: Add -Wredef option to enable redefinition warnings Signed-off-by: Andrew Andrianov --- ivlpp/globals.h | 2 ++ ivlpp/lexor.lex | 4 ++-- ivlpp/main.c | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/ivlpp/globals.h b/ivlpp/globals.h index 90b4632b9..1c4cee43d 100644 --- a/ivlpp/globals.h +++ b/ivlpp/globals.h @@ -53,6 +53,8 @@ extern char dep_mode; extern int verbose_flag; +extern int warn_redef; + /* This is the entry to the lexer. */ extern int yylex(void); diff --git a/ivlpp/lexor.lex b/ivlpp/lexor.lex index a0aa71b42..c03057d65 100644 --- a/ivlpp/lexor.lex +++ b/ivlpp/lexor.lex @@ -868,8 +868,8 @@ void define_macro(const char* name, const char* value, int keyword, int argc) prev = def_lookup(name); if (prev) { emit_pathline(istack); - fprintf(stderr, "warning: redefinition of macro %s\n", - name); + fprintf(stderr, "warning: redefinition of macro %s from value '%s' to '%s'\n", + name, prev->value, value); } def = malloc(sizeof(struct define_t)); diff --git a/ivlpp/main.c b/ivlpp/main.c index 7ec80cb86..b4909b647 100644 --- a/ivlpp/main.c +++ b/ivlpp/main.c @@ -98,6 +98,9 @@ int line_direct_flag = 0; unsigned error_count = 0; FILE *depend_file = NULL; +/* Should we warn about macro redefinitions? */ +int warn_redef = 0; + static int flist_read_flags(const char*path) { char line_buf[2048]; @@ -282,7 +285,7 @@ int main(int argc, char*argv[]) include_dir[0] = 0; /* 0 is reserved for the current files path. */ include_dir[1] = strdup("."); - while ((opt=getopt(argc, argv, "F:f:K:Lo:p:P:vV")) != EOF) switch (opt) { + while ((opt=getopt(argc, argv, "F:f:K:Lo:p:P:vVW:")) != EOF) switch (opt) { case 'F': flist_read_flags(optarg); @@ -336,6 +339,12 @@ int main(int argc, char*argv[]) break; } + case 'W': { + if (strcmp(optarg, "redef")==0) + warn_redef = 1; + break; + } + case 'v': fprintf(stderr, "Icarus Verilog Preprocessor version " VERSION " (" VERSION_TAG ")\n\n"); @@ -366,7 +375,8 @@ int main(int argc, char*argv[]) " -p - Write precompiled defines to \n" " -P - Read precompiled defines from \n" " -v - Verbose\n" - " -V - Print version information and quit\n", + " -V - Print version information and quit\n" + " -W - Enable extra ivlpp warning category (e.g. redef)\n", argv[0]); return flag_errors; } From 6c2fba1139f506323cefdf6cf551ab7953ba24c8 Mon Sep 17 00:00:00 2001 From: Andrew Andrianov Date: Thu, 19 Oct 2017 11:40:07 +0300 Subject: [PATCH 3/6] driver: Implement -Wr handling for macro redefinitions Signed-off-by: Andrew Andrianov --- driver/main.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/driver/main.c b/driver/main.c index bda5697ca..a4873e829 100644 --- a/driver/main.c +++ b/driver/main.c @@ -138,7 +138,7 @@ int gen_std_include = 1; of the include list. */ int gen_relative_include = 0; -char warning_flags[16] = "n"; +char warning_flags[17] = "n"; /* Boolean: true means ignore errors about missing modules */ int ignore_missing_modules = 0; @@ -505,6 +505,10 @@ static void process_warning_switch(const char*name) process_warning_switch("select-range"); process_warning_switch("timescale"); process_warning_switch("sensitivity-entire-array"); + process_warning_switch("macro-redefinition"); + } else if (strcmp(name,"macro-redefinition") == 0) { + if (! strchr(warning_flags, 'r')) + strcat(warning_flags, "r"); } else if (strcmp(name,"anachronisms") == 0) { if (! strchr(warning_flags, 'n')) strcat(warning_flags, "n"); @@ -1288,8 +1292,10 @@ int main(int argc, char **argv) /* Write the preprocessor command needed to preprocess a single file. This may be used to preprocess library files. */ - fprintf(iconfig_file, "ivlpp:%s%civlpp -L -F\"%s\" -P\"%s\"\n", - ivlpp_dir, sep, defines_path, compiled_defines_path); + fprintf(iconfig_file, "ivlpp:%s%civlpp -L -F\"%s\" -P\"%s\" %s\n", + ivlpp_dir, sep, defines_path, compiled_defines_path, + strchr(warning_flags, 'r') ? "-Wredef" : "" + ); /* Done writing to the iconfig file. Close it now. */ fclose(iconfig_file); From 94e42e5bfc0071a74d7ab3fe539ac26857487738 Mon Sep 17 00:00:00 2001 From: Andrew Andrianov Date: Thu, 19 Oct 2017 11:40:50 +0300 Subject: [PATCH 4/6] driver: Update manpage Signed-off-by: Andrew Andrianov --- driver/iverilog.man.in | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/driver/iverilog.man.in b/driver/iverilog.man.in index ba05d2a0a..f8425713a 100644 --- a/driver/iverilog.man.in +++ b/driver/iverilog.man.in @@ -310,8 +310,8 @@ after a \fB\-Wall\fP argument to suppress isolated warning types. .TP 8 .B all -This enables the anachronisms, implicit, portbind, select\-range, -timescale, and sensitivity\-entire\-array warning categories. +This enables the anachronisms, implicit, macro-redefinition, portbind, +select\-range, timescale, and sensitivity\-entire\-array warning categories. .TP 8 .B anachronisms @@ -324,6 +324,10 @@ This enables warnings for creation of implicit declarations. For example, if a scalar wire X is used but not declared in the Verilog source, this will print a warning at its first use. +.TP 8 +.B macro-redefinition +This enables preprocessor warnings when a macro is being redefined. + .TP 8 .B portbind This enables warnings for ports of module instantiations that are not From 9e2252bc398144e6505e6af81a31e59ef2e4de8c Mon Sep 17 00:00:00 2001 From: Andrew Andrianov Date: Sun, 22 Oct 2017 13:28:15 +0300 Subject: [PATCH 5/6] ivlpp: Actually take warn_redef flag into account Signed-off-by: Andrew Andrianov --- ivlpp/lexor.lex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ivlpp/lexor.lex b/ivlpp/lexor.lex index c03057d65..b8ccb6296 100644 --- a/ivlpp/lexor.lex +++ b/ivlpp/lexor.lex @@ -866,7 +866,7 @@ void define_macro(const char* name, const char* value, int keyword, int argc) */ prev = def_lookup(name); - if (prev) { + if (prev && warn_redef) { emit_pathline(istack); fprintf(stderr, "warning: redefinition of macro %s from value '%s' to '%s'\n", name, prev->value, value); From 3e87a4d242b9ad7919229ff29419149016cccdba Mon Sep 17 00:00:00 2001 From: Andrew Andrianov Date: Sun, 22 Oct 2017 13:53:07 +0300 Subject: [PATCH 6/6] driver: Add -Wno-macro-redefinition, properly pass -Wredef to ivlpp Signed-off-by: Andrew Andrianov --- driver/main.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/driver/main.c b/driver/main.c index a4873e829..c39e1bd37 100644 --- a/driver/main.c +++ b/driver/main.c @@ -344,9 +344,12 @@ static int t_version_only(void) static void build_preprocess_command(int e_flag) { - snprintf(tmp, sizeof tmp, "%s%civlpp %s%s -F\"%s\" -f\"%s\" -p\"%s\" ", - ivlpp_dir, sep, verbose_flag?" -v":"", - e_flag?"":" -L", defines_path, source_path, + snprintf(tmp, sizeof tmp, "%s%civlpp %s%s%s -F\"%s\" -f\"%s\" -p\"%s\" ", + ivlpp_dir, sep, + verbose_flag?" -v":"", + e_flag?"":" -L", + strchr(warning_flags, 'r') ? " -Wredef " : "", + defines_path, source_path, compiled_defines_path); } @@ -547,6 +550,12 @@ static void process_warning_switch(const char*name) cp[0] = cp[1]; cp += 1; } + } else if (strcmp(name,"no-macro-redefinition") == 0) { + char*cp = strchr(warning_flags, 'r'); + if (cp) while (*cp) { + cp[0] = cp[1]; + cp += 1; + } } else if (strcmp(name,"no-floating-nets") == 0) { char*cp = strchr(warning_flags, 'f'); if (cp) while (*cp) { @@ -1292,9 +1301,10 @@ int main(int argc, char **argv) /* Write the preprocessor command needed to preprocess a single file. This may be used to preprocess library files. */ - fprintf(iconfig_file, "ivlpp:%s%civlpp -L -F\"%s\" -P\"%s\" %s\n", - ivlpp_dir, sep, defines_path, compiled_defines_path, - strchr(warning_flags, 'r') ? "-Wredef" : "" + fprintf(iconfig_file, "ivlpp:%s%civlpp %s -L -F\"%s\" -P\"%s\"\n", + ivlpp_dir, sep, + strchr(warning_flags, 'r') ? "-Wredef" : "", + defines_path, compiled_defines_path ); /* Done writing to the iconfig file. Close it now. */