From 82ecace939df039b0216333cc587546267afcb20 Mon Sep 17 00:00:00 2001
From: Joel Rosdahl <joel@rosdahl.net>
Date: Fri, 29 May 2020 20:43:19 +0200
Subject: [PATCH] Only try to hard link object files to/from the cache
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The compiler unlinks the destination object file before writing, but it
apparently doesn’t do that for dependency files. This means that
compilation can corrupt a .d file that shares i-node with a cached .d
file when using the hard link mode. Here is a scenario where this can
happen:

1. There is a test.c which includes test.h.
2. When test.c is compiled, the compiler writes test.d which mentions
   test.h and ccache hard links test.d into cache entry 1. test.d and
   cache entry 1's .d file now share i-nodes.
3. The include of test.h is removed from test.c.
4. When test.c is compiled again the compiler overwrites test.d with new
   content without test.h and ccache hard links test.d into cache entry
   2. test.d, cache entry 1 and cache entry 2 now share i-nodes, all of
   which contain the new content without test.h.

Since we can’t be sure how the compiler behaves for other types of files
(.dwo, .cov, etc.), only try to to hard link object files.

Fixes #599.

(cherry picked from commit 443afc179e1de3f050850103a5ba62e3cb5ab398)
---
 src/result.cpp            | 28 +++++++++++++++-------------
 test/suites/hardlink.bash | 29 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/result.cpp b/src/result.cpp
index 462b90988..d78d1981c 100644
--- a/src/result.cpp
+++ b/src/result.cpp
@@ -437,21 +437,23 @@ should_store_raw_file(const Config& config, FileType type)
     return false;
   }
 
-  // - Don't store stderr outputs as raw files since they:
-  //   1. Never are large.
-  //   2. Will end up in a temporary file anyway.
+  // Only store object files as raw files since there are several problems with
+  // storing other file types:
   //
-  // - Don't store .d/dependency files since they:
-  //   1. Never are large.
-  //   2. Compress well.
-  //   3. Cause trouble for automake if hard-linked (see ccache issue 378).
+  // 1. The compiler unlinks object files before writing to them but it doesn't
+  //    unlink .d files, so just it's possible to corrupt .d files just by
+  //    running the compiler (see ccache issue 599).
+  // 2. .d files cause trouble for automake if hard-linked (see ccache issue
+  //    378).
+  // 3. It's unknown how the compiler treats other file types, so better safe
+  //    than sorry.
   //
-  // Note that .d files can't be stored as raw files for the file_clone case
-  // since the hard link mode happily will try to use them if they exist. This
-  // could be fixed by letting read_raw_file_entry refuse to hard link .d
-  // files, but it's easier to simply always store them embedded. This will
-  // also save i-nodes in the cache.
-  return type != FileType::stderr_output && type != FileType::dependency;
+  // It would be possible to store all files in raw form for the file_clone case
+  // and only hard link object files. However, most likely it's only object
+  // files that become large enough that it's of interest to clone or hard link
+  // them, so we keep things simple for now. This will also save i-nodes in the
+  // cache.
+  return type == FileType::object;
 }
 
 static void
diff --git a/test/suites/hardlink.bash b/test/suites/hardlink.bash
index e78871e2c..1cb5fc84b 100644
--- a/test/suites/hardlink.bash
+++ b/test/suites/hardlink.bash
@@ -92,4 +92,33 @@ SUITE_hardlink() {
     CCACHE_HARDLINK=1 CCACHE_DEPEND=1 $CCACHE_COMPILE -c -MMD -MF test1.d.tmp test1.c
     expect_stat 'cache hit (direct)' 1
     mv test1.d.tmp test1.d || test_failed "second mv failed"
+
+    # -------------------------------------------------------------------------
+    TEST ".d file corrupted by compiler"
+
+    unset CCACHE_NODIRECT
+    export CCACHE_SLOPPINESS=include_file_mtime,include_file_ctime
+    export CCACHE_HARDLINK=1
+
+    echo "int x;" >test1.c
+
+    $CCACHE_COMPILE -c -MMD test1.c
+    expect_stat 'cache hit (direct)' 0
+    expect_stat 'cache miss' 1
+    expect_file_content test1.d "test1.o: test1.c"
+
+    touch test1.h
+    echo '#include "test1.h"' >>test1.c
+
+    $CCACHE_COMPILE -c -MMD test1.c
+    expect_stat 'cache hit (direct)' 0
+    expect_stat 'cache miss' 2
+    expect_file_content test1.d "test1.o: test1.c test1.h"
+
+    echo "int x;" >test1.c
+
+    $CCACHE_COMPILE -c -MMD test1.c
+    expect_stat 'cache hit (direct)' 1
+    expect_stat 'cache miss' 2
+    expect_file_content test1.d "test1.o: test1.c"
 }
-- 
2.47.3