From 5463b55b7dadc1e9918edb2d8666bf3ed195bc61 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Tue, 12 Apr 2011 13:21:41 +0100 Subject: [PATCH] Cleanup sweep and fix a bug in RTS flag processing. This code has accumulated a great deal of cruft over the years, this pass cleans up a lot of the surrounding cruft but leaves the actual argument processing alone - so there's still more that could be done. Bug fixed: - ghc_rts_opts should not be subject to the --rtsopts setting. If the programmer explicitly declares options with ghc_rts_opts, they shouldn't also have to accept command-line RTS options to make them work. --- compiler/main/DriverPipeline.hs | 6 +- ghc/hschooks.c | 1 - includes/RtsFlags.h | 5 - includes/RtsOpts.h | 8 +- includes/rts/Flags.h | 7 -- rts/RtsFlags.c | 242 +++++++++++++++++++++++++-------------- rts/RtsFlags.h | 23 ++++ rts/RtsStartup.c | 4 +- rts/ghc.mk | 1 + rts/hooks/RtsOptsEnabled.c | 2 +- 10 files changed, 190 insertions(+), 109 deletions(-) delete mode 100644 includes/RtsFlags.h create mode 100644 rts/RtsFlags.h diff --git a/compiler/main/DriverPipeline.hs b/compiler/main/DriverPipeline.hs index bc16ede..d7d6ae3 100644 --- a/compiler/main/DriverPipeline.hs +++ b/compiler/main/DriverPipeline.hs @@ -1420,13 +1420,13 @@ mkExtraObjToLinkIntoBinary dflags dep_packages = do mk_rts_opts_enabled val = vcat [text "#include \"Rts.h\"", text "#include \"RtsOpts.h\"", - text "const rtsOptsEnabledEnum rtsOptsEnabled = " <> + text "const RtsOptsEnabledEnum rtsOptsEnabled = " <> text val <> semi ] rts_opts_enabled = case rtsOptsEnabled dflags of - RtsOptsNone -> mk_rts_opts_enabled "rtsOptsNone" + RtsOptsNone -> mk_rts_opts_enabled "RtsOptsNone" RtsOptsSafeOnly -> empty -- The default - RtsOptsAll -> mk_rts_opts_enabled "rtsOptsAll" + RtsOptsAll -> mk_rts_opts_enabled "RtsOptsAll" extra_rts_opts = case rtsOpts dflags of Nothing -> empty diff --git a/ghc/hschooks.c b/ghc/hschooks.c index 4878021..1867928 100644 --- a/ghc/hschooks.c +++ b/ghc/hschooks.c @@ -20,7 +20,6 @@ defaultsHook (void) RtsFlags.GcFlags.heapSizeSuggestion = 6*1024*1024 / BLOCK_SIZE; RtsFlags.GcFlags.maxStkSize = 512*1024*1024 / sizeof(W_); RtsFlags.GcFlags.giveStats = COLLECT_GC_STATS; - RtsFlags.GcFlags.statsFile = stderr; // See #3408: the default idle GC time of 0.3s is too short on // Windows where we receive console events once per second or so. diff --git a/includes/RtsFlags.h b/includes/RtsFlags.h deleted file mode 100644 index a6b4d2c..0000000 --- a/includes/RtsFlags.h +++ /dev/null @@ -1,5 +0,0 @@ -#ifndef MAKING_GHC_BUILD_SYSTEM_DEPENDENCIES -#warning RtsFlags.h is DEPRECATED; please just #include "Rts.h" -#endif - -#include "Rts.h" diff --git a/includes/RtsOpts.h b/includes/RtsOpts.h index e81a41c..b8eab68 100644 --- a/includes/RtsOpts.h +++ b/includes/RtsOpts.h @@ -9,8 +9,12 @@ #ifndef RTSOPTS_H #define RTSOPTS_H -typedef enum {rtsOptsNone, rtsOptsSafeOnly, rtsOptsAll} rtsOptsEnabledEnum; +typedef enum { + RtsOptsNone, // +RTS causes an error + RtsOptsSafeOnly, // safe RTS options allowed; others cause an error + RtsOptsAll // all RTS options allowed + } RtsOptsEnabledEnum; -extern const rtsOptsEnabledEnum rtsOptsEnabled; +extern const RtsOptsEnabledEnum rtsOptsEnabled; #endif /* RTSOPTS_H */ diff --git a/includes/rts/Flags.h b/includes/rts/Flags.h index 95ccfc0..b4e7b64 100644 --- a/includes/rts/Flags.h +++ b/includes/rts/Flags.h @@ -219,13 +219,6 @@ extern RTS_FLAGS RtsFlags[]; extern RTS_FLAGS RtsFlags; #endif -/* Routines that operate-on/to-do-with RTS flags: */ - -void initRtsFlagsDefaults(void); -void setupRtsFlags(int *argc, char *argv[], int *rts_argc, char *rts_argv[]); -void setProgName(char *argv[]); - - /* * The printf formats are here, so we are less likely to make * overly-long filenames (with disastrous results). No more than 128 diff --git a/rts/RtsFlags.c b/rts/RtsFlags.c index 2530edd..408e1c7 100644 --- a/rts/RtsFlags.c +++ b/rts/RtsFlags.c @@ -13,6 +13,7 @@ #include "RtsOpts.h" #include "RtsUtils.h" #include "Profiling.h" +#include "RtsFlags.h" #ifdef HAVE_CTYPE_H #include @@ -44,20 +45,26 @@ char *rts_argv[MAX_RTS_ARGS]; Static function decls -------------------------------------------------------------------------- */ -static int /* return NULL on error */ -open_stats_file ( - I_ arg, - int argc, char *argv[], - int rts_argc, char *rts_argv[], - const char *FILENAME_FMT, - FILE **file_ret); +static void procRtsOpts (int rts_argc0, RtsOptsEnabledEnum enabled); + +static void normaliseRtsOpts (void); + +static void initStatsFile (FILE *f); + +static int openStatsFile (char *filename, const char *FILENAME_FMT, + FILE **file_ret); + +static StgWord64 decodeSize (const char *flag, nat offset, + StgWord64 min, StgWord64 max); + +static void bad_option (const char *s); -static StgWord64 decodeSize(const char *flag, nat offset, StgWord64 min, StgWord64 max); -static void bad_option(const char *s); #ifdef TRACING static void read_trace_flags(char *arg); #endif +static void errorUsage (void) GNU_ATTRIBUTE(__noreturn__); + /* ----------------------------------------------------------------------------- * Command-line option parsing routines. * ---------------------------------------------------------------------------*/ @@ -360,8 +367,7 @@ strequal(const char *a, const char * b) return(strcmp(a, b) == 0); } -static void -splitRtsFlags(char *s, int *rts_argc, char *rts_argv[]) +static void splitRtsFlags(char *s) { char *c1, *c2; @@ -373,11 +379,11 @@ splitRtsFlags(char *s, int *rts_argc, char *rts_argv[]) if (c1 == c2) { break; } - if (*rts_argc < MAX_RTS_ARGS-1) { + if (rts_argc < MAX_RTS_ARGS-1) { s = stgMallocBytes(c2-c1+1, "RtsFlags.c:splitRtsFlags()"); strncpy(s, c1, c2-c1); s[c2-c1] = '\0'; - rts_argv[(*rts_argc)++] = s; + rts_argv[rts_argc++] = s; } else { barf("too many RTS arguments (max %d)", MAX_RTS_ARGS-1); } @@ -386,27 +392,48 @@ splitRtsFlags(char *s, int *rts_argc, char *rts_argv[]) } while (*c1 != '\0'); } -void -setupRtsFlags(int *argc, char *argv[], int *rts_argc, char *rts_argv[]) +/* ----------------------------------------------------------------------------- + Parse the command line arguments, collecting options for the RTS. + + On return: + - argv[] is *modified*, any RTS options have been stripped out + - *argc contains the new count of arguments in argv[] + + - rts_argv[] (global) contains the collected RTS args + - rts_argc (global) contains the count of args in rts_argv + + - prog_argv[] (global) contains the non-RTS args (== argv) + - prog_argc (global) contains the count of args in prog_argv + + - prog_name (global) contains the basename of argv[0] + + -------------------------------------------------------------------------- */ + +void setupRtsFlags (int *argc, char *argv[]) { - rtsBool error = rtsFalse; - I_ mode; - I_ arg, total_arg; + nat mode; + nat total_arg; + nat arg, rts_argc0; setProgName (argv); total_arg = *argc; arg = 1; *argc = 1; - *rts_argc = 0; + rts_argc = 0; + + rts_argc0 = rts_argc; // process arguments from the ghc_rts_opts global variable first. // (arguments from the GHCRTS environment variable and the command // line override these). { if (ghc_rts_opts != NULL) { - splitRtsFlags(ghc_rts_opts, rts_argc, rts_argv); - } + splitRtsFlags(ghc_rts_opts); + // opts from ghc_rts_opts are always enabled: + procRtsOpts(rts_argc0, RtsOptsAll); + rts_argc0 = rts_argc; + } } // process arguments from the GHCRTS environment variable next @@ -415,14 +442,15 @@ setupRtsFlags(int *argc, char *argv[], int *rts_argc, char *rts_argv[]) char *ghc_rts = getenv("GHCRTS"); if (ghc_rts != NULL) { - if (rtsOptsEnabled != rtsOptsNone) { - splitRtsFlags(ghc_rts, rts_argc, rts_argv); - } - else { + if (rtsOptsEnabled == RtsOptsNone) { errorBelch("Warning: Ignoring GHCRTS variable as RTS options are disabled.\n Link with -rtsopts to enable them."); // We don't actually exit, just warn + } else { + splitRtsFlags(ghc_rts); + procRtsOpts(rts_argc0, rtsOptsEnabled); + rts_argc0 = rts_argc; } - } + } } // Split arguments (argv) into PGM (argv) and RTS (rts_argv) parts @@ -440,19 +468,13 @@ setupRtsFlags(int *argc, char *argv[], int *rts_argc, char *rts_argv[]) break; } else if (strequal("+RTS", argv[arg])) { - if (rtsOptsEnabled != rtsOptsNone) { - mode = RTS; - } - else { - errorBelch("RTS options are disabled. Link with -rtsopts to enable them."); - stg_exit(EXIT_FAILURE); - } - } + mode = RTS; + } else if (strequal("-RTS", argv[arg])) { mode = PGM; } - else if (mode == RTS && *rts_argc < MAX_RTS_ARGS-1) { - rts_argv[(*rts_argc)++] = argv[arg]; + else if (mode == RTS && rts_argc < MAX_RTS_ARGS-1) { + rts_argv[rts_argc++] = argv[arg]; } else if (mode == PGM) { argv[(*argc)++] = argv[arg]; @@ -466,17 +488,45 @@ setupRtsFlags(int *argc, char *argv[], int *rts_argc, char *rts_argv[]) argv[(*argc)++] = argv[arg]; } argv[*argc] = (char *) 0; - rts_argv[*rts_argc] = (char *) 0; + rts_argv[rts_argc] = (char *) 0; + + procRtsOpts(rts_argc0, rtsOptsEnabled); + + normaliseRtsOpts(); + + setProgArgv(*argc, argv); + + if (RtsFlags.GcFlags.statsFile != NULL) { + initStatsFile (RtsFlags.GcFlags.statsFile); + } + if (RtsFlags.TickyFlags.tickyFile != NULL) { + initStatsFile (RtsFlags.GcFlags.statsFile); + } +} + +/* ----------------------------------------------------------------------------- + * procRtsOpts: Process rts_argv between rts_argc0 and rts_argc. + * -------------------------------------------------------------------------- */ + +static void procRtsOpts (int rts_argc0, RtsOptsEnabledEnum enabled) +{ + rtsBool error = rtsFalse; + int arg; // Process RTS (rts_argv) part: mainly to determine statsfile - for (arg = 0; arg < *rts_argc; arg++) { - if (rts_argv[arg][0] != '-') { + for (arg = rts_argc0; arg < rts_argc; arg++) { + if (rts_argv[arg][0] != '-') { fflush(stdout); errorBelch("unexpected RTS argument: %s", rts_argv[arg]); error = rtsTrue; } else { + if (enabled == RtsOptsNone) { + errorBelch("RTS options are disabled. Link with -rtsopts to enable them."); + stg_exit(EXIT_FAILURE); + } + switch(rts_argv[arg][1]) { case '-': if (strequal("info", &rts_argv[arg][2])) { @@ -488,8 +538,7 @@ setupRtsFlags(int *argc, char *argv[], int *rts_argc, char *rts_argv[]) break; } - if (rtsOptsEnabled != rtsOptsAll) - { + if (enabled == RtsOptsSafeOnly) { errorBelch("Most RTS options are disabled. Link with -rtsopts to enable them."); stg_exit(EXIT_FAILURE); } @@ -791,9 +840,8 @@ error = rtsTrue; stats: { int r; - r = open_stats_file(arg, *argc, argv, - *rts_argc, rts_argv, NULL, - &RtsFlags.GcFlags.statsFile); + r = openStatsFile(rts_argv[arg]+2, NULL, + &RtsFlags.GcFlags.statsFile); if (r == -1) { error = rtsTrue; } } break; @@ -1097,9 +1145,9 @@ error = rtsTrue; { int r; - r = open_stats_file(arg, *argc, argv, - *rts_argc, rts_argv, TICKY_FILENAME_FMT, - &RtsFlags.TickyFlags.tickyFile); + r = openStatsFile(rts_argv[arg]+2, + TICKY_FILENAME_FMT, + &RtsFlags.TickyFlags.tickyFile); if (r == -1) { error = rtsTrue; } } ) break; @@ -1184,6 +1232,16 @@ error = rtsTrue; } } + if (error) errorUsage(); +} + +/* ----------------------------------------------------------------------------- + * normaliseRtsOpts: Set some derived values, and make sure things are + * within sensible ranges. + * -------------------------------------------------------------------------- */ + +static void normaliseRtsOpts (void) +{ if (RtsFlags.MiscFlags.tickInterval < 0) { RtsFlags.MiscFlags.tickInterval = 50; } @@ -1235,20 +1293,20 @@ error = rtsTrue; if (RtsFlags.GcFlags.stkChunkBufferSize > RtsFlags.GcFlags.stkChunkSize / 2) { errorBelch("stack chunk buffer size (-kb) must be less than 50%% of the stack chunk size (-kc)"); - error = rtsTrue; + errorUsage(); } +} - if (error) { - const char **p; +static void errorUsage (void) +{ + const char **p; - fflush(stdout); - for (p = usage_text; *p; p++) - errorBelch("%s", *p); - stg_exit(EXIT_FAILURE); - } + fflush(stdout); + for (p = usage_text; *p; p++) + errorBelch("%s", *p); + stg_exit(EXIT_FAILURE); } - static void stats_fprintf(FILE *f, char *s, ...) { @@ -1262,49 +1320,62 @@ stats_fprintf(FILE *f, char *s, ...) va_end(ap); } -static int /* return -1 on error */ -open_stats_file ( - I_ arg, - int argc, char *argv[], - int rts_argc, char *rts_argv[], - const char *FILENAME_FMT, - FILE **file_ret) +/* ----------------------------------------------------------------------------- + * openStatsFile: open a file in which to put some runtime stats + * -------------------------------------------------------------------------- */ + +static int // return -1 on error +openStatsFile (char *filename, // filename, or NULL + const char *filename_fmt, // if filename == NULL, use + // this fmt with sprintf to + // generate the filename. %s + // expands to the program name. + FILE **file_ret) // return the FILE* { FILE *f = NULL; - if (strequal(rts_argv[arg]+2, "stderr") - || (FILENAME_FMT == NULL && rts_argv[arg][2] == '\0')) { + if (strequal(filename, "stderr") + || (filename_fmt == NULL && *filename == '\0')) { f = NULL; /* NULL means use debugBelch */ } else { - if (rts_argv[arg][2] != '\0') { /* stats file specified */ - f = fopen(rts_argv[arg]+2,"w"); + if (*filename != '\0') { /* stats file specified */ + f = fopen(filename,"w"); } else { char stats_filename[STATS_FILENAME_MAXLEN]; /* default . */ - sprintf(stats_filename, FILENAME_FMT, argv[0]); + sprintf(stats_filename, filename_fmt, prog_name); f = fopen(stats_filename,"w"); } if (f == NULL) { - errorBelch("Can't open stats file %s\n", rts_argv[arg]+2); + errorBelch("Can't open stats file %s\n", filename); return -1; } } *file_ret = f; - { - /* Write argv and rtsv into start of stats file */ - int count; - for(count = 0; count < argc; count++) { - stats_fprintf(f, "%s ", argv[count]); - } - stats_fprintf(f, "+RTS "); - for(count = 0; count < rts_argc; count++) - stats_fprintf(f, "%s ", rts_argv[count]); - stats_fprintf(f, "\n"); - } return 0; } +/* ----------------------------------------------------------------------------- + * initStatsFile: write a line to the file containing the program name + * and the arguments it was invoked with. +-------------------------------------------------------------------------- */ +static void initStatsFile (FILE *f) +{ + /* Write prog_argv and rts_argv into start of stats file */ + int count; + for (count = 0; count < prog_argc; count++) { + stats_fprintf(f, "%s ", prog_argv[count]); + } + stats_fprintf(f, "+RTS "); + for (count = 0; count < rts_argc; count++) + stats_fprintf(f, "%s ", rts_argv[count]); + stats_fprintf(f, "\n"); +} + +/* ----------------------------------------------------------------------------- + * decodeSize: parse a string containing a size, like 300K or 1.2M +-------------------------------------------------------------------------- */ static StgWord64 decodeSize(const char *flag, nat offset, StgWord64 min, StgWord64 max) @@ -1420,14 +1491,9 @@ getProgArgv(int *argc, char **argv[]) void setProgArgv(int argc, char *argv[]) { - /* Usually this is done by startupHaskell, so we don't need to call this. - However, sometimes Hugs wants to change the arguments which Haskell - getArgs >>= ... will be fed. So you can do that by calling here - _after_ calling startupHaskell. - */ - prog_argc = argc; - prog_argv = argv; - setProgName(prog_argv); + prog_argc = argc; + prog_argv = argv; + setProgName(prog_argv); } /* These functions record and recall the full arguments, including the diff --git a/rts/RtsFlags.h b/rts/RtsFlags.h new file mode 100644 index 0000000..3ebfef6 --- /dev/null +++ b/rts/RtsFlags.h @@ -0,0 +1,23 @@ +/* ----------------------------------------------------------------------------- + * + * (c) The AQUA Project, Glasgow University, 1994-1997 + * (c) The GHC Team, 1998-2006 + * + * Functions for parsing the argument list. + * + * ---------------------------------------------------------------------------*/ + +#ifndef RTSFLAGS_H +#define RTSFLAGS_H + +#include "BeginPrivate.h" + +/* Routines that operate-on/to-do-with RTS flags: */ + +void initRtsFlagsDefaults (void); +void setupRtsFlags (int *argc, char *argv[]); +void setProgName (char *argv[]); + +#include "EndPrivate.h" + +#endif /* RTSFLAGS_H */ diff --git a/rts/RtsStartup.c b/rts/RtsStartup.c index 266c048..b860667 100644 --- a/rts/RtsStartup.c +++ b/rts/RtsStartup.c @@ -16,6 +16,7 @@ #include "HsFFI.h" #include "sm/Storage.h" +#include "RtsFlags.h" #include "RtsUtils.h" #include "Prelude.h" #include "Schedule.h" /* initScheduler */ @@ -129,8 +130,7 @@ hs_init(int *argc, char **argv[]) /* Parse the flags, separating the RTS flags from the programs args */ if (argc != NULL && argv != NULL) { setFullProgArgv(*argc,*argv); - setupRtsFlags(argc, *argv, &rts_argc, rts_argv); - setProgArgv(*argc,*argv); + setupRtsFlags(argc, *argv); } /* Initialise the stats department, phase 1 */ diff --git a/rts/ghc.mk b/rts/ghc.mk index 3e0e11a..df68bc5 100644 --- a/rts/ghc.mk +++ b/rts/ghc.mk @@ -457,6 +457,7 @@ rts_dist_MKDEPENDC_OPTS += -Irts/dist/build endif $(eval $(call build-dependencies,rts,dist,1)) +$(eval $(call include-dependencies,rts,dist,1)) $(rts_dist_depfile_c_asm) : libffi/dist-install/build/ffi.h $(DTRACEPROBES_H) diff --git a/rts/hooks/RtsOptsEnabled.c b/rts/hooks/RtsOptsEnabled.c index f5d8157..f20c325 100644 --- a/rts/hooks/RtsOptsEnabled.c +++ b/rts/hooks/RtsOptsEnabled.c @@ -9,5 +9,5 @@ #include "Rts.h" #include "RtsOpts.h" -const rtsOptsEnabledEnum rtsOptsEnabled = rtsOptsSafeOnly; +const RtsOptsEnabledEnum rtsOptsEnabled = RtsOptsSafeOnly; -- 1.7.10.4