From 79e2dd843e1f86cf35f398dc578485f0c2410823 Mon Sep 17 00:00:00 2001 From: Robert Sesek Date: Thu, 2 Nov 2017 11:55:44 -0400 Subject: [PATCH] Include string annotation objects when uploading crash reports. This extracts string annotation objects from the minidumps and includes them as form POST key-value pairs. This change also starts building a crashpad_handler_test binary on Mac. Bug: crashpad:192 Change-Id: I68cbf6fda6f1e57c1e621d5e3de8717cfaea65bf Reviewed-on: https://chromium-review.googlesource.com/749793 Commit-Queue: Robert Sesek Reviewed-by: Mark Mentovai --- build/run_tests.py | 5 +- handler/crash_report_upload_thread.cc | 69 ++----------- handler/handler.gyp | 2 + handler/handler_test.gyp | 56 ++++++----- handler/minidump_to_upload_parameters.cc | 86 ++++++++++++++++ handler/minidump_to_upload_parameters.h | 61 ++++++++++++ handler/minidump_to_upload_parameters_test.cc | 99 +++++++++++++++++++ 7 files changed, 289 insertions(+), 89 deletions(-) create mode 100644 handler/minidump_to_upload_parameters.cc create mode 100644 handler/minidump_to_upload_parameters.h create mode 100644 handler/minidump_to_upload_parameters_test.cc diff --git a/build/run_tests.py b/build/run_tests.py index 3eae77a..10bd052 100755 --- a/build/run_tests.py +++ b/build/run_tests.py @@ -46,16 +46,13 @@ def main(args): tests = [ 'crashpad_client_test', + 'crashpad_handler_test', 'crashpad_minidump_test', 'crashpad_snapshot_test', 'crashpad_test_test', 'crashpad_util_test', ] - if sys.platform == 'win32': - tests.append('crashpad_handler_test') - tests = sorted(tests) - for test in tests: print '-' * 80 print test diff --git a/handler/crash_report_upload_thread.cc b/handler/crash_report_upload_thread.cc index 7038108..7505524 100644 --- a/handler/crash_report_upload_thread.cc +++ b/handler/crash_report_upload_thread.cc @@ -27,6 +27,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "client/settings.h" +#include "handler/minidump_to_upload_parameters.h" #include "snapshot/minidump/process_snapshot_minidump.h" #include "snapshot/module_snapshot.h" #include "util/file/file_reader.h" @@ -46,68 +47,6 @@ namespace crashpad { namespace { -void InsertOrReplaceMapEntry(std::map* map, - const std::string& key, - const std::string& value) { - std::string old_value; - if (!MapInsertOrReplace(map, key, value, &old_value)) { - LOG(WARNING) << "duplicate key " << key << ", discarding value " - << old_value; - } -} - -// Given a minidump file readable by |minidump_file_reader|, returns a map of -// key-value pairs to use as HTTP form parameters for upload to a Breakpad -// server. The map is built by combining the process simple annotations map with -// each module’s simple annotations map. In the case of duplicate keys, the map -// will retain the first value found for any key, and will log a warning about -// discarded values. Each module’s annotations vector is also examined and built -// into a single string value, with distinct elements separated by newlines, and -// stored at the key named “list_annotations”, which supersedes any other key -// found by that name. The client ID stored in the minidump is converted to -// a string and stored at the key named “guid”, which supersedes any other key -// found by that name. -// -// In the event of an error reading the minidump file, a message will be logged. -std::map BreakpadHTTPFormParametersFromMinidump( - FileReaderInterface* minidump_file_reader) { - ProcessSnapshotMinidump minidump_process_snapshot; - if (!minidump_process_snapshot.Initialize(minidump_file_reader)) { - return std::map(); - } - - std::map parameters = - minidump_process_snapshot.AnnotationsSimpleMap(); - - std::string list_annotations; - for (const ModuleSnapshot* module : minidump_process_snapshot.Modules()) { - for (const auto& kv : module->AnnotationsSimpleMap()) { - if (!parameters.insert(kv).second) { - LOG(WARNING) << "duplicate key " << kv.first << ", discarding value " - << kv.second; - } - } - - for (std::string annotation : module->AnnotationsVector()) { - list_annotations.append(annotation); - list_annotations.append("\n"); - } - } - - if (!list_annotations.empty()) { - // Remove the final newline character. - list_annotations.resize(list_annotations.size() - 1); - - InsertOrReplaceMapEntry(¶meters, "list_annotations", list_annotations); - } - - UUID client_id; - minidump_process_snapshot.ClientID(&client_id); - InsertOrReplaceMapEntry(¶meters, "guid", client_id.ToString()); - - return parameters; -} - // Calls CrashReportDatabase::RecordUploadAttempt() with |successful| set to // false upon destruction unless disarmed by calling Fire() or Disarm(). Fire() // triggers an immediate call. Armed upon construction. @@ -359,7 +298,11 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport( // minidump file. This may result in its being uploaded with few or no // parameters, but as long as there’s a dump file, the server can decide what // to do with it. - parameters = BreakpadHTTPFormParametersFromMinidump(&minidump_file_reader); + ProcessSnapshotMinidump minidump_process_snapshot; + if (minidump_process_snapshot.Initialize(&minidump_file_reader)) { + parameters = + BreakpadHTTPFormParametersFromMinidump(&minidump_process_snapshot); + } if (!minidump_file_reader.SeekSet(start_offset)) { return UploadResult::kPermanentFailure; diff --git a/handler/handler.gyp b/handler/handler.gyp index aefd4de..ff455de 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -45,6 +45,8 @@ 'mac/exception_handler_server.h', 'mac/file_limit_annotation.cc', 'mac/file_limit_annotation.h', + 'minidump_to_upload_parameters.cc', + 'minidump_to_upload_parameters.h', 'prune_crash_reports_thread.cc', 'prune_crash_reports_thread.h', 'user_stream_data_source.cc', diff --git a/handler/handler_test.gyp b/handler/handler_test.gyp index 588305f..4712c05 100644 --- a/handler/handler_test.gyp +++ b/handler/handler_test.gyp @@ -17,6 +17,40 @@ '../build/crashpad.gypi', ], 'targets': [ + { + 'target_name': 'crashpad_handler_test', + 'type': 'executable', + 'dependencies': [ + 'crashpad_handler_test_extended_handler', + 'handler.gyp:crashpad_handler_lib', + '../client/client.gyp:crashpad_client', + '../compat/compat.gyp:crashpad_compat', + '../snapshot/snapshot.gyp:crashpad_snapshot', + '../snapshot/snapshot_test.gyp:crashpad_snapshot_test_lib', + '../test/test.gyp:crashpad_gtest_main', + '../test/test.gyp:crashpad_test', + '../third_party/gtest/gtest.gyp:gtest', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../util/util.gyp:crashpad_util', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'crashpad_handler_test.cc', + 'minidump_to_upload_parameters_test.cc', + ], + 'conditions': [ + ['OS!="win"', { + 'dependencies!': [ + 'crashpad_handler_test_extended_handler', + ], + 'sources!': [ + 'crashpad_handler_test.cc', + ], + }], + ], + }, { 'target_name': 'crashpad_handler_test_extended_handler', 'type': 'executable', @@ -52,28 +86,6 @@ 'win/crash_other_program.cc', ], }, - { - # The handler is only tested on Windows for now. - 'target_name': 'crashpad_handler_test', - 'type': 'executable', - 'dependencies': [ - 'crashpad_handler_test_extended_handler', - 'handler.gyp:crashpad_handler_lib', - '../client/client.gyp:crashpad_client', - '../compat/compat.gyp:crashpad_compat', - '../test/test.gyp:crashpad_gtest_main', - '../test/test.gyp:crashpad_test', - '../third_party/gtest/gtest.gyp:gtest', - '../third_party/mini_chromium/mini_chromium.gyp:base', - '../util/util.gyp:crashpad_util', - ], - 'include_dirs': [ - '..', - ], - 'sources': [ - 'crashpad_handler_test.cc', - ], - }, { 'target_name': 'crashy_program', 'type': 'executable', diff --git a/handler/minidump_to_upload_parameters.cc b/handler/minidump_to_upload_parameters.cc new file mode 100644 index 0000000..03c6fe1 --- /dev/null +++ b/handler/minidump_to_upload_parameters.cc @@ -0,0 +1,86 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "handler/minidump_to_upload_parameters.h" + +#include "base/logging.h" +#include "client/annotation.h" +#include "snapshot/module_snapshot.h" +#include "util/stdlib/map_insert.h" + +namespace crashpad { + +namespace { + +void InsertOrReplaceMapEntry(std::map* map, + const std::string& key, + const std::string& value) { + std::string old_value; + if (!MapInsertOrReplace(map, key, value, &old_value)) { + LOG(WARNING) << "duplicate key " << key << ", discarding value " + << old_value; + } +} + +} // namespace + +std::map BreakpadHTTPFormParametersFromMinidump( + const ProcessSnapshot* process_snapshot) { + std::map parameters = + process_snapshot->AnnotationsSimpleMap(); + + std::string list_annotations; + for (const ModuleSnapshot* module : process_snapshot->Modules()) { + for (const auto& kv : module->AnnotationsSimpleMap()) { + if (!parameters.insert(kv).second) { + LOG(WARNING) << "duplicate key " << kv.first << ", discarding value " + << kv.second; + } + } + + for (std::string annotation : module->AnnotationsVector()) { + list_annotations.append(annotation); + list_annotations.append("\n"); + } + + for (const AnnotationSnapshot& annotation : module->AnnotationObjects()) { + if (annotation.type != static_cast(Annotation::Type::kString)) { + continue; + } + + std::string value(reinterpret_cast(annotation.value.data()), + annotation.value.size()); + std::pair entry(annotation.name, value); + if (!parameters.insert(entry).second) { + LOG(WARNING) << "duplicate annotation name " << annotation.name + << ", discarding value " << value; + } + } + } + + if (!list_annotations.empty()) { + // Remove the final newline character. + list_annotations.resize(list_annotations.size() - 1); + + InsertOrReplaceMapEntry(¶meters, "list_annotations", list_annotations); + } + + UUID client_id; + process_snapshot->ClientID(&client_id); + InsertOrReplaceMapEntry(¶meters, "guid", client_id.ToString()); + + return parameters; +} + +} // namespace crashpad diff --git a/handler/minidump_to_upload_parameters.h b/handler/minidump_to_upload_parameters.h new file mode 100644 index 0000000..41056f7 --- /dev/null +++ b/handler/minidump_to_upload_parameters.h @@ -0,0 +1,61 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef HANDLER_MINIDUMP_TO_UPLOAD_PARAMETERS_H_ +#define HANDLER_MINIDUMP_TO_UPLOAD_PARAMETERS_H_ + +#include +#include + +#include "snapshot/process_snapshot.h" + +namespace crashpad { + +//! \brief Given a ProcessSnapshot, returns a map of key-value pairs to use as +//! HTTP form parameters for upload to a Breakpad crash report colleciton +//! server. +//! +//! The map is built by combining the process simple annotations map with +//! each module’s simple annotations map and annotation objects. +//! +//! In the case of duplicate simple map keys or annotation names, the map will +//! retain the first value found for any key, and will log a warning about +//! discarded values. The precedence rules for annotation names are: the two +//! reserved keys discussed below, process simple annotations, module simple +//! annotations, and module annotation objects. +//! +//! For annotation objects, only ones of that are Annotation::Type::kString are +//! included. +//! +//! Each module’s annotations vector is also examined and built into a single +//! string value, with distinct elements separated by newlines, and stored at +//! the key named “list_annotations”, which supersedes any other key found by +//! that name. +//! +//! The client ID stored in the minidump is converted to a string and stored at +//! the key named “guid”, which supersedes any other key found by that name. +//! +//! In the event of an error reading the minidump file, a message will be +//! logged. +//! +//! \param[in] process_snapshot The process snapshot from which annotations +//! will be extracted. +//! +//! \returns A string map of the annotations. +std::map BreakpadHTTPFormParametersFromMinidump( + const ProcessSnapshot* process_snapshot); + +} // namespace crashpad + +#endif // HANDLER_MINIDUMP_TO_UPLOAD_PARAMETERS_H_ diff --git a/handler/minidump_to_upload_parameters_test.cc b/handler/minidump_to_upload_parameters_test.cc new file mode 100644 index 0000000..0c3a0e7 --- /dev/null +++ b/handler/minidump_to_upload_parameters_test.cc @@ -0,0 +1,99 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "handler/minidump_to_upload_parameters.h" + +#include "gtest/gtest.h" +#include "snapshot/test/test_module_snapshot.h" +#include "snapshot/test/test_process_snapshot.h" +#include "util/misc/uuid.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(MinidumpToUploadParameters, PrecedenceRules) { + const std::string guid = "00112233-4455-6677-8899-aabbccddeeff"; + UUID uuid; + ASSERT_TRUE(uuid.InitializeFromString(guid)); + + TestProcessSnapshot process_snapshot; + process_snapshot.SetClientID(uuid); + process_snapshot.SetAnnotationsSimpleMap({ + {"process-1", "abcdefg"}, + {"list_annotations", "BAD: process_annotations"}, + {"guid", "BAD: process_annotations"}, + {"first", "process"}, + }); + + auto module_snapshot_0 = std::make_unique(); + module_snapshot_0->SetAnnotationsVector( + {"list-module-0-1", "list-module-0-2"}); + module_snapshot_0->SetAnnotationsSimpleMap({ + {"module-0-1", "goat"}, + {"module-0-2", "doge"}, + {"list_annotations", "BAD: module 0"}, + {"guid", "BAD: module 0"}, + {"first", "BAD: module 0"}, + {"second", "module 0"}, + }); + module_snapshot_0->SetAnnotationObjects({ + {"module-0-3", 1, {'s', 't', 'a', 'r'}}, + {"module-0-4", 0xFFFA, {0x42}}, + {"guid", 1, {'B', 'A', 'D', '*', '0', '-', '0'}}, + {"list_annotations", 1, {'B', 'A', 'D', '*', '0', '-', '1'}}, + {"first", 1, {'B', 'A', 'D', '*', '0', '-', '2'}}, + }); + process_snapshot.AddModule(std::move(module_snapshot_0)); + + auto module_snapshot_1 = std::make_unique(); + module_snapshot_1->SetAnnotationsVector( + {"list-module-1-1", "list-module-1-2"}); + module_snapshot_1->SetAnnotationsSimpleMap({ + {"module-1-1", "bear"}, + {"list_annotations", "BAD: module 1"}, + {"guid", "BAD: module 1"}, + {"first", "BAD: module 1"}, + {"second", "BAD: module 1"}, + }); + module_snapshot_1->SetAnnotationObjects({ + {"module-1-3", 0xBEEF, {'a', 'b', 'c'}}, + {"module-1-4", 1, {'m', 'o', 'o', 'n'}}, + {"guid", 1, {'B', 'A', 'D', '*', '1', '-', '0'}}, + {"list_annotations", 1, {'B', 'A', 'D', '*', '1', '-', '1'}}, + {"second", 1, {'B', 'A', 'D', '*', '1', '-', '2'}}, + }); + process_snapshot.AddModule(std::move(module_snapshot_1)); + + auto upload_parameters = + BreakpadHTTPFormParametersFromMinidump(&process_snapshot); + + EXPECT_EQ(upload_parameters.size(), 10u); + EXPECT_EQ(upload_parameters["process-1"], "abcdefg"); + EXPECT_EQ(upload_parameters["first"], "process"); + EXPECT_EQ(upload_parameters["module-0-1"], "goat"); + EXPECT_EQ(upload_parameters["module-0-2"], "doge"); + EXPECT_EQ(upload_parameters["module-0-3"], "star"); + EXPECT_EQ(upload_parameters["second"], "module 0"); + EXPECT_EQ(upload_parameters["module-1-1"], "bear"); + EXPECT_EQ(upload_parameters["module-1-4"], "moon"); + EXPECT_EQ(upload_parameters["list_annotations"], + "list-module-0-1\nlist-module-0-2\n" + "list-module-1-1\nlist-module-1-2"); + EXPECT_EQ(upload_parameters["guid"], guid); +} + +} // namespace +} // namespace test +} // namespace crashpad