From 7ed3b986b9a8e13834112b65a5d9ba9bd82bf8cd Mon Sep 17 00:00:00 2001
From: ottmar-zittlau <74417452+ottmar-zittlau@users.noreply.github.com>
Date: Wed, 8 Jan 2025 01:04:16 +0100
Subject: [PATCH] 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
---
 toolchain/driver/compile_subcommand.cpp | 11 +++++++----
 toolchain/driver/driver_test.cpp        | 11 +++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp
index 175a0c6313063..f0cb9e47d1e85 100644
--- a/toolchain/driver/compile_subcommand.cpp
+++ b/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",
diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp
index 3e6fd0d77944e..3c8112d9b4589 100644
--- a/toolchain/driver/driver_test.cpp
+++ b/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("");