Revert of [chrome_elf] Big cleanup and removing dependencies that rec…

…ently crept in. Part 1. (patchset chromium#9 id:300001 of https://codereview.chromium.org/2183263003/ ) Reason for revert: Win7 Tests (dbg)(1) started failing. https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/51720 Excerpt from step 103, chrome_elf_unittests on Windows-7-SP1 chrome_elf_unittests on Windows-7-SP1 : [ RUN ] HookTest.IATHook Assertion failed: false, file c:\b\c\b\win\src\chrome_elf\hook_util\hook_util.cc, line 302 [43/49] HookTest.IATHook (TIMED OUT) Original issue's description: > [chrome_elf] Big cleanup and removing dependencies that recently crept in. PART 1. > > - Moving all crash related APIs into one place (and out of chrome_elf_main). > - Started to clean up external dependencies - starting with hook_utils. > - Removed dependency on base/win/iat_patch_function. > > BUG=631771 > > Committed: https://crrev.com/d2767f8b41a8f7f70bddbd0783a8ef27a9370e58 > Cr-Commit-Position: refs/heads/master@{#411982} TBR=robertshield@chromium.org,pennymac@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=631771 Review-Url: https://codereview.chromium.org/2242323002 Cr-Commit-Position: refs/heads/master@{#412026}
marcosholgado · Aug 15, 2016 · 4e989fa · 4e989fa
1 parent b6f12c5
commit 4e989fa
Show file tree

Hide file tree

Showing 21 changed files with 359 additions and 862 deletions.
diff --git a/chrome_elf/BUILD.gn b/chrome_elf/BUILD.gn
@@ -33,6 +33,9 @@ windows_manifest("chrome_elf_manifest") {
 # in the world.
 shared_library("chrome_elf") {
   sources = [
+    "//chrome/app/chrome_crash_reporter_client_win.cc",
+    "//chrome/app/chrome_crash_reporter_client_win.h",
+    "//chrome/common/chrome_result_codes.h",
     "chrome_elf.def",
     "chrome_elf_main.cc",
     "chrome_elf_main.h",
@@ -41,24 +44,31 @@ shared_library("chrome_elf") {
     ":blacklist",
     ":chrome_elf_manifest",
     ":chrome_elf_resources",
+    ":chrome_elf_security",
     ":constants",
-    ":crash",
     ":hook_util",
-    ":security",
+    "//base",
     "//build/config/sanitizers:deps",
     "//chrome/install_static:install_static_util",
     "//chrome_elf/nt_registry:nt_registry",
+    "//components/crash/content/app",
+    "//components/crash/core/common",
+    "//content/public/common:result_codes",
+    "//third_party/crashpad/crashpad/client:client",
   ]
   configs += [ "//build/config/win:windowed" ]
   configs -= [ "//build/config/win:console" ]
-
-  # Delay loads in this list will prevent user32.dll
-  # from loading too early.
   ldflags = [
-    "/DELAYLOAD:advapi32.dll",
+    "/NODEFAULTLIB:user32.lib",
     "/DELAYLOAD:dbghelp.dll",
+    "/DELAYLOAD:ole32.dll",
+    "/DELAYLOAD:psapi.dll",
     "/DELAYLOAD:rpcrt4.dll",
+    "/DELAYLOAD:shell32.dll",
+    "/DELAYLOAD:user32.dll",
+    "/DELAYLOAD:winhttp.dll",
     "/DELAYLOAD:winmm.dll",
+    "/DELAYLOAD:ws2_32.dll",
   ]
   if (current_cpu == "x86") {
     # Don"t set an x64 base address (to avoid breaking HE-ASLR).
@@ -70,7 +80,7 @@ shared_library("chrome_elf") {
 ## source sets
 ##------------------------------------------------------------------------------
 
-source_set("security") {
+source_set("chrome_elf_security") {
   sources = [
     "chrome_elf_security.cc",
     "chrome_elf_security.h",
@@ -119,48 +129,30 @@ static_library("blacklist") {
     "blacklist/blacklist.h",
     "blacklist/blacklist_interceptions.cc",
     "blacklist/blacklist_interceptions.h",
+    "blacklist/crashpad_helper.cc",
+    "blacklist/crashpad_helper.h",
   ]
   public_deps = [
     "//sandbox",
   ]
   deps = [
     ":constants",
-    ":crash",
     ":hook_util",
-    "//base:base_static",  # pe_image
     "//chrome/install_static:install_static_util",
     "//chrome_elf/nt_registry:nt_registry",
-  ]
-}
 
-static_library("crash") {
-  sources = [
-    "../chrome/app/chrome_crash_reporter_client_win.cc",
-    "../chrome/app/chrome_crash_reporter_client_win.h",
-    "../chrome/common/chrome_result_codes.h",
-    "crash/crash_helper.cc",
-    "crash/crash_helper.h",
-  ]
-  deps = [
-    ":hook_util",
-    "//base:base",  # This needs to go.  DEP of app, crash_keys, client.
-    "//base:base_static",  # pe_image
-    "//chrome/install_static:install_static_util",
-    "//components/crash/content/app:app",
-    "//components/crash/core/common",  # crash_keys
-    "//content/public/common:result_codes",
-    "//third_party/crashpad/crashpad/client:client",  # DumpWithoutCrash
+    # Still uses base/win/pe_image.h
+    "//base",
+    "//third_party/crashpad/crashpad/client:client",
   ]
 }
 
 static_library("hook_util") {
   sources = [
-    "../base/macros.h",
-    "hook_util/hook_util.cc",
-    "hook_util/hook_util.h",
+    "hook_util/thunk_getter.cc",
+    "hook_util/thunk_getter.h",
   ]
   deps = [
-    "//base:base_static",  # pe_image
     "//sandbox",
   ]
 }
@@ -175,18 +167,15 @@ test("chrome_elf_unittests") {
     "blacklist/test/blacklist_test.cc",
     "chrome_elf_util_unittest.cc",
     "elf_imports_unittest.cc",
-    "hook_util/test/hook_util_test.cc",
     "run_all_unittests.cc",
   ]
   include_dirs = [ "$target_gen_dir" ]
   deps = [
     ":blacklist",
     ":blacklist_test_main_dll",
+    ":chrome_elf_security",
     ":constants",
-    ":crash",
     ":hook_util",
-    ":hook_util_test_dll",
-    ":security",
     "//base",
     "//base/test:test_support",
     "//chrome",
@@ -210,15 +199,17 @@ test("chrome_elf_unittests") {
     ":blacklist_test_dll_3",
     ":chrome_elf",
   ]
-
-  # Don't want the test-specific dependencies to affect ChromeElfLoadSanityTest.
-  # In particular, a few system DLLs cause user32 to be loaded, which is bad.
   ldflags = [
-    "/DELAYLOAD:advapi32.dll",
+    "/DELAYLOAD:dbghelp.dll",
     "/DELAYLOAD:ole32.dll",
+    "/DELAYLOAD:psapi.dll",
+    "/DELAYLOAD:rpcrt4.dll",
     "/DELAYLOAD:shell32.dll",
+    "/DELAYLOAD:shlwapi.dll",
     "/DELAYLOAD:user32.dll",
+    "/DELAYLOAD:winhttp.dll",
     "/DELAYLOAD:winmm.dll",
+    "/DELAYLOAD:ws2_32.dll",
   ]
 }
 
@@ -234,6 +225,18 @@ shared_library("blacklist_test_main_dll") {
     "//chrome/install_static:install_static_util",
     "//chrome_elf/nt_registry:nt_registry",
   ]
+  ldflags = [
+    "/NODEFAULTLIB:user32.lib",
+    "/DELAYLOAD:dbghelp.dll",
+    "/DELAYLOAD:ole32.dll",
+    "/DELAYLOAD:psapi.dll",
+    "/DELAYLOAD:rpcrt4.dll",
+    "/DELAYLOAD:shell32.dll",
+    "/DELAYLOAD:user32.dll",
+    "/DELAYLOAD:winhttp.dll",
+    "/DELAYLOAD:winmm.dll",
+    "/DELAYLOAD:ws2_32.dll",
+  ]
 }
 
 loadable_module("blacklist_test_dll_1") {
@@ -269,13 +272,3 @@ loadable_module("blacklist_test_dll_3") {
     "//build/config/sanitizers:deps",
   ]
 }
-
-shared_library("hook_util_test_dll") {
-  sources = [
-    "hook_util/test/hook_util_test_dll.cc",
-    "hook_util/test/hook_util_test_dll.h",
-  ]
-  deps = [
-    "//build/config/sanitizers:deps",
-  ]
-}
diff --git a/chrome_elf/blacklist.gypi b/chrome_elf/blacklist.gypi
@@ -15,13 +15,16 @@
         'blacklist/blacklist.h',
         'blacklist/blacklist_interceptions.cc',
         'blacklist/blacklist_interceptions.h',
+        'blacklist/crashpad_helper.cc',
+        'blacklist/crashpad_helper.h',
       ],
       'dependencies': [
         '../base/base.gyp:base',
         '../chrome/chrome.gyp:install_static_util',
         '../chrome_elf/chrome_elf.gyp:chrome_elf_constants',
         '../chrome_elf/chrome_elf.gyp:chrome_elf_hook_util',
         '../chrome_elf/nt_registry/nt_registry.gyp:chrome_elf_nt_registry',
+        '../components/components.gyp:crash_component',
         '../sandbox/sandbox.gyp:sandbox',
       ],
     },
@@ -35,7 +38,6 @@
       'dependencies': [
         '../base/base.gyp:base',
         '../chrome/chrome.gyp:install_static_util',
-        '../chrome_elf/chrome_elf.gyp:chrome_elf_crash',
         '../chrome_elf/nt_registry/nt_registry.gyp:chrome_elf_nt_registry',
         'blacklist',
       ],

diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
@@ -12,7 +12,7 @@
 #include "chrome/install_static/install_util.h"
 #include "chrome_elf/blacklist/blacklist_interceptions.h"
 #include "chrome_elf/chrome_elf_constants.h"
-#include "chrome_elf/hook_util/hook_util.h"
+#include "chrome_elf/hook_util/thunk_getter.h"
 #include "chrome_elf/nt_registry/nt_registry.h"
 #include "sandbox/win/src/interception_internal.h"
 #include "sandbox/win/src/internal_types.h"
@@ -291,7 +291,7 @@ bool Initialize(bool force) {
   const bool kRelaxed = false;
 
   // Create a thunk via the appropriate ServiceResolver instance.
-  sandbox::ServiceResolverThunk* thunk = elf_hook::HookSystemService(kRelaxed);
+  sandbox::ServiceResolverThunk* thunk = GetThunk(kRelaxed);
 
   // Don't try blacklisting on unsupported OS versions.
   if (!thunk)

diff --git a/chrome_elf/blacklist/blacklist_interceptions.cc b/chrome_elf/blacklist/blacklist_interceptions.cc
@@ -18,7 +18,7 @@
 // base_static (see base/base.gyp) are allowed here.
 #include "base/win/pe_image.h"
 #include "chrome_elf/blacklist/blacklist.h"
-#include "chrome_elf/crash/crash_helper.h"
+#include "chrome_elf/blacklist/crashpad_helper.h"
 #include "sandbox/win/src/internal_types.h"
 #include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/sandbox_nt_util.h"
@@ -262,7 +262,7 @@ BlNtMapViewOfSection(NtMapViewOfSectionFunction orig_MapViewOfSection,
                                    base, zero_bits, commit_size, offset,
                                    view_size, inherit, allocation_type,
                                    protect);
-  } __except (elf_crash::GenerateCrashDump(GetExceptionInformation())) {
+  } __except(GenerateCrashDump(GetExceptionInformation())) {
   }
 
   return ret;

diff --git a/chrome_elf/blacklist/crashpad_helper.cc b/chrome_elf/blacklist/crashpad_helper.cc
@@ -0,0 +1,13 @@
+// Copyright 2016 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 "chrome_elf/blacklist/crashpad_helper.h"
+
+#include "third_party/crashpad/crashpad/client/crashpad_client.h"
+
+int GenerateCrashDump(EXCEPTION_POINTERS* exception_pointers) {
+  crashpad::CrashpadClient::DumpWithoutCrash(
+      *(exception_pointers->ContextRecord));
+  return EXCEPTION_CONTINUE_SEARCH;
+}
diff --git a/chrome_elf/blacklist/crashpad_helper.h b/chrome_elf/blacklist/crashpad_helper.h
@@ -0,0 +1,15 @@
+// Copyright 2016 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.
+
+#ifndef CHROME_ELF_BLACKLIST_CRASHPAD_HELPER_H_
+#define CHROME_ELF_BLACKLIST_CRASHPAD_HELPER_H_
+
+#include <windows.h>
+
+// Exception handler for exceptions in chrome_elf which need to be passed on to
+// the next handler in the chain. Examples include exceptions in DllMain,
+// blacklist interception code, etc.
+int GenerateCrashDump(EXCEPTION_POINTERS* exception_pointers);
+
+#endif  // CHROME_ELF_BLACKLIST_CRASHPAD_HELPER_H_
diff --git a/chrome_elf/chrome_elf.gyp b/chrome_elf/chrome_elf.gyp
@@ -40,16 +40,20 @@
         'chrome_elf.def',
         'chrome_elf_main.cc',
         'chrome_elf_main.h',
+        '../chrome/app/chrome_crash_reporter_client_win.cc',
+        '../chrome/app/chrome_crash_reporter_client_win.h',
         '<(SHARED_INTERMEDIATE_DIR)/chrome_elf/chrome_elf_version.rc',
       ],
       'dependencies': [
         '../chrome/chrome.gyp:install_static_util',
         'blacklist',
-        'chrome_elf_crash',
         'chrome_elf_hook_util',
         'chrome_elf_resources',
         'chrome_elf_security',
         'nt_registry/nt_registry.gyp:chrome_elf_nt_registry',
+        '../chrome/chrome.gyp:install_static_util',
+        '../components/components.gyp:crash_component',
+        '../components/components.gyp:crash_core_common',
       ],
       'msvs_settings': {
         'VCLinkerTool': {
@@ -84,42 +88,15 @@
         'chrome_elf_constants.h',
       ],
     },
-    {
-      'target_name': 'chrome_elf_crash',
-      'type': 'static_library',
-      'include_dirs': [
-        '..',
-      ],
-      'sources': [
-        '../chrome/app/chrome_crash_reporter_client_win.cc',
-        '../chrome/app/chrome_crash_reporter_client_win.h',
-        '../chrome/common/chrome_result_codes.h',
-        'crash/crash_helper.cc',
-        'crash/crash_helper.h',
-      ],
-      'dependencies': [
-        '../base/base.gyp:base', # This needs to go.
-        '../base/base.gyp:base_static', # pe_image
-        '../chrome/chrome.gyp:install_static_util',
-        '../components/components.gyp:crash_component',
-        '../components/components.gyp:crash_core_common', #crash_keys
-        'chrome_elf_hook_util',
-      ],
-    },
     {
       'target_name': 'chrome_elf_hook_util',
       'type': 'static_library',
       'include_dirs': [
         '..',
       ],
       'sources': [
-        '../base/macros.h',
-        'hook_util/hook_util.cc',
-        'hook_util/hook_util.h',
-      ],
-      'dependencies': [
-        '../base/base.gyp:base_static', # pe_image
-        '../sandbox/sandbox.gyp:sandbox',
+        'hook_util/thunk_getter.cc',
+        'hook_util/thunk_getter.h',
       ],
     },
     {
@@ -166,7 +143,6 @@
         'blacklist_test_dll_2',
         'blacklist_test_dll_3',
         'blacklist_test_main_dll',
-        'chrome_elf_crash',
         'chrome_elf_hook_util',
         'chrome_elf_security',
         'nt_registry/nt_registry.gyp:chrome_elf_nt_registry',