[m-rev.] for review [juliensf]: Avoid command line buffer limits

Peter Ross pro at missioncriticalit.com
Wed Oct 8 10:59:10 AEDT 2008


On Mon, Oct 06, 2008 at 03:23:30PM +1100, Peter Ross wrote:
> Hi,
> 
> 
> ===================================================================
> 
> 
> Estimated hours taken: 4
> Branches: main
> 
> Windows has a command line length limit, and I have started to hit
> it when using a MinGW version of the compiler, so use files to
> pass argument lists which are potentially very long to programs.
> 
> 
> compiler/compile_target_code.m:
> 	Make all calls to mkinit use -f to pass the list 
> 	of files to process.
> 	For linking use the @file syntax to pass all the
> 	arguments to a command as when linking we
> 	can have very long lists of object files.
> 
> compiler/handle_options.m:
> 	Describe how mmc handles @files.
> 	
> compiler/mercury_compile.m:
> 	Handle @file arguments on the command line.
> 
> 
Here is the interdiff addressing the review comments.

Plus fixing a problem I found under mingw with '\' in path names.
Plus a final problem when linking using gcc using @file under mingw
would call a sub-command collect2 with a long list of arguments
which would fail.

The solution for that problem was to create an archive with all
the program .o files in it and then use gcc to link that thus
generating a much shorter command line.

diff -u compiler/compile_target_code.m compiler/compile_target_code.m
--- compiler/compile_target_code.m	6 Oct 2008 04:03:26 -0000
+++ compiler/compile_target_code.m	7 Oct 2008 23:53:59 -0000
@@ -1724,7 +1726,6 @@
     % Find which system libraries are needed.
     get_system_libs(LinkTargetType, SystemLibs, !IO),
 
-    join_quoted_string_list(ObjectsList, "", "", " ", Objects),
     globals.io_lookup_accumulating_option(LDFlagsOpt, LDFlagsList, !IO),
     join_string_list(LDFlagsList, "", "", " ", LDFlags),
     globals.io_lookup_accumulating_option(link_library_directories,
@@ -1811,41 +1814,76 @@
         join_quoted_string_list(LinkLibrariesList, "", "", " ",
             LinkLibraries),
 
-        % Note that LDFlags may contain `-l' options so it should come
-        % after Objects.
-        globals.io_lookup_string_option(CommandOpt, Command, !IO),
-        string.append_list([
-                StaticOpts, " ",
-                StripOpt, " ",
-                UndefOpt, " ",
-                ThreadOpts, " ",
-                TraceOpts, " ",
-                " -o ", OutputFileName, " ",
-                Objects, " ",
-                LinkOptSep, " ",
-                LinkLibraryDirectories, " ",
-                RpathOpts, " ",
-                InstallNameOpt, " ",
-                DebugOpts, " ",
-                LDFlags, " ",
-                LinkLibraries, " ",
-                MercuryStdLibs, " ",
-                SystemLibs], Args),
-
-        globals.io_lookup_bool_option(demangle, Demangle, !IO),
+        globals.io_lookup_bool_option(restricted_command_line,
+            RestrictedCommandLine, !IO),
         (
-            Demangle = yes,
-            globals.io_lookup_string_option(demangle_command,
-                DemangleCmd, !IO),
-            MaybeDemangleCmd = yes(DemangleCmd)
+            % If we have a restricted command line then it's possible
+            % that following link command will call sub-commands its self
+            % and thus overflow the command line, so in this case
+            % we first create an archive of all of the 
+            %
+            RestrictedCommandLine = yes,
+            io.make_temp(TmpArchive, !IO),
+            create_archive(ErrorStream, TmpArchive, yes, ObjectsList,
+                ArchiveSucceeded, !IO),
+            MaybeDeleteTmpArchive = yes(TmpArchive),
+            Objects = TmpArchive
         ;
-            Demangle = no,
-            MaybeDemangleCmd = no
+            RestrictedCommandLine = no,
+            ArchiveSucceeded = yes,
+            MaybeDeleteTmpArchive = no,
+            join_quoted_string_list(ObjectsList, "", "", " ", Objects)
         ),
+        
+        ( 
+            ArchiveSucceeded = yes,
+
+            % Note that LDFlags may contain `-l' options so it should come
+            % after Objects.
+            globals.io_lookup_string_option(CommandOpt, Command, !IO),
+            string.append_list([
+                    Command, " ",
+                    StaticOpts, " ",
+                    StripOpt, " ",
+                    UndefOpt, " ",
+                    ThreadOpts, " ",
+                    TraceOpts, " ",
+                    " -o ", OutputFileName, " ",
+                    Objects, " ",
+                    LinkOptSep, " ",
+                    LinkLibraryDirectories, " ",
+                    RpathOpts, " ",
+                    InstallNameOpt, " ",
+                    DebugOpts, " ",
+                    LDFlags, " ",
+                    LinkLibraries, " ",
+                    MercuryStdLibs, " ",
+                    SystemLibs], LinkCmd),
 
-        invoke_long_system_command_maybe_filter_output(ErrorStream,
-            cmd_verbose_commands, Command, Args, MaybeDemangleCmd,
-            LinkSucceeded, !IO)
+            globals.io_lookup_bool_option(demangle, Demangle, !IO),
+            (
+                Demangle = yes,
+                globals.io_lookup_string_option(demangle_command,
+                    DemangleCmd, !IO),
+                MaybeDemangleCmd = yes(DemangleCmd)
+            ;
+                Demangle = no,
+                MaybeDemangleCmd = no
+            ),
+
+            invoke_system_command_maybe_filter_output(ErrorStream,
+                cmd_verbose_commands, LinkCmd, MaybeDemangleCmd,
+                LinkSucceeded, !IO)
+        ;
+            ArchiveSucceeded = no,
+            LinkSucceeded = no
+        ),
+        (
+            MaybeDeleteTmpArchive = yes(FileToDelete),
+            io.remove_file(FileToDelete, _, !IO)
+        ;
+            MaybeDeleteTmpArchive = no
+        )
     ;
         LibrariesSucceeded = no,
         LinkSucceeded = no
@@ -2661,8 +2699,12 @@
 
 invoke_long_system_command_maybe_filter_output(ErrorStream, Verbosity,
         Cmd, Args, MaybeProcessOutput, Succeeded, !IO) :-
-    support_at_file(SupportAtFile, !IO),
-    ( SupportAtFile = yes,
+    globals.io_lookup_bool_option(restricted_command_line,
+        RestrictedCommandLine, !IO),
+
+    (
+        RestrictedCommandLine = yes,
+
         %
         % Avoid generating very long command lines by using @files
         %
@@ -2670,9 +2712,25 @@
         io.open_output(TmpFile, OpenResult, !IO),
         (
             OpenResult = ok(TmpStream),
-            io.write_string(TmpStream, Args, !IO),
+
+            % We need to escape any \ before writing them to
+            % the file, otherwise we lose them
+            TmpFileArgs = string.replace_all(Args, "\\", "\\\\"),
+
+            io.write_string(TmpStream, TmpFileArgs, !IO),
             io.close_output(TmpStream, !IO),
 
+            globals.io_lookup_bool_option(very_verbose, VeryVerbose, !IO),
+            (
+                VeryVerbose = yes,
+                io.write_string("% Args placed in @" ++ TmpFile ++ ": `", !IO),
+                io.write_string(TmpFileArgs, !IO),
+                io.write_string("'\n", !IO),
+                io.flush_output(!IO)
+            ;
+                VeryVerbose = no
+            ),
+
             FullCmd = string.append_list([Cmd, " @", TmpFile]),
             invoke_system_command(ErrorStream, Verbosity,
                 FullCmd, Succeeded0, !IO),
@@ -2690,21 +2748,13 @@
             Succeeded = no
         )
         
-    ; SupportAtFile = no,
+    ;
+        RestrictedCommandLine = no,
         FullCmd = Cmd ++ " " ++ Args,
         invoke_system_command_maybe_filter_output(ErrorStream, Verbosity,
             FullCmd, MaybeProcessOutput, Succeeded, !IO)
     ).
 
-    % Does the underlying tool chain support using "@file" to pass
-    % arguments?
-    %
-    % XXX This should be made a conditional option, so that toolchains
-    % which don't support this can be used.
-:- pred support_at_file(bool::out, io::di, io::uo) is det.
-
-support_at_file(yes, !IO).
-
 %-----------------------------------------------------------------------------%
 %
 % C compiler flags
only in patch2:
unchanged:
--- doc/user_guide.texi	30 Sep 2008 04:30:32 -0000	1.576
+++ doc/user_guide.texi	7 Oct 2008 23:53:58 -0000
@@ -269,6 +269,13 @@
 to file name, where @var{sources-files} is the list of source files in
 the directory (@pxref{Output options}).
 
+Arguments to @samp{mmc} may also be in @samp{@@file}.
+The @samp{@@file} argument is replaced with arguments
+representing the contents of the file.
+This argument processing is done recursively.
+The contents of the @samp{@@file} is split into arguments
+one per line in the file.
+
 To compile a program which consists of just a single source file,
 use the command
 
@@ -8989,6 +8996,14 @@
 Install the specified C header file along with a Mercury library.
 (This option is only supported by @samp{mmc --make})
 
+ at sp 1
+ at item --restricted-command-line
+ at findex --restricted-command-line
+ at findex --extra-lib-header
+Enable this option if your shell doesn't support long command lines.
+This option uses temporary files to pass arguments to sub-commands.
+(This option is only supported by @samp{mmc --make})
+
 @end table
 
 @node Miscellaneous options
only in patch2:
unchanged:
--- compiler/options.m	30 Sep 2008 04:30:28 -0000	1.635
+++ compiler/options.m	7 Oct 2008 23:54:00 -0000
@@ -889,6 +889,7 @@
     ;       libgrade_install_check
     ;       show_make_times
     ;       extra_library_header
+    ;       restricted_command_line
 
     % Miscellaneous Options
     ;       filenames_from_stdin
@@ -1722,7 +1723,8 @@
     use_search_directories_for_intermod -   bool(yes),
     libgrade_install_check              -   bool(yes),
     show_make_times                     -   bool(no),
-    extra_library_header                -   accumulating([])
+    extra_library_header                -   accumulating([]),
+    restricted_command_line             -   bool(no)
 ]).
 option_defaults_2(miscellaneous_option, [
     % Miscellaneous Options
@@ -2603,6 +2605,7 @@
 long_option("show-make-times",      show_make_times).
 long_option("extra-lib-header",     extra_library_header).
 long_option("extra-library-header", extra_library_header).
+long_option("restricted-command-line", restricted_command_line).
 
 % misc options
 long_option("typecheck-ambiguity-warn-limit",
@@ -5304,6 +5307,10 @@
         "--extra-library-header <file>, --extra-lib-header <file>",
         "\tInstall the specified C header file with along with",
         "\ta Mercury library.",
+        "\t(This option is only supported by `mmc --make'.)",
+        "--restricted-command-line",
+        "\tEnable this option if your shell doesn't support long command lines.",
+        "\tThis option uses temporary files to pass arguments to sub-commands.",
         "\t(This option is only supported by `mmc --make'.)"
     ]).
 
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------



More information about the reviews mailing list