Update Crashpad to 2f3a8b8f72dccc7c1738eb496e46e86ee60ec4ed
03abd1bb3497 mac: Tolerate the new flavor of weird cl_kernels modules on
10.14
98ebb0060b3e Add whitespace file
e50ea6032180 Make BuildHandlerArgvStrings() return its result
3af81d7012e4 DEPS: Use gclient's native cipd instead of runhooks
c11c8833f78f Add ProcessSnapshotMinidump::ProcessID()
2418cb8fbef8 Make upload report metrics optional
0909bee2e26c linux: Fix broken tests with address sanitizer
2f3a8b8f72dc Add CrashSkippedReason::kPrepareForUploadFailed
Change-Id: Iefdaf2365615586fec6f4dcefc4f857e6e166074
Reviewed-on: https://chromium-review.googlesource.com/1148933
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577866}diff --git a/third_party/crashpad/README.chromium b/third_party/crashpad/README.chromium
index aed01e0..73d39da 100644
--- a/third_party/crashpad/README.chromium
+++ b/third_party/crashpad/README.chromium
@@ -2,7 +2,7 @@
Short Name: crashpad
URL: https://crashpad.chromium.org/
Version: unknown
-Revision: fb0f7ca8d7eb55ca51b77eb89813a3283a461b0c
+Revision: 2f3a8b8f72dccc7c1738eb496e46e86ee60ec4ed
License: Apache 2.0
License File: crashpad/LICENSE
Security Critical: yes
diff --git a/third_party/crashpad/crashpad/DEPS b/third_party/crashpad/crashpad/DEPS
index dbdc3a4..34f1ef68 100644
--- a/third_party/crashpad/crashpad/DEPS
+++ b/third_party/crashpad/crashpad/DEPS
@@ -37,6 +37,90 @@
'crashpad/third_party/zlib/zlib':
Var('chromium_git') + '/chromium/src/third_party/zlib@' +
'13dc246a58e4b72104d35f9b1809af95221ebda7',
+
+ # CIPD packages below.
+ 'crashpad/third_party/linux/clang/linux-amd64': {
+ 'packages': [
+ {
+ 'package': 'fuchsia/clang/linux-amd64',
+ 'version': 'goma',
+ },
+ ],
+ 'condition': 'checkout_linux and pull_linux_clang',
+ 'dep_type': 'cipd'
+ },
+ 'crashpad/third_party/linux/clang/mac-amd64': {
+ 'packages': [
+ {
+ 'package': 'fuchsia/clang/mac-amd64',
+ 'version': 'goma',
+ },
+ ],
+ 'condition': 'checkout_fuchsia and host_os == "mac"',
+ 'dep_type': 'cipd'
+ },
+ 'crashpad/third_party/fuchsia/clang/linux-amd64': {
+ 'packages': [
+ {
+ 'package': 'fuchsia/clang/linux-amd64',
+ 'version': 'goma',
+ },
+ ],
+ 'condition': 'checkout_fuchsia and host_os == "linux"',
+ 'dep_type': 'cipd'
+ },
+ 'crashpad/third_party/fuchsia/qemu/mac-amd64': {
+ 'packages': [
+ {
+ 'package': 'fuchsia/qemu/mac-amd64',
+ 'version': 'latest'
+ },
+ ],
+ 'condition': 'checkout_fuchsia and host_os == "mac"',
+ 'dep_type': 'cipd'
+ },
+ 'crashpad/third_party/fuchsia/qemu/linux-amd64': {
+ 'packages': [
+ {
+ 'package': 'fuchsia/qemu/linux-amd64',
+ 'version': 'latest'
+ },
+ ],
+ 'condition': 'checkout_fuchsia and host_os == "linux"',
+ 'dep_type': 'cipd'
+ },
+ 'crashpad/third_party/fuchsia/sdk/linux-amd64': {
+ # The SDK is keyed to the host system because it contains build tools.
+ # Currently, linux-amd64 is the only SDK published (see
+ # https://chrome-infra-packages.appspot.com/#/?path=fuchsia/sdk).
+ # As long as this is the case, use that SDK package
+ # even on other build hosts.
+ # The sysroot (containing headers and libraries) and other components are
+ # related to the target and should be functional with an appropriate
+ # toolchain that runs on the build host (fuchsia_clang, above).
+ 'packages': [
+ {
+ 'package': 'fuchsia/sdk/linux-amd64',
+ 'version': 'latest'
+ },
+ ],
+ 'condition': 'checkout_fuchsia',
+ 'dep_type': 'cipd'
+ },
+ 'crashpad/third_party/win/toolchain': {
+ # This package is only updated when the solution in .gclient includes an
+ # entry like:
+ # "custom_vars": { "pull_win_toolchain": True }
+ # This is because the contained bits are not redistributable.
+ 'packages': [
+ {
+ 'package': 'chrome_internal/third_party/sdk/windows',
+ 'version': 'uploaded:2018-06-13'
+ },
+ ],
+ 'condition': 'checkout_win and pull_win_toolchain',
+ 'dep_type': 'cipd'
+ },
}
hooks = [
@@ -119,28 +203,6 @@
],
},
{
- # This uses “cipd install” so that mac-amd64 and linux-amd64 can coexist
- # peacefully. “cipd ensure” would remove the macOS package when running on a
- # Linux build host and vice-versa. https://crbug.com/789364. This package is
- # only updated when the solution in .gclient includes an entry like:
- # "custom_vars": { "pull_linux_clang": True }
- # The ref used is "goma". This is like "latest", but is considered a more
- # stable latest by the Fuchsia toolchain team.
- 'name': 'clang_linux',
- 'pattern': '.',
- 'condition': 'checkout_linux and pull_linux_clang',
- 'action': [
- 'cipd',
- 'install',
- # sic, using Fuchsia team's generic build of clang for linux-amd64 to
- # build for linux-amd64 target too.
- 'fuchsia/clang/linux-amd64',
- 'goma',
- '-root', 'crashpad/third_party/linux/clang/linux-amd64',
- '-log-level', 'info',
- ],
- },
- {
# If using a local clang ("pull_linux_clang" above), also pull down a
# sysroot.
'name': 'sysroot_linux',
@@ -151,105 +213,6 @@
],
},
{
- # Same rationale for using "install" rather than "ensure" as for first clang
- # package. https://crbug.com/789364.
- # Same rationale for using "goma" instead of "latest" as clang_linux above.
- 'name': 'fuchsia_clang_mac',
- 'pattern': '.',
- 'condition': 'checkout_fuchsia and host_os == "mac"',
- 'action': [
- 'cipd',
- 'install',
- 'fuchsia/clang/mac-amd64',
- 'goma',
- '-root', 'crashpad/third_party/fuchsia/clang/mac-amd64',
- '-log-level', 'info',
- ],
- },
- {
- # Same rationale for using "install" rather than "ensure" as for first clang
- # package. https://crbug.com/789364.
- # Same rationale for using "goma" instead of "latest" as clang_linux above.
- 'name': 'fuchsia_clang_linux',
- 'pattern': '.',
- 'condition': 'checkout_fuchsia and host_os == "linux"',
- 'action': [
- 'cipd',
- 'install',
- 'fuchsia/clang/linux-amd64',
- 'goma',
- '-root', 'crashpad/third_party/fuchsia/clang/linux-amd64',
- '-log-level', 'info',
- ],
- },
- {
- # Same rationale for using "install" rather than "ensure" as for clang
- # packages. https://crbug.com/789364.
- 'name': 'fuchsia_qemu_mac',
- 'pattern': '.',
- 'condition': 'checkout_fuchsia and host_os == "mac"',
- 'action': [
- 'cipd',
- 'install',
- 'fuchsia/qemu/mac-amd64',
- 'latest',
- '-root', 'crashpad/third_party/fuchsia/qemu/mac-amd64',
- '-log-level', 'info',
- ],
- },
- {
- # Same rationale for using "install" rather than "ensure" as for clang
- # packages. https://crbug.com/789364.
- 'name': 'fuchsia_qemu_linux',
- 'pattern': '.',
- 'condition': 'checkout_fuchsia and host_os == "linux"',
- 'action': [
- 'cipd',
- 'install',
- 'fuchsia/qemu/linux-amd64',
- 'latest',
- '-root', 'crashpad/third_party/fuchsia/qemu/linux-amd64',
- '-log-level', 'info',
- ],
- },
- {
- # The SDK is keyed to the host system because it contains build tools.
- # Currently, linux-amd64 is the only SDK published (see
- # https://chrome-infra-packages.appspot.com/#/?path=fuchsia/sdk). As long as
- # this is the case, use that SDK package even on other build hosts. The
- # sysroot (containing headers and libraries) and other components are
- # related to the target and should be functional with an appropriate
- # toolchain that runs on the build host (fuchsia_clang, above).
- 'name': 'fuchsia_sdk',
- 'pattern': '.',
- 'condition': 'checkout_fuchsia',
- 'action': [
- 'cipd',
- 'install',
- 'fuchsia/sdk/linux-amd64',
- 'latest',
- '-root', 'crashpad/third_party/fuchsia/sdk/linux-amd64',
- '-log-level', 'info',
- ],
- },
- {
- 'name': 'toolchain_win',
- 'pattern': '.',
- # This package is only updated when the solution in .gclient includes an
- # entry like:
- # "custom_vars": { "pull_win_toolchain": True }
- # This is because the contained bits are not redistributable.
- 'condition': 'checkout_win and pull_win_toolchain',
- 'action': [
- 'cipd',
- 'install',
- 'chrome_internal/third_party/sdk/windows',
- 'uploaded:2018-06-13',
- '-root', 'crashpad/third_party/win/toolchain',
- '-log-level', 'info',
- ],
- },
- {
'name': 'gyp',
'pattern': '\.gypi?$',
'action': ['python', 'crashpad/build/gyp_crashpad.py'],
diff --git a/third_party/crashpad/crashpad/client/client_argv_handling.cc b/third_party/crashpad/crashpad/client/client_argv_handling.cc
index 9b813b1..6933aac 100644
--- a/third_party/crashpad/crashpad/client/client_argv_handling.cc
+++ b/third_party/crashpad/crashpad/client/client_argv_handling.cc
@@ -27,38 +27,38 @@
} // namespace
-void BuildHandlerArgvStrings(
+std::vector<std::string> BuildHandlerArgvStrings(
const base::FilePath& handler,
const base::FilePath& database,
const base::FilePath& metrics_dir,
const std::string& url,
const std::map<std::string, std::string>& annotations,
- const std::vector<std::string>& arguments,
- std::vector<std::string>* argv_strings) {
- argv_strings->clear();
+ const std::vector<std::string>& arguments) {
+ std::vector<std::string> argv_strings(1, handler.value());
- argv_strings->push_back(handler.value());
for (const auto& argument : arguments) {
- argv_strings->push_back(argument);
+ argv_strings.push_back(argument);
}
if (!database.empty()) {
- argv_strings->push_back(FormatArgumentString("database", database.value()));
+ argv_strings.push_back(FormatArgumentString("database", database.value()));
}
if (!metrics_dir.empty()) {
- argv_strings->push_back(
+ argv_strings.push_back(
FormatArgumentString("metrics-dir", metrics_dir.value()));
}
if (!url.empty()) {
- argv_strings->push_back(FormatArgumentString("url", url));
+ argv_strings.push_back(FormatArgumentString("url", url));
}
for (const auto& kv : annotations) {
- argv_strings->push_back(
+ argv_strings.push_back(
FormatArgumentString("annotation", kv.first + '=' + kv.second));
}
+
+ return argv_strings;
}
void ConvertArgvStrings(const std::vector<std::string>& argv_strings,
diff --git a/third_party/crashpad/crashpad/client/client_argv_handling.h b/third_party/crashpad/crashpad/client/client_argv_handling.h
index 0ef3a15..d380b1a 100644
--- a/third_party/crashpad/crashpad/client/client_argv_handling.h
+++ b/third_party/crashpad/crashpad/client/client_argv_handling.h
@@ -28,16 +28,14 @@
//!
//! See StartHandlerAtCrash() for documentation on the input arguments.
//!
-//! \param[out] A argv_strings vector of arguments suitable for starting the
-//! handler with.
-void BuildHandlerArgvStrings(
+//! \return A vector of arguments suitable for starting the handler with.
+std::vector<std::string> BuildHandlerArgvStrings(
const base::FilePath& handler,
const base::FilePath& database,
const base::FilePath& metrics_dir,
const std::string& url,
const std::map<std::string, std::string>& annotations,
- const std::vector<std::string>& arguments,
- std::vector<std::string>* argv_strings);
+ const std::vector<std::string>& arguments);
//! \brief Flattens a string vector into a const char* vector suitable for use
//! in an exec() call.
diff --git a/third_party/crashpad/crashpad/client/crash_report_database.cc b/third_party/crashpad/crashpad/client/crash_report_database.cc
index aa365be..d300a8f9 100644
--- a/third_party/crashpad/crashpad/client/crash_report_database.cc
+++ b/third_party/crashpad/crashpad/client/crash_report_database.cc
@@ -69,7 +69,8 @@
reader_(std::make_unique<FileReader>()),
database_(nullptr),
attachment_readers_(),
- attachment_map_() {}
+ attachment_map_(),
+ report_metrics_(false) {}
CrashReportDatabase::UploadReport::~UploadReport() {
if (database_) {
diff --git a/third_party/crashpad/crashpad/client/crash_report_database.h b/third_party/crashpad/crashpad/client/crash_report_database.h
index 9ceeddc33..d115c74 100644
--- a/third_party/crashpad/crashpad/client/crash_report_database.h
+++ b/third_party/crashpad/crashpad/client/crash_report_database.h
@@ -178,6 +178,7 @@
CrashReportDatabase* database_;
std::vector<std::unique_ptr<FileReader>> attachment_readers_;
std::map<std::string, FileReader*> attachment_map_;
+ bool report_metrics_;
DISALLOW_COPY_AND_ASSIGN(UploadReport);
};
@@ -326,11 +327,16 @@
//! \param[in] uuid The unique identifier for the crash report record.
//! \param[out] report A crash report record for the report to be uploaded.
//! Only valid if this returns #kNoError.
+ //! \param[in] report_metrics If `false`, metrics will not be recorded for
+ //! this upload attempt when RecordUploadComplete() is called or \a report
+ //! is destroyed. Metadata for the upload attempt will still be recorded
+ //! in the database.
//!
//! \return The operation status code.
virtual OperationStatus GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) = 0;
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics = true) = 0;
//! \brief Records a successful upload for a report and updates the last
//! upload attempt time as returned by
diff --git a/third_party/crashpad/crashpad/client/crash_report_database_generic.cc b/third_party/crashpad/crashpad/client/crash_report_database_generic.cc
index 6dc8e9e..8e93237 100644
--- a/third_party/crashpad/crashpad/client/crash_report_database_generic.cc
+++ b/third_party/crashpad/crashpad/client/crash_report_database_generic.cc
@@ -180,7 +180,8 @@
OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
OperationStatus GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) override;
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics) override;
OperationStatus SkipReportUpload(const UUID& uuid,
Metrics::CrashSkippedReason reason) override;
OperationStatus DeleteReport(const UUID& uuid) override;
@@ -455,7 +456,8 @@
OperationStatus CrashReportDatabaseGeneric::GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) {
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
auto upload_report = std::make_unique<LockfileUploadReport>();
@@ -470,6 +472,7 @@
if (!upload_report->Initialize(path, this)) {
return kFileSystemError;
}
+ upload_report->report_metrics_ = report_metrics;
report->reset(upload_report.release());
return kNoError;
@@ -609,7 +612,9 @@
const std::string& id) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
- Metrics::CrashUploadAttempted(successful);
+ if (report->report_metrics_) {
+ Metrics::CrashUploadAttempted(successful);
+ }
time_t now = time(nullptr);
report->id = id;
diff --git a/third_party/crashpad/crashpad/client/crash_report_database_mac.mm b/third_party/crashpad/crashpad/client/crash_report_database_mac.mm
index 79b876d..3106dc2c 100644
--- a/third_party/crashpad/crashpad/client/crash_report_database_mac.mm
+++ b/third_party/crashpad/crashpad/client/crash_report_database_mac.mm
@@ -141,7 +141,8 @@
OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
OperationStatus GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) override;
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics) override;
OperationStatus SkipReportUpload(const UUID& uuid,
Metrics::CrashSkippedReason reason) override;
OperationStatus DeleteReport(const UUID& uuid) override;
@@ -422,7 +423,8 @@
CrashReportDatabase::OperationStatus
CrashReportDatabaseMac::GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) {
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
auto upload_report = std::make_unique<UploadReportMac>();
@@ -444,6 +446,7 @@
upload_report->database_ = this;
upload_report->lock_fd.reset(lock.release());
+ upload_report->report_metrics_ = report_metrics;
report->reset(upload_report.release());
return kNoError;
}
@@ -454,7 +457,9 @@
const std::string& id) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
- Metrics::CrashUploadAttempted(successful);
+ if (report->report_metrics_) {
+ Metrics::CrashUploadAttempted(successful);
+ }
DCHECK(report);
DCHECK(successful || id.empty());
diff --git a/third_party/crashpad/crashpad/client/crash_report_database_win.cc b/third_party/crashpad/crashpad/client/crash_report_database_win.cc
index e19e605..8967770 100644
--- a/third_party/crashpad/crashpad/client/crash_report_database_win.cc
+++ b/third_party/crashpad/crashpad/client/crash_report_database_win.cc
@@ -594,7 +594,8 @@
OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
OperationStatus GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) override;
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics) override;
OperationStatus SkipReportUpload(const UUID& uuid,
Metrics::CrashSkippedReason reason) override;
OperationStatus DeleteReport(const UUID& uuid) override;
@@ -731,7 +732,8 @@
OperationStatus CrashReportDatabaseWin::GetReportForUploading(
const UUID& uuid,
- std::unique_ptr<const UploadReport>* report) {
+ std::unique_ptr<const UploadReport>* report,
+ bool report_metrics) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
std::unique_ptr<Metadata> metadata(AcquireMetadata());
@@ -749,6 +751,7 @@
if (!upload_report->Initialize(upload_report->file_path, this)) {
return kFileSystemError;
}
+ upload_report->report_metrics_ = report_metrics;
report->reset(upload_report.release());
}
@@ -761,7 +764,9 @@
const std::string& id) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
- Metrics::CrashUploadAttempted(successful);
+ if (report->report_metrics_) {
+ Metrics::CrashUploadAttempted(successful);
+ }
std::unique_ptr<Metadata> metadata(AcquireMetadata());
if (!metadata)
diff --git a/third_party/crashpad/crashpad/client/crashpad_client_fuchsia.cc b/third_party/crashpad/crashpad/client/crashpad_client_fuchsia.cc
index 2678109..0d1b65b 100644
--- a/third_party/crashpad/crashpad/client/crashpad_client_fuchsia.cc
+++ b/third_party/crashpad/crashpad/client/crashpad_client_fuchsia.cc
@@ -58,14 +58,8 @@
return false;
}
- std::vector<std::string> argv_strings;
- BuildHandlerArgvStrings(handler,
- database,
- metrics_dir,
- url,
- annotations,
- arguments,
- &argv_strings);
+ std::vector<std::string> argv_strings = BuildHandlerArgvStrings(
+ handler, database, metrics_dir, url, annotations, arguments);
std::vector<const char*> argv;
ConvertArgvStrings(argv_strings, &argv);
diff --git a/third_party/crashpad/crashpad/client/crashpad_client_linux.cc b/third_party/crashpad/crashpad/client/crashpad_client_linux.cc
index e7fa162c..54fe268 100644
--- a/third_party/crashpad/crashpad/client/crashpad_client_linux.cc
+++ b/third_party/crashpad/crashpad/client/crashpad_client_linux.cc
@@ -175,9 +175,8 @@
const std::string& url,
const std::map<std::string, std::string>& annotations,
const std::vector<std::string>& arguments) {
- std::vector<std::string> argv;
- BuildHandlerArgvStrings(
- handler, database, metrics_dir, url, annotations, arguments, &argv);
+ std::vector<std::string> argv = BuildHandlerArgvStrings(
+ handler, database, metrics_dir, url, annotations, arguments);
auto signal_handler = LaunchAtCrashHandler::Get();
if (signal_handler->Initialize(&argv)) {
@@ -197,9 +196,8 @@
const std::map<std::string, std::string>& annotations,
const std::vector<std::string>& arguments,
int socket) {
- std::vector<std::string> argv;
- BuildHandlerArgvStrings(
- handler, database, metrics_dir, url, annotations, arguments, &argv);
+ std::vector<std::string> argv = BuildHandlerArgvStrings(
+ handler, database, metrics_dir, url, annotations, arguments);
argv.push_back(FormatArgumentInt("initial-client", socket));
diff --git a/third_party/crashpad/crashpad/client/prune_crash_reports_test.cc b/third_party/crashpad/crashpad/client/prune_crash_reports_test.cc
index e1be2f4..27a2836 100644
--- a/third_party/crashpad/crashpad/client/prune_crash_reports_test.cc
+++ b/third_party/crashpad/crashpad/client/prune_crash_reports_test.cc
@@ -41,9 +41,10 @@
MOCK_METHOD2(LookUpCrashReport, OperationStatus(const UUID&, Report*));
MOCK_METHOD1(GetPendingReports, OperationStatus(std::vector<Report>*));
MOCK_METHOD1(GetCompletedReports, OperationStatus(std::vector<Report>*));
- MOCK_METHOD2(GetReportForUploading,
+ MOCK_METHOD3(GetReportForUploading,
OperationStatus(const UUID&,
- std::unique_ptr<const UploadReport>*));
+ std::unique_ptr<const UploadReport>*,
+ bool report_metrics));
MOCK_METHOD3(RecordUploadAttempt,
OperationStatus(UploadReport*, bool, const std::string&));
MOCK_METHOD2(SkipReportUpload,
diff --git a/third_party/crashpad/crashpad/client/simple_string_dictionary_test.cc b/third_party/crashpad/crashpad/client/simple_string_dictionary_test.cc
index 5fdeb5b..7af51a2 100644
--- a/third_party/crashpad/crashpad/client/simple_string_dictionary_test.cc
+++ b/third_party/crashpad/crashpad/client/simple_string_dictionary_test.cc
@@ -115,8 +115,7 @@
// Add a bunch of values to the dictionary, remove some entries in the middle,
// and then add more.
TEST(SimpleStringDictionary, Iterator) {
- SimpleStringDictionary* dict = new SimpleStringDictionary;
- ASSERT_TRUE(dict);
+ SimpleStringDictionary dict;
char key[SimpleStringDictionary::key_size];
char value[SimpleStringDictionary::value_size];
@@ -135,32 +134,32 @@
for (int i = 0; i < kPartitionIndex; ++i) {
sprintf(key, "key%d", i);
sprintf(value, "value%d", i);
- dict->SetKeyValue(key, value);
+ dict.SetKeyValue(key, value);
}
expected_dictionary_size = kPartitionIndex;
// set a couple of the keys twice (with the same value) - should be nop
- dict->SetKeyValue("key2", "value2");
- dict->SetKeyValue("key4", "value4");
- dict->SetKeyValue("key15", "value15");
+ dict.SetKeyValue("key2", "value2");
+ dict.SetKeyValue("key4", "value4");
+ dict.SetKeyValue("key15", "value15");
// Remove some random elements in the middle
- dict->RemoveKey("key7");
- dict->RemoveKey("key18");
- dict->RemoveKey("key23");
- dict->RemoveKey("key31");
+ dict.RemoveKey("key7");
+ dict.RemoveKey("key18");
+ dict.RemoveKey("key23");
+ dict.RemoveKey("key31");
expected_dictionary_size -= 4; // we just removed four key/value pairs
// Set some more key/value pairs like key59/value59, key60/value60, ...
for (int i = kPartitionIndex; i < kDictionaryCapacity; ++i) {
sprintf(key, "key%d", i);
sprintf(value, "value%d", i);
- dict->SetKeyValue(key, value);
+ dict.SetKeyValue(key, value);
}
expected_dictionary_size += kDictionaryCapacity - kPartitionIndex;
// Now create an iterator on the dictionary
- SimpleStringDictionary::Iterator iter(*dict);
+ SimpleStringDictionary::Iterator iter(dict);
// We then verify that it iterates through exactly the number of key/value
// pairs we expect, and that they match one-for-one with what we would expect.
diff --git a/third_party/crashpad/crashpad/handler/crash_report_upload_thread.cc b/third_party/crashpad/crashpad/handler/crash_report_upload_thread.cc
index 4783ecb..205d860 100644
--- a/third_party/crashpad/crashpad/handler/crash_report_upload_thread.cc
+++ b/third_party/crashpad/crashpad/handler/crash_report_upload_thread.cc
@@ -227,6 +227,10 @@
database_->RecordUploadComplete(std::move(upload_report), response_body);
break;
case UploadResult::kPermanentFailure:
+ upload_report.reset();
+ database_->SkipReportUpload(
+ report.uuid, Metrics::CrashSkippedReason::kPrepareForUploadFailed);
+ break;
case UploadResult::kRetry:
upload_report.reset();
diff --git a/third_party/crashpad/crashpad/snapshot/linux/process_reader_linux_test.cc b/third_party/crashpad/crashpad/snapshot/linux/process_reader_linux_test.cc
index 6c0b98a..940114c6 100644
--- a/third_party/crashpad/crashpad/snapshot/linux/process_reader_linux_test.cc
+++ b/third_party/crashpad/crashpad/snapshot/linux/process_reader_linux_test.cc
@@ -40,6 +40,7 @@
#include "test/multiprocess.h"
#include "util/file/file_io.h"
#include "util/linux/direct_ptrace_connection.h"
+#include "util/misc/address_sanitizer.h"
#include "util/misc/from_pointer_cast.h"
#include "util/synchronization/semaphore.h"
@@ -264,12 +265,17 @@
iterator->second.tls);
ASSERT_TRUE(memory_map.FindMapping(thread.stack_region_address));
- EXPECT_LE(thread.stack_region_address, iterator->second.stack_address);
-
ASSERT_TRUE(memory_map.FindMapping(thread.stack_region_address +
thread.stack_region_size - 1));
+
+#if !defined(ADDRESS_SANITIZER)
+ // AddressSanitizer causes stack variables to be stored separately from the
+ // call stack.
+ EXPECT_LE(thread.stack_region_address, iterator->second.stack_address);
EXPECT_GE(thread.stack_region_address + thread.stack_region_size,
iterator->second.stack_address);
+#endif // !defined(ADDRESS_SANITIZER)
+
if (iterator->second.max_stack_size) {
EXPECT_LT(thread.stack_region_size, iterator->second.max_stack_size);
}
diff --git a/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.cc b/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.cc
index 1be829d..478b37d 100644
--- a/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.cc
+++ b/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.cc
@@ -15,6 +15,7 @@
#include "snapshot/mac/mach_o_image_segment_reader.h"
#include <mach-o/loader.h>
+#include <string.h>
#include <utility>
@@ -35,6 +36,37 @@
} // namespace
+bool IsMalformedCLKernelsModule(uint32_t mach_o_file_type,
+ const std::string& module_name,
+ bool* has_timestamp) {
+ if (mach_o_file_type != MH_BUNDLE) {
+ return false;
+ }
+
+ if (module_name == "cl_kernels") {
+ if (MacOSXMinorVersion() >= 10) {
+ if (has_timestamp) {
+ *has_timestamp = false;
+ }
+ return true;
+ }
+ return false;
+ }
+
+ static const char kCvmsObjectPathPrefix[] =
+ "/private/var/db/CVMS/cvmsCodeSignObj";
+ if (module_name.compare(
+ 0, strlen(kCvmsObjectPathPrefix), kCvmsObjectPathPrefix) == 0 &&
+ MacOSXMinorVersion() >= 14) {
+ if (has_timestamp) {
+ *has_timestamp = true;
+ }
+ return true;
+ }
+
+ return false;
+}
+
MachOImageSegmentReader::MachOImageSegmentReader()
: segment_command_(),
sections_(),
@@ -121,21 +153,13 @@
load_command_info.c_str());
// cl_kernels modules (for OpenCL) aren’t ld output, and they’re formatted
- // incorrectly on OS X 10.10 and later. They have a single __TEXT segment,
- // but one of the sections within it claims to belong to the __LD segment.
- // This mismatch shouldn’t happen. This errant section also has the
- // S_ATTR_DEBUG flag set, which shouldn’t happen unless all of the other
- // sections in the segment also have this bit set (they don’t). These odd
- // sections are reminiscent of unwind information stored in MH_OBJECT
- // images, although cl_kernels images claim to be MH_BUNDLE. Because at
- // least one cl_kernels module will commonly be found in a process, and
- // sometimes more will be, tolerate this quirk.
+ // incorrectly on OS X 10.10 and later. Because at least one cl_kernels
+ // module will commonly be found in a process, and sometimes more will be,
+ // tolerate this quirk.
//
// https://openradar.appspot.com/20239912
if (section_segment_name != segment_name &&
- !(file_type == MH_BUNDLE &&
- module_name == "cl_kernels" &&
- MacOSXMinorVersion() >= 10 &&
+ !(IsMalformedCLKernelsModule(file_type, module_name, nullptr) &&
segment_name == SEG_TEXT &&
section_segment_name == "__LD" &&
section_name == "__compact_unwind" &&
diff --git a/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.h b/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.h
index bdd5771..1e72785 100644
--- a/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.h
+++ b/third_party/crashpad/crashpad/snapshot/mac/mach_o_image_segment_reader.h
@@ -29,6 +29,41 @@
namespace crashpad {
+//! \brief Determines whether a module appears to be a malformed OpenCL
+//! `cl_kernels` module based on its name and Mach-O file type.
+//!
+//! `cl_kernels` modules require special handling because they’re malformed on
+//! OS X 10.10 and later. A `cl_kernels` module always has Mach-O type
+//! `MH_BUNDLE` and is named `"cl_kernels"` until macOS 10.14, and
+//! `"/private/var/db/CVMS/cvmsCodeSignObj"` plus 16 random characters on macOS
+//! 10.14.
+//!
+//! Malformed `cl_kernels` modules have a single `__TEXT` segment, but one of
+//! the sections within it claims to belong to the `__LD` segment. This mismatch
+//! shouldn’t happen. This errant section also has the `S_ATTR_DEBUG` flag set,
+//! which shouldn’t happen unless all of the other sections in the segment also
+//! have this bit set (they don’t). These odd sections are reminiscent of unwind
+//! information stored in `MH_OBJECT` images, although `cl_kernels` images claim
+//! to be `MH_BUNDLE`.
+//!
+//! This function is exposed for testing purposes only.
+//!
+//! \param[in] mach_o_file_type The Mach-O type of the module being examined.
+//! \param[in] module_name The pathname that `dyld` reported having loaded the
+//! module from.
+//! \param[out] has_timestamp Optional, may be `nullptr`. If provided, and the
+//! module is a maformed `cl_kernels` module, this will be set to `true` if
+//! the module was loaded from the filesystem (as is the case when loaded
+//! from the CVMS directory) and is expected to have a timestamp, and
+//! `false` otherwise. Note that even when loaded from the filesystem, these
+//! modules are unlinked from the filesystem after loading.
+//!
+//! \return `true` if the module appears to be a malformed `cl_kernels` module
+//! based on the provided information, `false` otherwise.
+bool IsMalformedCLKernelsModule(uint32_t mach_o_file_type,
+ const std::string& module_name,
+ bool* has_timestamp);
+
//! \brief A reader for `LC_SEGMENT` or `LC_SEGMENT_64` load commands in Mach-O
//! images mapped into another process.
//!
diff --git a/third_party/crashpad/crashpad/snapshot/mac/process_reader_mac_test.cc b/third_party/crashpad/crashpad/snapshot/mac/process_reader_mac_test.cc
index faec147..59884a6 100644
--- a/third_party/crashpad/crashpad/snapshot/mac/process_reader_mac_test.cc
+++ b/third_party/crashpad/crashpad/snapshot/mac/process_reader_mac_test.cc
@@ -32,6 +32,7 @@
#include "build/build_config.h"
#include "gtest/gtest.h"
#include "snapshot/mac/mach_o_image_reader.h"
+#include "snapshot/mac/mach_o_image_segment_reader.h"
#include "test/errors.h"
#include "test/mac/dyld.h"
#include "test/mac/mach_errors.h"
@@ -663,14 +664,20 @@
modules[index].reader->Address(),
FromPointerCast<mach_vm_address_t>(_dyld_get_image_header(index)));
+ bool expect_timestamp;
if (index == 0) {
// dyld didn’t load the main executable, so it couldn’t record its
// timestamp, and it is reported as 0.
EXPECT_EQ(modules[index].timestamp, 0);
- } else if (modules[index].reader->FileType() == MH_BUNDLE &&
- modules[index].name == "cl_kernels") {
- // cl_kernels doesn’t exist as a file.
- EXPECT_EQ(modules[index].timestamp, 0);
+ } else if (IsMalformedCLKernelsModule(modules[index].reader->FileType(),
+ modules[index].name,
+ &expect_timestamp)) {
+ // cl_kernels doesn’t exist as a file, but may still have a timestamp.
+ if (!expect_timestamp) {
+ EXPECT_EQ(modules[index].timestamp, 0);
+ } else {
+ EXPECT_NE(modules[index].timestamp, 0);
+ }
found_cl_kernels = true;
} else {
// Hope that the module didn’t change on disk.
@@ -747,14 +754,20 @@
ASSERT_TRUE(modules[index].reader);
EXPECT_EQ(modules[index].reader->Address(), expect_address);
+ bool expect_timestamp;
if (index == 0 || index == modules.size() - 1) {
// dyld didn’t load the main executable or itself, so it couldn’t record
// these timestamps, and they are reported as 0.
EXPECT_EQ(modules[index].timestamp, 0);
- } else if (modules[index].reader->FileType() == MH_BUNDLE &&
- modules[index].name == "cl_kernels") {
- // cl_kernels doesn’t exist as a file.
- EXPECT_EQ(modules[index].timestamp, 0);
+ } else if (IsMalformedCLKernelsModule(modules[index].reader->FileType(),
+ modules[index].name,
+ &expect_timestamp)) {
+ // cl_kernels doesn’t exist as a file, but may still have a timestamp.
+ if (!expect_timestamp) {
+ EXPECT_EQ(modules[index].timestamp, 0);
+ } else {
+ EXPECT_NE(modules[index].timestamp, 0);
+ }
found_cl_kernels = true;
} else {
// Hope that the module didn’t change on disk.
diff --git a/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.cc b/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.cc
index 90bfac54..652e0e4 100644
--- a/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.cc
+++ b/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.cc
@@ -31,6 +31,7 @@
crashpad_info_(),
annotations_simple_map_(),
file_reader_(nullptr),
+ process_id_(static_cast<pid_t>(-1)),
initialized_() {
}
@@ -83,19 +84,19 @@
stream_map_[stream_type] = &directory.Location;
}
- INITIALIZATION_STATE_SET_VALID(initialized_);
-
- if (!InitializeCrashpadInfo()) {
+ if (!InitializeCrashpadInfo() ||
+ !InitializeMiscInfo() ||
+ !InitializeModules()) {
return false;
}
- return InitializeModules();
+ INITIALIZATION_STATE_SET_VALID(initialized_);
+ return true;
}
pid_t ProcessSnapshotMinidump::ProcessID() const {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
- NOTREACHED(); // https://crashpad.chromium.org/bug/10
- return 0;
+ return process_id_;
}
pid_t ProcessSnapshotMinidump::ParentProcessID() const {
@@ -234,6 +235,45 @@
&annotations_simple_map_);
}
+bool ProcessSnapshotMinidump::InitializeMiscInfo() {
+ const auto& stream_it = stream_map_.find(kMinidumpStreamTypeMiscInfo);
+ if (stream_it == stream_map_.end()) {
+ return true;
+ }
+
+ if (!file_reader_->SeekSet(stream_it->second->Rva)) {
+ return false;
+ }
+
+ const size_t size = stream_it->second->DataSize;
+ if (size != sizeof(MINIDUMP_MISC_INFO_5) &&
+ size != sizeof(MINIDUMP_MISC_INFO_4) &&
+ size != sizeof(MINIDUMP_MISC_INFO_3) &&
+ size != sizeof(MINIDUMP_MISC_INFO_2) &&
+ size != sizeof(MINIDUMP_MISC_INFO)) {
+ LOG(ERROR) << "misc_info size mismatch";
+ return false;
+ }
+
+ MINIDUMP_MISC_INFO_5 info;
+ if (!file_reader_->ReadExactly(&info, size)) {
+ return false;
+ }
+
+ switch (stream_it->second->DataSize) {
+ case sizeof(MINIDUMP_MISC_INFO_5):
+ case sizeof(MINIDUMP_MISC_INFO_4):
+ case sizeof(MINIDUMP_MISC_INFO_3):
+ case sizeof(MINIDUMP_MISC_INFO_2):
+ case sizeof(MINIDUMP_MISC_INFO):
+ // TODO(jperaza): Save the remaining misc info.
+ // https://crashpad.chromium.org/bug/10
+ process_id_ = info.ProcessId;
+ }
+
+ return true;
+}
+
bool ProcessSnapshotMinidump::InitializeModules() {
const auto& stream_it = stream_map_.find(kMinidumpStreamTypeModuleList);
if (stream_it == stream_map_.end()) {
diff --git a/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.h b/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.h
index 211805e..9ba5d9c 100644
--- a/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.h
+++ b/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump.h
@@ -92,6 +92,10 @@
std::map<uint32_t, MINIDUMP_LOCATION_DESCRIPTOR>*
module_crashpad_info_links);
+ // Initializes data carried in a MINIDUMP_MISC_INFO structure on behalf of
+ // Initialize().
+ bool InitializeMiscInfo();
+
MINIDUMP_HEADER header_;
std::vector<MINIDUMP_DIRECTORY> stream_directory_;
std::map<MinidumpStreamType, const MINIDUMP_LOCATION_DESCRIPTOR*> stream_map_;
@@ -100,6 +104,7 @@
MinidumpCrashpadInfo crashpad_info_;
std::map<std::string, std::string> annotations_simple_map_;
FileReaderInterface* file_reader_; // weak
+ pid_t process_id_;
InitializationStateDcheck initialized_;
DISALLOW_COPY_AND_ASSIGN(ProcessSnapshotMinidump);
diff --git a/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump_test.cc b/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump_test.cc
index 82e14a7a..ed17939 100644
--- a/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump_test.cc
+++ b/third_party/crashpad/crashpad/snapshot/minidump/process_snapshot_minidump_test.cc
@@ -420,6 +420,38 @@
EXPECT_EQ(annotation_objects, annotations_4);
}
+TEST(ProcessSnapshotMinidump, ProcessID) {
+ StringFile string_file;
+
+ MINIDUMP_HEADER header = {};
+ ASSERT_TRUE(string_file.Write(&header, sizeof(header)));
+
+ static const pid_t kTestProcessId = 42;
+ MINIDUMP_MISC_INFO misc_info = {};
+ misc_info.SizeOfInfo = sizeof(misc_info);
+ misc_info.Flags1 = MINIDUMP_MISC1_PROCESS_ID;
+ misc_info.ProcessId = kTestProcessId;
+
+ MINIDUMP_DIRECTORY misc_directory = {};
+ misc_directory.StreamType = kMinidumpStreamTypeMiscInfo;
+ misc_directory.Location.DataSize = sizeof(misc_info);
+ misc_directory.Location.Rva = static_cast<RVA>(string_file.SeekGet());
+ ASSERT_TRUE(string_file.Write(&misc_info, sizeof(misc_info)));
+
+ header.StreamDirectoryRva = static_cast<RVA>(string_file.SeekGet());
+ ASSERT_TRUE(string_file.Write(&misc_directory, sizeof(misc_directory)));
+
+ header.Signature = MINIDUMP_SIGNATURE;
+ header.Version = MINIDUMP_VERSION;
+ header.NumberOfStreams = 1;
+ ASSERT_TRUE(string_file.SeekSet(0));
+ ASSERT_TRUE(string_file.Write(&header, sizeof(header)));
+
+ ProcessSnapshotMinidump process_snapshot;
+ ASSERT_TRUE(process_snapshot.Initialize(&string_file));
+ EXPECT_EQ(process_snapshot.ProcessID(), kTestProcessId);
+}
+
} // namespace
} // namespace test
} // namespace crashpad
diff --git a/third_party/crashpad/crashpad/snapshot/sanitized/process_snapshot_sanitized_test.cc b/third_party/crashpad/crashpad/snapshot/sanitized/process_snapshot_sanitized_test.cc
index 485ef87..cb413e3b 100644
--- a/third_party/crashpad/crashpad/snapshot/sanitized/process_snapshot_sanitized_test.cc
+++ b/third_party/crashpad/crashpad/snapshot/sanitized/process_snapshot_sanitized_test.cc
@@ -19,6 +19,7 @@
#include "gtest/gtest.h"
#include "test/multiprocess_exec.h"
#include "util/file/file_io.h"
+#include "util/misc/address_sanitizer.h"
#include "util/numeric/safe_assignment.h"
#if defined(OS_LINUX) || defined(OS_ANDROID)
@@ -162,6 +163,23 @@
// MemorySnapshot::Delegate
bool MemorySnapshotDelegateRead(void* data, size_t size) override {
+#if defined(ADDRESS_SANITIZER) && (defined(OS_LINUX) || defined(OS_ANDROID))
+ // AddressSanitizer causes stack variables to be stored separately from the
+ // call stack.
+ auto addr_not_in_stack_range =
+ [](VMAddress addr, VMAddress stack_addr, VMSize stack_size) {
+ return addr < stack_addr || addr >= stack_addr + stack_size;
+ };
+ EXPECT_PRED3(addr_not_in_stack_range,
+ addrs_.code_pointer_address,
+ stack_->Address(),
+ size);
+ EXPECT_PRED3(addr_not_in_stack_range,
+ addrs_.string_address,
+ stack_->Address(),
+ size);
+ return true;
+#else
size_t pointer_offset;
if (!AssignIfInRange(&pointer_offset,
addrs_.code_pointer_address - stack_->Address())) {
@@ -192,6 +210,7 @@
EXPECT_STREQ(string, kSensitiveStackData);
}
return true;
+#endif // ADDRESS_SANITIZER && (OS_LINUX || OS_ANDROID)
}
private:
diff --git a/third_party/crashpad/crashpad/util/misc/capture_context_test.cc b/third_party/crashpad/crashpad/util/misc/capture_context_test.cc
index 1117789d..7f3890f 100644
--- a/third_party/crashpad/crashpad/util/misc/capture_context_test.cc
+++ b/third_party/crashpad/crashpad/util/misc/capture_context_test.cc
@@ -60,12 +60,16 @@
kReferencePC);
#endif // !defined(ADDRESS_SANITIZER)
- // Declare sp and context_2 here because all local variables need to be
- // declared before computing the stack pointer reference value, so that the
- // reference value can be the lowest value possible.
- uintptr_t sp;
+ const uintptr_t sp = StackPointerFromContext(context_1);
+
+ // Declare context_2 here because all local variables need to be declared
+ // before computing the stack pointer reference value, so that the reference
+ // value can be the lowest value possible.
NativeCPUContext context_2;
+// AddressSanitizer on Linux causes stack variables to be stored separately from
+// the call stack.
+#if !defined(ADDRESS_SANITIZER) || (!defined(OS_LINUX) && !defined(OS_ANDROID))
// The stack pointer reference value is the lowest address of a local variable
// in this function. The captured program counter will be slightly less than
// or equal to the reference stack pointer.
@@ -74,11 +78,11 @@
reinterpret_cast<uintptr_t>(&context_2)),
std::min(reinterpret_cast<uintptr_t>(&pc),
reinterpret_cast<uintptr_t>(&sp)));
- sp = StackPointerFromContext(context_1);
EXPECT_PRED2([](uintptr_t actual,
uintptr_t reference) { return reference - actual < 768u; },
sp,
kReferenceSP);
+#endif // !ADDRESS_SANITIZER || (!OS_LINUX && !OS_ANDROID)
// Capture the context again, expecting that the stack pointer stays the same
// and the program counter increases. Strictly speaking, there’s no guarantee
diff --git a/third_party/crashpad/crashpad/util/misc/metrics.h b/third_party/crashpad/crashpad/util/misc/metrics.h
index 3e9337a..8046497 100644
--- a/third_party/crashpad/crashpad/util/misc/metrics.h
+++ b/third_party/crashpad/crashpad/util/misc/metrics.h
@@ -77,6 +77,10 @@
//! server.
kUploadFailed = 4,
+ //! \brief There was an error between accessing the report from the database
+ //! and uploading it to the crash server.
+ kPrepareForUploadFailed = 5,
+
//! \brief The number of values in this enumeration; not a valid value.
kMaxValue
};
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 57ab8ab2..0249d5d 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -8226,6 +8226,7 @@
<int value="2" label="kUnexpectedTime"/>
<int value="3" label="kDatabaseError"/>
<int value="4" label="kUploadFailed"/>
+ <int value="5" label="kPrepareForUploadFailed"/>
</enum>
<enum name="CrashpadWinExceptionCodes">
