1. Make sure chrome_elf.dll imports nothing besides kernel32, advapi32, and...

1. Make sure chrome_elf.dll imports nothing besides kernel32, advapi32, and some msvc libs (DEBUG builds)
2. Add gyp action and test to ensure chrome_elf.dll is always the first entry in chrome.exe's import table.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242834

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243048

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245197

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245312 0039d316-1c4b-4281-b951-d872f2087c98

build/win/reorder-imports.py

+#!/usr/bin/env python
+# Copyright 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import glob
+import optparse
+import os
+import shutil
+import subprocess
+import sys
+
+def reorder_imports(input_dir, output_dir):
+  """Run swapimports.exe on the initial chrome.exe, and write to the output
+  directory. Also copy over any related files that might be needed
+  (pdbs, manifests etc.).
+  """
+  input_image = '--input-image=%s' % (os.path.join(input_dir, 'chrome.exe'))
+  output_image = '--output-image=%s' % (os.path.join(output_dir, 'chrome.exe'))
+
+  swap_exe = os.path.join(
+      __file__,
+      '..\\..\\..\\third_party\\syzygy\\binaries\\exe\\swapimport.exe')
+  subprocess.call(
+      [swap_exe, input_image, output_image, '--overwrite', 'chrome_elf.dll'])
+
+  for fname in glob.iglob(os.path.join(input_dir, 'chrome.exe.*')):
+    shutil.copy(fname, os.path.join(output_dir, os.path.basename(fname)))
+  return 0
+
+
+def main(argv):
+  usage = 'reorder_imports.py -i <input_dir> -o <output_dir>'
+  parser = optparse.OptionParser(usage=usage)
+  parser.add_option('-i', '--input', help='reorder chrome.exe in DIR',
+      metavar='DIR')
+  parser.add_option('-o', '--output', help='write new chrome.exe to DIR',
+      metavar='DIR')
+  opts, args = parser.parse_args()
+
+  if not opts.input or not opts.output:
+    parser.error('Please provide and input and output directory')
+  return reorder_imports(opts.input, opts.output)
+
+if __name__ == "__main__":
+  sys.exit(main(sys.argv[1:]))

chrome/chrome_exe.gypi

@@ -6,7 +6,42 @@
   'targets': [
     {
       'target_name': 'chrome',
+      'type': 'none',
+      'dependencies': [ 'chrome_initial', ],
+      'conditions': [
+        ['OS == "win"', {
+          'actions': [
+            {
+              'variables': {
+                'reorder_py_path': '<(DEPTH)/build/win/reorder-imports.py',
+                'exe_input_path':'$(OutDir)\\initial',
+                'exe_output_path':'<(PRODUCT_DIR)',
+              },
+              'action_name': 'reorder_imports',
+              'inputs': [
+                '<(reorder_py_path)',
+              ],
+              'outputs': [
+                '<(PRODUCT_DIR)\\chrome.exe',
+                '<(PRODUCT_DIR)\\chrome.exe.pdb',
+              ],
+              'action': [
+                'python',
+                '<(reorder_py_path)',
+                '-i', '<(exe_input_path)',
+                '-o', '<(exe_output_path)',
+              ],
+              'message': 'Reordering Imports',
+            },
+          ],
+        }],
+      ],
+    },
+    {
+      'target_name': 'chrome_initial',
       'type': 'executable',
+      # Name the exe chrome.exe, not chrome_initial.exe.
+      'product_name': 'chrome',
       'mac_bundle': 1,
       'variables': {
         'use_system_xdg_utils%': 0,
@@ -468,8 +503,6 @@
         }],
         ['OS=="win"', {
           'dependencies': [
-            # Note that chrome_elf must be listed first. Do not reorder it.
-            '../chrome_elf/chrome_elf.gyp:chrome_elf',
             'chrome_dll',
             'chrome_nacl_win64',
             'chrome_process_finder',
@@ -481,6 +514,7 @@
             '../breakpad/breakpad.gyp:breakpad_sender',
             '../components/components.gyp:breakpad_component',
             '../components/components.gyp:policy',
+            '../chrome_elf/chrome_elf.gyp:chrome_elf',
             '../sandbox/sandbox.gyp:sandbox',
           ],
           'sources': [
@@ -494,6 +528,7 @@
           'msvs_settings': {
             'VCLinkerTool': {
               'ImportLibrary': '$(OutDir)\\lib\\chrome_exe.lib',
+              'OutputFile': '$(OutDir)\\initial\\chrome.exe',
               'DelayLoadDLLs': [
                 'dbghelp.dll',
                 'dwmapi.dll',

chrome_elf/chrome_elf.gyp

@@ -45,6 +45,7 @@
       'type': 'executable',
       'sources': [
         'blacklist/test/blacklist_test.cc',
+        'elf_imports_unittest.cc',
         'ntdll_cache_unittest.cc',
       ],
       'include_dirs': [

chrome_elf/elf_imports_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stdint.h>
+#include <windows.h>
+
+#include <algorithm>
+#include <vector>
+
+#include "base/base_paths.h"
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/files/file_path.h"
+#include "base/files/memory_mapped_file.h"
+#include "base/path_service.h"
+#include "base/strings/string_util.h"
+#include "base/win/pe_image.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class ELFImportsTest : public testing::Test {
+ protected:
+  static bool ImportsCallback(const base::win::PEImage &image,
+                              LPCSTR module,
+                              PIMAGE_THUNK_DATA name_table,
+                              PIMAGE_THUNK_DATA iat,
+                              PVOID cookie) {
+    std::vector<std::string>* import_list =
+        reinterpret_cast<std::vector<std::string>*>(cookie);
+    import_list->push_back(module);
+    return true;
+  }
+
+  void GetImports(const base::FilePath& module_path,
+                  std::vector<std::string>* imports) {
+    ASSERT_TRUE(imports != NULL);
+
+    base::MemoryMappedFile module_mmap;
+
+    ASSERT_TRUE(module_mmap.Initialize(module_path));
+    base::win::PEImageAsData pe_image_data(
+        reinterpret_cast<HMODULE>(const_cast<uint8*>(module_mmap.data())));
+    pe_image_data.EnumImportChunks(ELFImportsTest::ImportsCallback, imports);
+  }
+};
+
+TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
+  std::vector<std::string> elf_imports;
+
+  base::FilePath dll;
+  ASSERT_TRUE(PathService::Get(base::DIR_EXE, &dll));
+  dll = dll.Append(L"chrome_elf.dll");
+  GetImports(dll, &elf_imports);
+
+  // Check that ELF has imports.
+  ASSERT_LT(0u, elf_imports.size());
+
+  std::vector<std::string>::iterator it(elf_imports.begin());
+
+  static const char* const kValidFilePatterns[] = {
+      "KERNEL32.dll",
+      "MSVC*",
+      "ADVAPI32.dll"};
+
+  // Make sure all of ELF's imports are in the valid imports list.
+  for (; it != elf_imports.end(); it++) {
+    bool match = false;
+    for (int i = 0; i < arraysize(kValidFilePatterns); ++i) {
+      if (MatchPattern(*it, kValidFilePatterns[i]))
+        match = true;
+    }
+    ASSERT_TRUE(match) << "Illegal import in chrome_elf.dll.";
+  }
+}
+
+#if defined(ARCH_CPU_64_BITS)
+#define MAYBE_ChromeExeSanityCheck DISABLED_ChromeExeSanityCheck
+#else
+#define MAYBE_ChromeExeSanityCheck ChromeExeSanityCheck
+#endif
+// Fails on 64-bit Windows, see http://crbug.com/335173.
+TEST_F(ELFImportsTest, MAYBE_ChromeExeSanityCheck) {
+  std::vector<std::string> exe_imports;
+
+  base::FilePath exe;
+  ASSERT_TRUE(PathService::Get(base::DIR_EXE, &exe));
+  exe = exe.Append(L"chrome.exe");
+  GetImports(exe, &exe_imports);
+
+  // Check that chrome.exe has imports.
+  ASSERT_LT(0u, exe_imports.size());
+
+  // Chrome.exe's first import must be ELF.
+  EXPECT_EQ("chrome_elf.dll", exe_imports[0]) <<
+      "Illegal import order in chrome.exe";
+}
+
+}  // namespace