From 30bff99e04ce6937dc2bfd84783d2623f75eef6a Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 28 Oct 2020 23:55:06 -0700 Subject: [PATCH 1/5] fix: Initialize logging for crashpad --- patches/chromium/.patches | 3 + .../crashpad-initialize-logging.patch | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 patches/chromium/crashpad-initialize-logging.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index d126dec5aca47..be126340319b2 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -149,3 +149,6 @@ cherry-pick-30261f9de11e.patch cherry-pick-88f263f401b4.patch cherry-pick-229fdaf8fc05.patch cherry-pick-1ed869ad4bb3.patch +chore_expose_v8_initialization_isolate_callbacks.patch +rename_the_v8_context_snapshot_on_arm64_macos_builds.patch +crashpad-initialize-logging.patch diff --git a/patches/chromium/crashpad-initialize-logging.patch b/patches/chromium/crashpad-initialize-logging.patch new file mode 100644 index 0000000000000..e6bcf843c6431 --- /dev/null +++ b/patches/chromium/crashpad-initialize-logging.patch @@ -0,0 +1,78 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Joshua Peraza +Date: Wed, 21 Oct 2020 11:10:25 -0700 +Subject: Initialize logging for crashpad + +Although logging to files is not yet supported by mini_chromium, it is +the default behavior for OS_WIN in chromium. This change should +cause crashpad to log via OutputDebugString() on Windows, instead of +debug.log files. Future work (crbug.com/crashpad/26) should arrange for +logs to be uploaded with reports, embedded in associated minidumps or as +file attachments. + +Bug: chromium:711159 +Change-Id: I0f9004f7de94dd29d555cc7d23c48a63da6b4bba +Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2425108 +Reviewed-by: Mark Mentovai + +diff --git a/third_party/crashpad/crashpad/handler/handler_main.cc b/third_party/crashpad/crashpad/handler/handler_main.cc +index 33649291e253a7a9bd281b892bab415ff1950b6a..a3ba9bb17e221e8330b09eae572d116bd29e4ba4 100644 +--- a/third_party/crashpad/crashpad/handler/handler_main.cc ++++ b/third_party/crashpad/crashpad/handler/handler_main.cc +@@ -504,16 +504,26 @@ class ScopedStoppable { + DISALLOW_COPY_AND_ASSIGN(ScopedStoppable); + }; + ++void InitCrashpadLogging() { ++ logging::LoggingSettings settings; ++#if defined(OS_CHROMEOS) ++ settings.logging_dest = logging::LOG_TO_FILE; ++ settings.log_file_path = "/var/log/chrome/chrome"; ++#elif defined(OS_WIN) ++ settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG; ++#else ++ settings.logging_dest = ++ logging::LOG_TO_SYSTEM_DEBUG_LOG | logging::LOG_TO_STDERR; ++#endif ++ logging::InitLogging(settings); ++} ++ + } // namespace + + int HandlerMain(int argc, + char* argv[], + const UserStreamDataSources* user_stream_sources) { +-#if defined(OS_CHROMEOS) +- if (freopen("/var/log/chrome/chrome", "a", stderr) == nullptr) { +- PLOG(ERROR) << "Failed to redirect stderr to /var/log/chrome/chrome"; +- } +-#endif ++ InitCrashpadLogging(); + + InstallCrashHandler(); + CallMetricsRecordNormalExit metrics_record_normal_exit; +diff --git a/third_party/crashpad/crashpad/test/gtest_main.cc b/third_party/crashpad/crashpad/test/gtest_main.cc +index 67cfa0d72d7eb469775201f3a9df906f27c302a9..c67b8e24bb940935d5da88428ed3058a135f5a57 100644 +--- a/third_party/crashpad/crashpad/test/gtest_main.cc ++++ b/third_party/crashpad/crashpad/test/gtest_main.cc +@@ -12,6 +12,7 @@ + // See the License for the specific language governing permissions and + // limitations under the License. + ++#include "base/logging.h" + #include "build/build_config.h" + #include "gtest/gtest.h" + #include "test/main_arguments.h" +@@ -99,6 +100,12 @@ int main(int argc, char* argv[]) { + + #endif // CRASHPAD_IS_IN_CHROMIUM + ++ // base::TestSuite initializes logging when using Chromium's test launcher. ++ logging::LoggingSettings settings; ++ settings.logging_dest = ++ logging::LOG_TO_STDERR | logging::LOG_TO_SYSTEM_DEBUG_LOG; ++ logging::InitLogging(settings); ++ + #if defined(CRASHPAD_TEST_LAUNCHER_GOOGLEMOCK) + testing::InitGoogleMock(&argc, argv); + #elif defined(CRASHPAD_TEST_LAUNCHER_GOOGLETEST) From 180d5ad5069eb521ffa51a14da13ee9d804f7f4d Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 29 Oct 2020 02:15:14 -0700 Subject: [PATCH 2/5] chore: update mini_chromium --- patches/chromium/crashpad-initialize-logging.patch | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/patches/chromium/crashpad-initialize-logging.patch b/patches/chromium/crashpad-initialize-logging.patch index e6bcf843c6431..f33d212d02eec 100644 --- a/patches/chromium/crashpad-initialize-logging.patch +++ b/patches/chromium/crashpad-initialize-logging.patch @@ -15,6 +15,19 @@ Change-Id: I0f9004f7de94dd29d555cc7d23c48a63da6b4bba Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2425108 Reviewed-by: Mark Mentovai +diff --git a/third_party/crashpad/crashpad/DEPS b/third_party/crashpad/crashpad/DEPS +index 83995e0cdea91522c272415330c57af764d23163..7e83327aac2582e81a38720086a52832de58a37a 100644 +--- a/third_party/crashpad/crashpad/DEPS ++++ b/third_party/crashpad/crashpad/DEPS +@@ -42,7 +42,7 @@ deps = { + '7bde79cc274d06451bf65ae82c012a5d3e476b5a', + 'crashpad/third_party/mini_chromium/mini_chromium': + Var('chromium_git') + '/chromium/mini_chromium@' + +- '76a9bb7475f6217eaf108789246379d3972b4e6a', ++ '5fc64bfbf1c000161445c586de45e40464ff2314', + 'crashpad/third_party/libfuzzer/src': + Var('chromium_git') + '/chromium/llvm-project/compiler-rt/lib/fuzzer.git@' + + 'fda403cf93ecb8792cb1d061564d89a6553ca020', diff --git a/third_party/crashpad/crashpad/handler/handler_main.cc b/third_party/crashpad/crashpad/handler/handler_main.cc index 33649291e253a7a9bd281b892bab415ff1950b6a..a3ba9bb17e221e8330b09eae572d116bd29e4ba4 100644 --- a/third_party/crashpad/crashpad/handler/handler_main.cc From 2291e54f96eddd8be39c9de28fa002cfefc3e4a9 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 29 Oct 2020 10:26:43 -0700 Subject: [PATCH 3/5] conditionally access CommandLine because crashpad doesn't initialize one. https://chromium-review.googlesource.com/c/chromium/src/+/2490880 --- patches/chromium/.patches | 1 - .../crashpad-initialize-logging.patch | 55 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/patches/chromium/.patches b/patches/chromium/.patches index be126340319b2..e1d1755a52084 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -150,5 +150,4 @@ cherry-pick-88f263f401b4.patch cherry-pick-229fdaf8fc05.patch cherry-pick-1ed869ad4bb3.patch chore_expose_v8_initialization_isolate_callbacks.patch -rename_the_v8_context_snapshot_on_arm64_macos_builds.patch crashpad-initialize-logging.patch diff --git a/patches/chromium/crashpad-initialize-logging.patch b/patches/chromium/crashpad-initialize-logging.patch index f33d212d02eec..61b399dc61479 100644 --- a/patches/chromium/crashpad-initialize-logging.patch +++ b/patches/chromium/crashpad-initialize-logging.patch @@ -15,6 +15,61 @@ Change-Id: I0f9004f7de94dd29d555cc7d23c48a63da6b4bba Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2425108 Reviewed-by: Mark Mentovai +diff --git a/base/logging.cc b/base/logging.cc +index b5cf2c4933d0cbb89f2f1b410c5c249a0b8647f0..698dca03914934b294457d05d89722a27cdebb56 100644 +--- a/base/logging.cc ++++ b/base/logging.cc +@@ -369,21 +369,23 @@ bool BaseInitLoggingImpl(const LoggingSettings& settings) { + g_log_format = settings.log_format; + #endif + +- base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); +- // Don't bother initializing |g_vlog_info| unless we use one of the +- // vlog switches. +- if (command_line->HasSwitch(switches::kV) || +- command_line->HasSwitch(switches::kVModule)) { +- // NOTE: If |g_vlog_info| has already been initialized, it might be in use +- // by another thread. Don't delete the old VLogInfo, just create a second +- // one. We keep track of both to avoid memory leak warnings. +- CHECK(!g_vlog_info_prev); +- g_vlog_info_prev = g_vlog_info; +- +- g_vlog_info = +- new VlogInfo(command_line->GetSwitchValueASCII(switches::kV), +- command_line->GetSwitchValueASCII(switches::kVModule), +- &g_min_log_level); ++ if (base::CommandLine::InitializedForCurrentProcess()) { ++ base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); ++ // Don't bother initializing |g_vlog_info| unless we use one of the ++ // vlog switches. ++ if (command_line->HasSwitch(switches::kV) || ++ command_line->HasSwitch(switches::kVModule)) { ++ // NOTE: If |g_vlog_info| has already been initialized, it might be in use ++ // by another thread. Don't delete the old VLogInfo, just create a second ++ // one. We keep track of both to avoid memory leak warnings. ++ CHECK(!g_vlog_info_prev); ++ g_vlog_info_prev = g_vlog_info; ++ ++ g_vlog_info = ++ new VlogInfo(command_line->GetSwitchValueASCII(switches::kV), ++ command_line->GetSwitchValueASCII(switches::kVModule), ++ &g_min_log_level); ++ } + } + + g_logging_destination = settings.logging_dest; +@@ -394,7 +396,10 @@ bool BaseInitLoggingImpl(const LoggingSettings& settings) { + config.min_severity = FX_LOG_INFO; + config.console_fd = -1; + config.log_service_channel = ZX_HANDLE_INVALID; +- std::string log_tag = command_line->GetProgram().BaseName().AsUTF8Unsafe(); ++ std::string log_tag = base::CommandLine::ForCurrentProcess() ++ ->GetProgram() ++ .BaseName() ++ .AsUTF8Unsafe(); + const char* log_tag_data = log_tag.data(); + config.tags = &log_tag_data; + config.num_tags = 1; diff --git a/third_party/crashpad/crashpad/DEPS b/third_party/crashpad/crashpad/DEPS index 83995e0cdea91522c272415330c57af764d23163..7e83327aac2582e81a38720086a52832de58a37a 100644 --- a/third_party/crashpad/crashpad/DEPS From 4db97a37ac08bcf96dd887511f2ed6d748151a8c Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 9 Nov 2020 23:17:22 -0800 Subject: [PATCH 4/5] chore: update patches --- .../crashpad-initialize-logging.patch | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/patches/chromium/crashpad-initialize-logging.patch b/patches/chromium/crashpad-initialize-logging.patch index 61b399dc61479..a4ca6223589ed 100644 --- a/patches/chromium/crashpad-initialize-logging.patch +++ b/patches/chromium/crashpad-initialize-logging.patch @@ -16,11 +16,11 @@ Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2425 Reviewed-by: Mark Mentovai diff --git a/base/logging.cc b/base/logging.cc -index b5cf2c4933d0cbb89f2f1b410c5c249a0b8647f0..698dca03914934b294457d05d89722a27cdebb56 100644 +index 608cc1122b98803b66c4b4a4eee1020ffe94e20c..6f9005ba2f21efe2f81b4fde0360d672b9f58022 100644 --- a/base/logging.cc +++ b/base/logging.cc -@@ -369,21 +369,23 @@ bool BaseInitLoggingImpl(const LoggingSettings& settings) { - g_log_format = settings.log_format; +@@ -410,21 +410,23 @@ bool BaseInitLoggingImpl(const LoggingSettings& settings) { + 0u); #endif - base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); @@ -58,7 +58,7 @@ index b5cf2c4933d0cbb89f2f1b410c5c249a0b8647f0..698dca03914934b294457d05d89722a2 } g_logging_destination = settings.logging_dest; -@@ -394,7 +396,10 @@ bool BaseInitLoggingImpl(const LoggingSettings& settings) { +@@ -435,7 +437,10 @@ bool BaseInitLoggingImpl(const LoggingSettings& settings) { config.min_severity = FX_LOG_INFO; config.console_fd = -1; config.log_service_channel = ZX_HANDLE_INVALID; @@ -71,23 +71,23 @@ index b5cf2c4933d0cbb89f2f1b410c5c249a0b8647f0..698dca03914934b294457d05d89722a2 config.tags = &log_tag_data; config.num_tags = 1; diff --git a/third_party/crashpad/crashpad/DEPS b/third_party/crashpad/crashpad/DEPS -index 83995e0cdea91522c272415330c57af764d23163..7e83327aac2582e81a38720086a52832de58a37a 100644 +index 8b958f1887be77ac54c878a9e1474fb67574950a..0cd250f0bb67888e83fcdd8cce856e0d8a8d29a5 100644 --- a/third_party/crashpad/crashpad/DEPS +++ b/third_party/crashpad/crashpad/DEPS @@ -42,7 +42,7 @@ deps = { '7bde79cc274d06451bf65ae82c012a5d3e476b5a', 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + -- '76a9bb7475f6217eaf108789246379d3972b4e6a', +- 'c426ff98e1d9e9d59777fe8b883a5c0ceeca9ca3', + '5fc64bfbf1c000161445c586de45e40464ff2314', 'crashpad/third_party/libfuzzer/src': Var('chromium_git') + '/chromium/llvm-project/compiler-rt/lib/fuzzer.git@' + 'fda403cf93ecb8792cb1d061564d89a6553ca020', diff --git a/third_party/crashpad/crashpad/handler/handler_main.cc b/third_party/crashpad/crashpad/handler/handler_main.cc -index 33649291e253a7a9bd281b892bab415ff1950b6a..a3ba9bb17e221e8330b09eae572d116bd29e4ba4 100644 +index e0a262cd1f38bb4b663b6d13b28fcc272d69bef1..6e6adc536208f5094589e885ce6a7610e994b5d3 100644 --- a/third_party/crashpad/crashpad/handler/handler_main.cc +++ b/third_party/crashpad/crashpad/handler/handler_main.cc -@@ -504,16 +504,26 @@ class ScopedStoppable { +@@ -519,16 +519,26 @@ class ScopedStoppable { DISALLOW_COPY_AND_ASSIGN(ScopedStoppable); }; @@ -120,7 +120,7 @@ index 33649291e253a7a9bd281b892bab415ff1950b6a..a3ba9bb17e221e8330b09eae572d116b InstallCrashHandler(); CallMetricsRecordNormalExit metrics_record_normal_exit; diff --git a/third_party/crashpad/crashpad/test/gtest_main.cc b/third_party/crashpad/crashpad/test/gtest_main.cc -index 67cfa0d72d7eb469775201f3a9df906f27c302a9..c67b8e24bb940935d5da88428ed3058a135f5a57 100644 +index ad3a095ea7657b60b6fc05d09fb04319dbee1b17..5b64a2f886bca5531691963059dd47e89174fb04 100644 --- a/third_party/crashpad/crashpad/test/gtest_main.cc +++ b/third_party/crashpad/crashpad/test/gtest_main.cc @@ -12,6 +12,7 @@ @@ -131,16 +131,16 @@ index 67cfa0d72d7eb469775201f3a9df906f27c302a9..c67b8e24bb940935d5da88428ed3058a #include "build/build_config.h" #include "gtest/gtest.h" #include "test/main_arguments.h" -@@ -99,6 +100,12 @@ int main(int argc, char* argv[]) { +@@ -91,6 +92,12 @@ int main(int argc, char* argv[]) { #endif // CRASHPAD_IS_IN_CHROMIUM -+ // base::TestSuite initializes logging when using Chromium's test launcher. -+ logging::LoggingSettings settings; -+ settings.logging_dest = -+ logging::LOG_TO_STDERR | logging::LOG_TO_SYSTEM_DEBUG_LOG; -+ logging::InitLogging(settings); ++// base::TestSuite initializes logging when using Chromium's test launcher. ++logging::LoggingSettings settings; ++settings.logging_dest = ++ logging::LOG_TO_STDERR | logging::LOG_TO_SYSTEM_DEBUG_LOG; ++logging::InitLogging(settings); + - #if defined(CRASHPAD_TEST_LAUNCHER_GOOGLEMOCK) + #if defined(CRASHPAD_TEST_LAUNCHER_GMOCK) testing::InitGoogleMock(&argc, argv); - #elif defined(CRASHPAD_TEST_LAUNCHER_GOOGLETEST) + #elif defined(CRASHPAD_TEST_LAUNCHER_GTEST) From 3a2fc9411bad882d9ebc407e22ce15f73cf06dfb Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 9 Nov 2020 23:20:37 -0800 Subject: [PATCH 5/5] update patch list --- patches/chromium/.patches | 1 - 1 file changed, 1 deletion(-) diff --git a/patches/chromium/.patches b/patches/chromium/.patches index e1d1755a52084..cfdce9c6d583e 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -149,5 +149,4 @@ cherry-pick-30261f9de11e.patch cherry-pick-88f263f401b4.patch cherry-pick-229fdaf8fc05.patch cherry-pick-1ed869ad4bb3.patch -chore_expose_v8_initialization_isolate_callbacks.patch crashpad-initialize-logging.patch