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

@@ -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

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.

includes/RtsFlags.h

-#ifndef MAKING_GHC_BUILD_SYSTEM_DEPENDENCIES
-#warning RtsFlags.h is DEPRECATED; please just #include "Rts.h"
-#endif
-
-#include "Rts.h"

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 */

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

rts/RtsFlags.c

@@ -13,6 +13,7 @@
 #include "RtsOpts.h"
 #include "RtsUtils.h"
 #include "Profiling.h"
+#include "RtsFlags.h"
 
 #ifdef HAVE_CTYPE_H
 #include <ctype.h>
@@ -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 <program>.<ext> */
-            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

rts/RtsFlags.h

+/* -----------------------------------------------------------------------------
+ *
+ * (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 */

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 */

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)
 

rts/hooks/RtsOptsEnabled.c

@@ -9,5 +9,5 @@
 #include "Rts.h"
 #include "RtsOpts.h"
 
-const rtsOptsEnabledEnum rtsOptsEnabled = rtsOptsSafeOnly;
+const RtsOptsEnabledEnum rtsOptsEnabled = RtsOptsSafeOnly;