Fix bug in depfile when output filename includes special characters

The rewriting of the dependency file content introduced in 2ab31632 just
replaces the object file part of the dependency file with the raw
filename. This is incorrect if the filename includes special characters
such as space or dollar.

Fix this by escaping the filename properly. Note that newlines are not
treated specially since Make cannot handle them anyway.

See also the similar bug fix for the depend mode (but for parsing
instead of escaping in that case) in 1d2b5bf4.

diff --git a/src/Depfile.cpp b/src/Depfile.cpp
index 90da9c9ccf9ba6ede2a05b9c54b39a00247b51f7..ed491f4dc17a14d3a9a03f340c8839df1ecb65c2 100644 (file)
--- a/src/Depfile.cpp
+++ b/src/Depfile.cpp
@@ -31,6 +31,29 @@ is_blank(const std::string& s)
 
 namespace Depfile {
 
+std::string
+escape_filename(nonstd::string_view filename)
+{
+  std::string result;
+  result.reserve(filename.size());
+  for (const char c : filename) {
+    switch (c) {
+    case '\\':
+    case '#':
+    case ':':
+    case ' ':
+    case '\t':
+      result.push_back('\\');
+      break;
+    case '$':
+      result.push_back('$');
+      break;
+    }
+    result.push_back(c);
+  }
+  return result;
+}
+
 nonstd::optional<std::string>
 rewrite_paths(const Context& ctx, const std::string& file_content)
 {

diff --git a/src/Depfile.hpp b/src/Depfile.hpp
index a6a67e5005609041fcea4752c41894bf00fe9e35..770f78964d314eadb57710a52d6fbc04992c7809 100644 (file)
--- a/src/Depfile.hpp
+++ b/src/Depfile.hpp
@@ -29,6 +29,7 @@ class Hash;
 
 namespace Depfile {
 
+std::string escape_filename(nonstd::string_view filename);
 nonstd::optional<std::string> rewrite_paths(const Context& ctx,
                                             const std::string& file_content);
 void make_paths_relative_in_output_dep(const Context& ctx);

diff --git a/src/ResultRetriever.cpp b/src/ResultRetriever.cpp
index 04deaaf588c655208f9e548e989d7da9176c68f8..77e044dd0ee85674a4a010e68c49033b20a2f5e8 100644 (file)
--- a/src/ResultRetriever.cpp
+++ b/src/ResultRetriever.cpp
@@ -19,6 +19,7 @@
 #include "ResultRetriever.hpp"
 
 #include "Context.hpp"
+#include "Depfile.hpp"
 #include "Logging.hpp"
 
 using Result::FileType;
@@ -163,9 +164,10 @@ ResultRetriever::write_dependency_file()
     if (m_rewrite_dependency_target) {
       size_t colon_pos = m_dest_data.find(':');
       if (colon_pos != std::string::npos) {
-        Util::write_fd(*m_dest_fd,
-                       m_ctx.args_info.output_obj.data(),
-                       m_ctx.args_info.output_obj.length());
+        const auto escaped_output_obj =
+          Depfile::escape_filename(m_ctx.args_info.output_obj);
+        Util::write_fd(
+          *m_dest_fd, escaped_output_obj.data(), escaped_output_obj.length());
         start_pos = colon_pos;
       }
     }

diff --git a/test/suites/depend.bash b/test/suites/depend.bash
index ca48bee4533116d7036aa11f31093b647b19120f..0d9fcc5bef45af9017d34e61bcd3340c8058b466 100644 (file)
--- a/test/suites/depend.bash
+++ b/test/suites/depend.bash
@@ -294,6 +294,23 @@ EOF
     expect_stat 'files in cache' 5
 
     # -------------------------------------------------------------------------
+    TEST "Source file with special characters"
 
-    # TODO: Add more test cases (see direct.bash for inspiration)
+    touch 'file with$special#characters.c'
+    $REAL_COMPILER -MMD -c 'file with$special#characters.c'
+    mv 'file with$special#characters.d' reference.d
+
+    CCACHE_DEPEND=1 $CCACHE_COMPILE -MMD -c 'file with$special#characters.c'
+    expect_stat 'cache hit (direct)' 0
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 1
+    expect_equal_content 'file with$special#characters.d' reference.d
+
+    rm 'file with$special#characters.d'
+
+    CCACHE_DEPEND=1 $CCACHE_COMPILE -MMD -c 'file with$special#characters.c'
+    expect_stat 'cache hit (direct)' 1
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 1
+    expect_equal_content 'file with$special#characters.d' reference.d
 }

diff --git a/unittest/test_Depfile.cpp b/unittest/test_Depfile.cpp
index e0bbf142fc3d90476dfe0613a57fab07dc437a27..b098800b41ba6246b1363287d97f876baa9c443c 100644 (file)
--- a/unittest/test_Depfile.cpp
+++ b/unittest/test_Depfile.cpp
@@ -27,6 +27,17 @@ using TestUtil::TestContext;
 
 TEST_SUITE_BEGIN("Depfile");
 
+TEST_CASE("Depfile::escape_filename")
+{
+  CHECK(Depfile::escape_filename("") == "");
+  CHECK(Depfile::escape_filename("foo") == "foo");
+  CHECK(Depfile::escape_filename("foo\\bar") == "foo\\\\bar");
+  CHECK(Depfile::escape_filename("foo#bar") == "foo\\#bar");
+  CHECK(Depfile::escape_filename("foo bar") == "foo\\ bar");
+  CHECK(Depfile::escape_filename("foo\tbar") == "foo\\\tbar");
+  CHECK(Depfile::escape_filename("foo$bar") == "foo$$bar");
+}
+
 TEST_CASE("Depfile::rewrite_paths")
 {
   Context ctx;