Bladeren bron

Fix unchecked optional access in compile subcommand (#4756)

Hi,

I fixed a small issue that I found inside the "compile subcommand"
component:
The program can be crashed by running ```bazel run -- toolchain:carbon
compile --dump-mem-usage "non-existing-file.carbon"``` - i.e. by
activating the memory usage dump flag and passing a non-existing file.

Best regards,
oz
ottmar-zittlau 1 jaar geleden
bovenliggende
commit
7ed3b986b9
2 gewijzigde bestanden met toevoegingen van 18 en 4 verwijderingen
  1. 7 4
      toolchain/driver/compile_subcommand.cpp
  2. 11 0
      toolchain/driver/driver_test.cpp

+ 7 - 4
toolchain/driver/compile_subcommand.cpp

@@ -369,14 +369,17 @@ class CompilationUnit {
       source_ = SourceBuffer::MakeFromFileOrStdin(*driver_env_->fs,
                                                   input_filename_, *consumer_);
     });
-    if (mem_usage_) {
-      mem_usage_->Add("source_", source_->text().size(),
-                      source_->text().size());
-    }
+
     if (!source_) {
       success_ = false;
       return;
     }
+
+    if (mem_usage_) {
+      mem_usage_->Add("source_", source_->text().size(),
+                      source_->text().size());
+    }
+
     CARBON_VLOG("*** SourceBuffer ***\n```\n{0}\n```\n", source_->text());
 
     LogCall("Lex::Lex", "lex",

+ 11 - 0
toolchain/driver/driver_test.cpp

@@ -122,6 +122,17 @@ TEST_F(DriverTest, CompileCommandErrors) {
       StrEq("error: not all required positional arguments were provided; first "
             "missing and required positional argument: `FILE`\n"));
 
+  // Pass non-existing file
+  EXPECT_FALSE(driver_
+                   .RunCommand({"compile", "--dump-mem-usage",
+                                "non-existing-file.carbon"})
+                   .success);
+  EXPECT_THAT(
+      test_error_stream_.TakeStr(),
+      ContainsRegex("No such file or directory[\\n]*non-existing-file.carbon"));
+  // Flush output stream, because it's ok that it's not empty here.
+  test_output_stream_.TakeStr();
+
   // Invalid output filename. No reliably error message here.
   // TODO: Likely want a different filename on Windows.
   auto empty_file = MakeTestFile("");