Remove memory corruption testing from base::File().

This was intended to help debug issue 424562 (see Issue 702473009 for
more details), and it isn't needed any more.

R=thakis@chromium.org,pasko@chromium.org,thestig@chromium.org
BUG=424562

Review URL: https://codereview.chromium.org/1372113002

Cr-Commit-Position: refs/heads/master@{#351194}

3 files changed, 2 insertions, 163 deletions

diff --git a/base/files/file.h b/base/files/file.h
index cba4353..976188b 100644
--- a/base/files/file.h
+++ b/base/files/file.h
@@ -21,7 +21,6 @@
 #include "base/files/file_path.h"
 #include "base/files/file_tracing.h"
 #include "base/files/scoped_file.h"
-#include "base/gtest_prod_util.h"
 #include "base/move.h"
 #include "base/time/time.h"
 
@@ -29,8 +28,6 @@
 #include "base/win/scoped_handle.h"
 #endif
 
-FORWARD_DECLARE_TEST(FileTest, MemoryCorruption);
-
 namespace base {
 
 #if defined(OS_WIN)
@@ -306,55 +303,8 @@ class BASE_EXPORT File {
   static std::string ErrorToString(Error error);
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(::FileTest, MemoryCorruption);
-
   friend class FileTracing::ScopedTrace;
 
-#if defined(OS_POSIX)
-  // Encloses a single ScopedFD, saving a cheap tamper resistent memory checksum
-  // alongside it. This checksum is validated at every access, allowing early
-  // detection of memory corruption.
-
-  // TODO(gavinp): This is in place temporarily to help us debug
-  // https://crbug.com/424562 , which can't be reproduced in valgrind. Remove
-  // this code after we have fixed this issue.
-  class MemoryCheckingScopedFD {
-   public:
-    MemoryCheckingScopedFD();
-    MemoryCheckingScopedFD(int fd);
-    ~MemoryCheckingScopedFD();
-
-    bool is_valid() const { Check(); return file_.is_valid(); }
-    int get() const { Check(); return file_.get(); }
-
-    void reset() { Check(); file_.reset(); UpdateChecksum(); }
-    void reset(int fd) { Check(); file_.reset(fd); UpdateChecksum(); }
-    int release() {
-      Check();
-      int fd = file_.release();
-      UpdateChecksum();
-      return fd;
-    }
-
-   private:
-    FRIEND_TEST_ALL_PREFIXES(::FileTest, MemoryCorruption);
-
-    // Computes the checksum for the current value of |file_|. Returns via an
-    // out parameter to guard against implicit conversions of unsigned integral
-    // types.
-    void ComputeMemoryChecksum(unsigned int* out_checksum) const;
-
-    // Confirms that the current |file_| and |file_memory_checksum_| agree,
-    // failing a CHECK if they do not.
-    void Check() const;
-
-    void UpdateChecksum();
-
-    ScopedFD file_;
-    unsigned int file_memory_checksum_;
-  };
-#endif
-
   // Creates or opens the given file. Only called if |path| has no
   // traversal ('..') components.
   void DoInitialize(const FilePath& path, uint32 flags);
@@ -368,7 +318,7 @@ class BASE_EXPORT File {
 #if defined(OS_WIN)
   win::ScopedHandle file_;
 #elif defined(OS_POSIX)
-  MemoryCheckingScopedFD file_;
+  ScopedFD file_;
 #endif
 
   // A path to use for tracing purposes. Set if file tracing is enabled during
@@ -386,3 +336,4 @@ class BASE_EXPORT File {
 }  // namespace base
 
 #endif  // BASE_FILES_FILE_H_
+
diff --git a/base/files/file_posix.cc b/base/files/file_posix.cc
index 7fb617c..a5aee01 100644
--- a/base/files/file_posix.cc
+++ b/base/files/file_posix.cc
@@ -420,49 +420,6 @@ File::Error File::OSErrorToFileError(int saved_errno) {
   }
 }
 
-File::MemoryCheckingScopedFD::MemoryCheckingScopedFD() {
-  UpdateChecksum();
-}
-
-File::MemoryCheckingScopedFD::MemoryCheckingScopedFD(int fd) : file_(fd) {
-  UpdateChecksum();
-}
-
-File::MemoryCheckingScopedFD::~MemoryCheckingScopedFD() {}
-
-// static
-void File::MemoryCheckingScopedFD::ComputeMemoryChecksum(
-    unsigned int* out_checksum) const {
-  // Use a single iteration of a linear congruentional generator (lcg) to
-  // provide a cheap checksum unlikely to be accidentally matched by a random
-  // memory corruption.
-
-  // By choosing constants that satisfy the Hull-Duebell Theorem on lcg cycle
-  // length, we insure that each distinct fd value maps to a distinct checksum,
-  // which maximises the utility of our checksum.
-
-  // This code uses "unsigned int" throughout for its defined modular semantics,
-  // which implicitly gives us a divisor that is a power of two.
-
-  const unsigned int kMultiplier = 13035 * 4 + 1;
-  COMPILE_ASSERT(((kMultiplier - 1) & 3) == 0, pred_must_be_multiple_of_four);
-  const unsigned int kIncrement = 1595649551;
-  COMPILE_ASSERT(kIncrement & 1, must_be_coprime_to_powers_of_two);
-
-  *out_checksum =
-      static_cast<unsigned int>(file_.get()) * kMultiplier + kIncrement;
-}
-
-void File::MemoryCheckingScopedFD::Check() const {
-  unsigned int computed_checksum;
-  ComputeMemoryChecksum(&computed_checksum);
-  CHECK_EQ(file_memory_checksum_, computed_checksum) << "corrupted fd memory";
-}
-
-void File::MemoryCheckingScopedFD::UpdateChecksum() {
-  ComputeMemoryChecksum(&file_memory_checksum_);
-}
-
 // NaCl doesn't implement system calls to open files directly.
 #if !defined(OS_NACL)
 // TODO(erikkay): does it make sense to support FLAG_EXCLUSIVE_* here?
diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc
index fd79a37..67dbbfd 100644
--- a/base/files/file_unittest.cc
+++ b/base/files/file_unittest.cc
@@ -5,7 +5,6 @@
 #include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -510,71 +509,3 @@ TEST(FileTest, GetInfoForDirectory) {
   EXPECT_EQ(0, info.size);
 }
 #endif  // defined(OS_WIN)
-
-#if defined(OS_POSIX) && defined(GTEST_HAS_DEATH_TEST)
-TEST(FileTest, MemoryCorruption) {
-  {
-    // Test that changing the checksum value is detected.
-    base::File file;
-    EXPECT_NE(file.file_.file_memory_checksum_,
-              static_cast<unsigned int>(file.GetPlatformFile()));
-    file.file_.file_memory_checksum_ = file.GetPlatformFile();
-    EXPECT_DEATH(file.IsValid(), "");
-
-    file.file_.UpdateChecksum();  // Do not crash on File::~File().
-  }
-
-  {
-    // Test that changing the file descriptor value is detected.
-    base::File file;
-    file.file_.file_.reset(17);
-    EXPECT_DEATH(file.IsValid(), "");
-
-    // Do not crash on File::~File().
-    ignore_result(file.file_.file_.release());
-    file.file_.UpdateChecksum();
-  }
-
-  {
-    // Test that GetPlatformFile() checks for corruption.
-    base::File file;
-    file.file_.file_memory_checksum_ = file.GetPlatformFile();
-    EXPECT_DEATH(file.GetPlatformFile(), "");
-
-    file.file_.UpdateChecksum();  // Do not crash on File::~File().
-  }
-
-  {
-    // Test that the base::File destructor checks for corruption.
-    scoped_ptr<base::File> file(new File());
-    file->file_.file_memory_checksum_ = file->GetPlatformFile();
-    EXPECT_DEATH(file.reset(), "");
-
-    // Do not crash on this thread's destructor call.
-    file->file_.UpdateChecksum();
-  }
-
-  {
-    // Test that the base::File constructor checks for corruption.
-    base::File file;
-    file.file_.file_memory_checksum_ = file.GetPlatformFile();
-    EXPECT_DEATH(File f(file.Pass()), "");
-
-    file.file_.UpdateChecksum();  // Do not crash on File::~File().
-  }
-
-  {
-    // Test that doing IO checks for corruption.
-    base::File file;
-    file.file_.file_.reset(17);  // A fake open FD value.
-
-    EXPECT_DEATH(file.Seek(File::FROM_BEGIN, 0), "");
-    EXPECT_DEATH(file.Read(0, NULL, 0), "");
-    EXPECT_DEATH(file.ReadAtCurrentPos(NULL, 0), "");
-    EXPECT_DEATH(file.Write(0, NULL, 0), "");
-
-    ignore_result(file.file_.file_.release());
-    file.file_.UpdateChecksum();
-  }
-}
-#endif  // defined(OS_POSIX)