cilly does not handle input on stdin correctly

[stephenrkell]

cilly assumes that source files have names with meaningful extensions. In the case of the source file "-", denoting stdin, this doesn't work.

$ gcc hello.c -S -o hello.s # works okay
$ file hello.s; rm -f hello.s
hello.s: assembler source text

$ cilly hello.c -S -o hello.s # works okay
$ file hello.s; rm -f hello.s
hello.s: assembler source text

$ cat hello.c | gcc -x c - -S -o hello.s # works okay
$ file hello.s; rm -f hello.s
hello.s assembler source text

$ cat hello.c | cilly -x c - -S -o hello.s # BUG!
$ file hello.s
hello.s: ERROR: cannot open `hello.s' (No such file or directory)

A related problem is that cilly doesn't correctly handle gcc's "-x " option, which is often used in combination with input on stdin. Currently cilly just passes this options to the compiler --
which sometimes works, but doesn't preserve the option's semantics. For example,

$ gcc -E -o hello.cc hello.c
$ gcc -x cpp-output -o hello hello.cc

... works, whereas

$ cilly -E -o hello.cc hello.c
$ cilly -x cpp-output -o hello hello.cc
(snip)
/tmp/cil-jSfqyHOy.o:6:4: error: stray ?\2? in program
/tmp/cil-jSfqyHOy.o:6:21: warning: null character(s) ignored
/tmp/cil-jSfqyHOy.o:6:4: error: stray ?\2? in program
/tmp/cil-jSfqyHOy.o:6:25: warning: null character(s) ignored

I have prepared a rough patch with fixes both of these problems. It's not great code (my Perl is amateurish) and could use a quick review. (Am happy to submit by e-mail, and/or rework any part that could do with a better solution.)

diff --git a/lib/perl5/App/Cilly.pm.in b/lib/perl5/App/Cilly.pm.in
index 63f9163..6e48d1f 100644
--- a/lib/perl5/App/Cilly.pm.in
+++ b/lib/perl5/App/Cilly.pm.in
@@ -199,7 +199,7 @@ sub collectArgumentList {
 #
 #        my $arg = shift @args; # Grab the next one
         if(! $self->collectOneArgument($arg, \@args)) {
-            print "Warning: Unknown argument $arg\n";
+            if (! $arg eq "-") { print STDERR "Warning: Unknown argument $arg\n"; }
             push @{$self->{CCARGS}}, $arg;
         }
     }
@@ -541,25 +541,38 @@ sub preprocess_compile {
     &mydebug("preprocess_compile(src=$src, dest=$dest)\n");
     Carp::confess "bad dest: $dest" unless $dest->isa('App::Cilly::OutputFile');

-    my ($base, $dir, $ext) = fileparse($src, "\\.[^.]+");
-    if($ext eq ".c" || $ext eq ".cpp" || $ext eq ".cc") {
-        if($self->leaveAlone($src)) {
-            print "Leaving alone $src\n";
-            # We leave this alone. So just compile as usual
-            return $self->straight_compile($src, $dest, [@{$early_ppargs}, @{$ppargs}], $ccargs);
-        }
-        my $out    = $self->preprocessOutputFile($src);
-        $out = $self->preprocess($src, $out, 
-                                 [@{$early_ppargs}, @{$ppargs},
-                                  "$self->{DEFARG}CIL=1"]);
-        return $self->compile($out, $dest, $ppargs, $ccargs);
+    if (! $src eq "-") {
+      my ($base, $dir, $ext) = fileparse($src, "\\.[^.]+");
+      # check that they're sane
+      die "preprocess_compile requires an unpreprocessed source file! got $src\n"
+         unless ($ext eq ".c" || $ext eq ".cpp" || $ext eq ".cc");
     }
-    if($ext eq ".i") {
-        return $self->compile($src, $dest, $ppargs, $ccargs);
+
+    if($self->leaveAlone($src)) {
+        print "Leaving alone $src\n";
+        # We leave this alone. So just compile as usual
+        return $self->straight_compile($src, $dest, [@{$early_ppargs}, @{$ppargs}], $ccargs);
     }
-    if($ext eq ".$::cilbin") {
-        return $self->compile($src, $dest, $ppargs, $ccargs);
+    my $out    = $self->preprocessOutputFile($src);
+    $out = $self->preprocess($src, $out,
+                             [@{$early_ppargs}, @{$ppargs},
+                              "$self->{DEFARG}CIL=1"]);
+    return $self->compile($out, $dest, $ppargs, $ccargs);
+}
+
+sub compile_nopp {
+    my ($self, $src, $dest, $early_ppargs, $ppargs, $ccargs) = @_;
+    &mydebug("compile_nopp(src=$src, dest=$dest)\n");
+    Carp::confess "bad dest: $dest" unless $dest->isa('App::Cilly::OutputFile');
+
+    if (! $src eq "-") {
+      my ($base, $dir, $ext) = fileparse($src, "\\.[^.]+");
+      # check that they're sane
+      die "compile_nopp requires a preprocessed source file! got $src\n"
+         unless ($ext eq ".i" || $ext eq ".$::cilbin");
     }
+
+    return $self->compile($src, $dest, $ppargs, $ccargs);
 }

 # THIS IS THE ENTRY POINT FOR JUST PREPROCESSING A FILE
@@ -1095,8 +1108,8 @@ sub doit {
     # we can go ahead with the compilation, without having to save 
     # files
     if(! $self->{SEPARATE} && # Not already separate mode
-       $self->{OPERATION} eq "TOEXE" &&  # We are linking to an executable
-       @{$self->{CFILES}} + @{$self->{IFILES}} <= 1) { # At most one source
+       @{$self->{CFILES}} + @{$self->{IFILES}} <= 1 &&  # At most one source
+       ($self->{OPERATION} eq "TOEXE" || defined $self->{OUTARG})) {  # We are linking to an executable
         # If we have object files, we should keep merging if at least one 
         # object file is a disguised source
         my $turnOffMerging = 0;
@@ -1117,16 +1130,22 @@ sub doit {
         }
     }

-    # Turn everything into OBJ files
+    # Turn everything into OBJ files (or assembly, if the user asked for that)
     my @tolink = ();
-
-    foreach $file (@{$self->{IFILES}}, @{$self->{CFILES}}) {
+    foreach $file (@{$self->{CFILES}}) {
         $out = $self->compileOutputFile($file);
         $self->preprocess_compile($file, $out,
                  $self->{EARLY_PPARGS},
                                   $self->{PPARGS}, $self->{CCARGS});
         push @tolink, $out;
     }
+    foreach $file (@{$self->{IFILES}}) {
+        $out = $self->compileOutputFile($file);
+        $self->compile_nopp($file, $out,
+                 $self->{EARLY_PPARGS},
+                                  $self->{PPARGS}, $self->{CCARGS});
+        push @tolink, $out;
+    }
     # Now do the assembly language file
     foreach $file (@{$self->{SFILES}}) {
         $out = $self->assembleOutputFile($file);
@@ -1184,23 +1203,25 @@ sub doit {
 }

 sub classifyArgDebug {
-    if(0) { print @_; }
+    if(1) { print @_; }
 }

 sub mydebug {
-    if(0) { print @_; }
+    if(1) { print @_; }
 }

 sub compilerArgument {
     my($self, $options, $arg, $pargs) = @_;
     &classifyArgDebug("Classifying arg: $arg\n");
     my $idx = 0;
+    my $matched = 0;
     for($idx=0; $idx < $#$options; $idx += 2) {
         my $key = ${$options}[$idx];
         my $action = ${$options}[$idx + 1];
         &classifyArgDebug("Try match with $key\n");
         if($arg =~ m|^$key|) {
           &classifyArgDebug(" match with $key\n");
+          $matched = 1;
           my @fullarg = ($arg);
           my $onemore;
           if(defined $action->{'ONEMORE'}) {
@@ -1218,6 +1239,21 @@ sub compilerArgument {
               }
               &classifyArgDebug(" onemore=$onemore\n");
           }
+          # If we have CURLANG, it might override the action the caller
+          # supplied.
+          if (defined $self->{'CURLANG'}) {
+            if ($action->{TYPE} =~ /.*SOURCE$/) {
+              if    ($self->{CURLANG} =~ /c|c-header/)          { $action->{TYPE} = "CSOURCE"; }
+              elsif ($self->{CURLANG} =~ /cpp-output/)          { $action->{TYPE} = "ISOURCE"; }
+              elsif ($self->{CURLANG} =~ /assembler/)           { $action->{TYPE} = "ASMSOURCE"; }
+              elsif ($self->{CURLANG} =~ /assembler-woth-cpp/)  { $action->{TYPE} = "ASMSOURCE"; }
+              else {
+                print "Warning: didn't understand current language $self->{CURLANG}\n";
+              }
+            }
+          } elsif ($action->{TYPE} eq "UNKNOWNSOURCE") {
+            print "Warning: saw source of unknown language, with no preceding -x <lang>\n";
+          }
           # Now see what action we must perform
           my $argument_done = 1;
           if(defined $action->{'RUN'}) {
@@ -1240,7 +1276,17 @@ sub compilerArgument {
               elsif($action->{TYPE} eq "CC") {
                   push @{$self->{CCARGS}}, @fullarg; return 1;
               }
-              elsif($action->{TYPE} eq "LINKCC") {
+              elsif($action->{TYPE} eq "SETLANG") {
+                  # We pass it to the C compiler, as if it were a "CC" action,
+                  # but we also keep some state so we can classify input files.
+                  if ($onemore eq "none") {
+                    delete $self->{CURLANG};
+                  } else {
+                    $self->{CURLANG} = $onemore;
+                  }
+                  push @{$self->{CCARGS}}, @fullarg; return 1;
+              }
+            elsif($action->{TYPE} eq "LINKCC") {
                   push @{$self->{CCARGS}}, @fullarg; 
                   push @{$self->{LINKARGS}}, @fullarg; return 1;
               }
@@ -1255,6 +1301,7 @@ sub compilerArgument {
               elsif($action->{TYPE} eq "CSOURCE") {
                 App::Cilly::OutputFile->protect(@fullarg);
                   $fullarg[0] = &normalizeFileName($fullarg[0]);
+                  print "Pushing $fullarg[0]\n";
                   push @{$self->{CFILES}}, @fullarg; return 1;
               }
               elsif($action->{TYPE} eq "ASMSOURCE") {
@@ -1285,6 +1332,7 @@ sub compilerArgument {
           print "Don't know what to do with option $arg\n"; 
           return 0;
       }
+
    }
    return 0;
 }
@@ -1924,6 +1972,7 @@ sub new {
             "[^-].*\\.($::cilbin|c|cpp|cc)\$" => { TYPE => 'CSOURCE' },
             "[^-].*\\.(s|S)\$" => { TYPE => 'ASMSOURCE' },
             "[^-].*\\.i\$" => { TYPE => 'ISOURCE' },
+            "-\$" => { TYPE => 'UNKNOWNSOURCE' },
             # .o files can be linker scripts
             "[^-]" => { RUN => sub { &GNUCC::parseLinkerScript(@_); }},

@@ -1935,7 +1984,7 @@ sub new {
             "-o" => { ONEMORE => 1, TYPE => 'OUT' },
        "-combine\$" => { TYPE => 'ALLARGS' },
        "-pipe\$" => { TYPE => 'ALLARGS' },
-            "-x" => { ONEMORE => 1, TYPE => "CC" },
+       "-x" => { ONEMORE => 1, TYPE => "SETLANG" },
        "-v" => { TYPE => 'ALLARGS',
              RUN => sub { $stub->{TRACE_COMMANDS} = 1; } },
        # skipping -###, --help, --target-help, --version
@@ -2176,8 +2225,11 @@ sub lineDirective {
 sub compileOutputFile {
     my($self, $src) = @_;

-    die "objectOutputFile: not a C source file: $src\n"
-   unless $src =~ /\.($::cilbin|c|cc|cpp|i|s|S)$/;
+    # in the "-" case we will have an OUTARG
+    if ($src eq "-") { die "input is '-' but no outarg\n" unless defined $self->{OUTARG}; }
+    else { die "objectOutputFile: not a C source file: $src\n"
+   unless $src =~ /\.($::cilbin|c|cc|cpp|i|asm)$/;
+    }

     if ($self->{OPERATION} eq 'TOOBJ'
         || ($self->{OPERATION} eq 'TOASM')) {