diff --git a/DEPS b/DEPS index 77428536..0da988b7 100644 --- a/DEPS +++ b/DEPS @@ -19,13 +19,13 @@ vars = { deps = { 'buildtools': Var('chromium_git') + '/chromium/buildtools.git@' + - 'f8fc76ea5ce4a60cda2fa5d7df3d4a62935b3113', + 'a7cc7a3e21a061975b33dcdcd81a9716ba614c3c', 'crashpad/third_party/gtest/gtest': Var('chromium_git') + '/external/github.com/google/googletest@' + - 'ec44c6c1675c25b9827aacd08c02433cccde7780', + 'd62d6c6556d96dda924382547c54a4b3afedb22c', 'crashpad/third_party/gyp/gyp': Var('chromium_git') + '/external/gyp@' + - '93cc6e2c23e4d5ebd179f388e67aa907d0dfd43d', + 'a7055b3989c1074adca03b4b4829e7f0e57f6efd', # TODO(scottmg): Consider pinning these. For now, we don't have any particular # reason to do so. @@ -38,7 +38,7 @@ deps = { 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - 'de1afb04f4afc074ec6d23bd9ee7b1e6b365427f', + 'f65519e442d23498937251e680a3b113927613b0', } hooks = [ @@ -100,3 +100,7 @@ hooks = [ 'action': ['python', 'crashpad/build/gyp_crashpad.py'], }, ] + +recursedeps = [ + 'buildtools', +] diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 3ada8c3e..9433a6af 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -14,9 +14,11 @@ #include "handler/handler_main.h" +#include #include #include #include +#include #include #include @@ -38,6 +40,8 @@ #include "handler/prune_crash_reports_thread.h" #include "tools/tool_support.h" #include "util/file/file_io.h" +#include "util/misc/metrics.h" +#include "util/numeric/in_range_cast.h" #include "util/stdlib/map_insert.h" #include "util/stdlib/string_number_conversion.h" #include "util/string/split_string.h" @@ -105,6 +109,122 @@ void Usage(const base::FilePath& me) { #if defined(OS_MACOSX) +struct sigaction g_original_crash_sigaction[NSIG]; + +void HandleCrashSignal(int sig, siginfo_t* siginfo, void* context) { + // Is siginfo->si_code useful? The only interesting values on macOS are 0 (not + // useful, signals generated asynchronously such as by kill() or raise()) and + // small positive numbers (useful, signal generated via a hardware fault). The + // standard specifies these other constants, and while xnu never uses them, + // they are intended to denote signals generated asynchronously and are + // included here. Additionally, existing practice on other systems + // (acknowledged by the standard) is for negative numbers to indicate that a + // signal was generated asynchronously. Although xnu does not do this, allow + // for the possibility for completeness. + bool si_code_valid = !(siginfo->si_code <= 0 || + siginfo->si_code == SI_USER || + siginfo->si_code == SI_QUEUE || + siginfo->si_code == SI_TIMER || + siginfo->si_code == SI_ASYNCIO || + siginfo->si_code == SI_MESGQ); + + // 0x5343 = 'SC', signifying “signal and code”, disambiguates from the schema + // used by ExceptionCodeForMetrics(). That system primarily uses Mach + // exception types and codes, which are not available to a POSIX signal + // handler. It does provide a way to encode only signal numbers, but does so + // with the understanding that certain “raw” signals would not be encountered + // without a Mach exception. Furthermore, it does not allow siginfo->si_code + // to be encoded, because that’s not available to Mach exception handlers. It + // would be a shame to lose that information available to a POSIX signal + // handler. + int metrics_code = 0x53430000 | (InRangeCast(sig, 0xff) << 8); + if (si_code_valid) { + metrics_code |= InRangeCast(siginfo->si_code, 0xff); + } + Metrics::HandlerCrashed(metrics_code); + + // Restore the previous signal handler. + DCHECK_GT(sig, 0); + DCHECK_LT(static_cast(sig), arraysize(g_original_crash_sigaction)); + struct sigaction* osa = &g_original_crash_sigaction[sig]; + int rv = sigaction(sig, osa, nullptr); + DPLOG_IF(ERROR, rv != 0) << "sigaction " << sig; + + // If the signal was received synchronously resulting from a hardware fault, + // returning from the signal handler will cause the kernel to re-raise it, + // because this handler hasn’t done anything to alleviate the condition that + // caused the signal to be raised in the first place. With the old signal + // handler in place (expected to be SIG_DFL), it will cause the same behavior + // to be taken as though this signal handler had never been installed at all + // (expected to be a crash). This is ideal, because the signal is re-raised + // with the same properties and from the same context that initially triggered + // it, providing the best debugging experience. + + if ((sig != SIGILL && sig != SIGFPE && sig != SIGBUS && sig != SIGSEGV) || + !si_code_valid) { + // Signals received other than via hardware faults, such as those raised + // asynchronously via kill() and raise(), and those arising via hardware + // traps such as int3 (resulting in SIGTRAP but advancing the instruction + // pointer), will not reoccur on their own when returning from the signal + // handler. Re-raise them or call to the previous signal handler as + // appropriate. + // + // Unfortunately, when SIGBUS is received asynchronously via kill(), + // siginfo->si_code makes it appear as though it was actually received via a + // hardware fault. See 10.12.3 xnu-3789.41.3/bsd/dev/i386/unix_signal.c + // sendsig(). An asynchronous SIGBUS will thus cause the handler-crashed + // metric to be logged but will not cause the process to terminate. This + // isn’t ideal, but asynchronous SIGBUS is an unexpected condition. The + // alternative, to re-raise here on any SIGBUS, is a bad idea because it + // would lose properties associated with the the original signal, which are + // very valuable for debugging and are visible to a Mach exception handler. + // Since SIGBUS is normally received synchronously in response to a hardware + // fault, don’t sweat the unexpected asynchronous case. + if (osa->sa_handler == SIG_DFL) { + // Because this signal handler executes with the signal blocked, this + // raise() cannot immediately deliver the signal. Delivery is deferred + // until this signal handler returns and the signal becomes unblocked. The + // re-raised signal will appear with the same context as where it was + // initially triggered. + rv = raise(sig); + DPLOG_IF(ERROR, rv != 0) << "raise"; + } else if (osa->sa_handler != SIG_IGN) { + if (osa->sa_flags & SA_SIGINFO) { + osa->sa_sigaction(sig, siginfo, context); + } else { + osa->sa_handler(sig); + } + } + } +} + +void InstallCrashHandler() { + struct sigaction sa = {}; + sigemptyset(&sa.sa_mask); + sa.sa_flags = SA_SIGINFO; + sa.sa_sigaction = HandleCrashSignal; + + // These are the core-generating signals from 10.12.3 + // xnu-3789.41.3/bsd/sys/signalvar.h sigprop: entries with SA_CORE are in the + // set. + const int kSignals[] = {SIGQUIT, + SIGILL, + SIGTRAP, + SIGABRT, + SIGEMT, + SIGFPE, + SIGBUS, + SIGSEGV, + SIGSYS}; + + for (int sig : kSignals) { + DCHECK_GT(sig, 0); + DCHECK_LT(static_cast(sig), arraysize(g_original_crash_sigaction)); + int rv = sigaction(sig, &sa, &g_original_crash_sigaction[sig]); + PCHECK(rv == 0) << "sigaction " << sig; + } +} + struct ResetSIGTERMTraits { static struct sigaction* InvalidValue() { return nullptr; @@ -126,9 +246,8 @@ void HandleSIGTERM(int sig, siginfo_t* siginfo, void* context) { g_exception_handler_server->Stop(); } -#endif // OS_MACOSX +#elif defined(OS_WIN) -#if defined(OS_WIN) LONG(WINAPI* g_original_exception_filter)(EXCEPTION_POINTERS*) = nullptr; LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { @@ -139,15 +258,18 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { else return EXCEPTION_CONTINUE_SEARCH; } -#endif // OS_WIN + +void InstallCrashHandler() { + g_original_exception_filter = + SetUnhandledExceptionFilter(&UnhandledExceptionHandler); +} + +#endif // OS_MACOSX } // namespace int HandlerMain(int argc, char* argv[]) { -#if defined(OS_WIN) - g_original_exception_filter = - SetUnhandledExceptionFilter(&UnhandledExceptionHandler); -#endif + InstallCrashHandler(); const base::FilePath argv0( ToolSupport::CommandLineArgumentToFilePathStringType(argv[0])); diff --git a/minidump/minidump_handle_writer.cc b/minidump/minidump_handle_writer.cc index c5d4659f..098b40a1 100644 --- a/minidump/minidump_handle_writer.cc +++ b/minidump/minidump_handle_writer.cc @@ -17,7 +17,6 @@ #include #include "base/logging.h" -#include "base/stl_util.h" #include "minidump/minidump_extensions.h" #include "util/file/file_writer.h" #include "util/numeric/safe_assignment.h" @@ -29,7 +28,8 @@ MinidumpHandleDataWriter::MinidumpHandleDataWriter() } MinidumpHandleDataWriter::~MinidumpHandleDataWriter() { - base::STLDeleteContainerPairSecondPointers(strings_.begin(), strings_.end()); + for (auto& item : strings_) + delete item.second; } void MinidumpHandleDataWriter::InitializeFromSnapshot( diff --git a/minidump/minidump_simple_string_dictionary_writer.cc b/minidump/minidump_simple_string_dictionary_writer.cc index b4dd722e..ae8bfaba 100644 --- a/minidump/minidump_simple_string_dictionary_writer.cc +++ b/minidump/minidump_simple_string_dictionary_writer.cc @@ -18,7 +18,6 @@ #include "base/logging.h" #include "base/memory/ptr_util.h" -#include "base/stl_util.h" #include "util/file/file_writer.h" #include "util/numeric/safe_assignment.h" @@ -100,7 +99,8 @@ MinidumpSimpleStringDictionaryWriter::MinidumpSimpleStringDictionaryWriter() } MinidumpSimpleStringDictionaryWriter::~MinidumpSimpleStringDictionaryWriter() { - base::STLDeleteContainerPairSecondPointers(entries_.begin(), entries_.end()); + for (auto& item : entries_) + delete item.second; } void MinidumpSimpleStringDictionaryWriter::InitializeFromMap( diff --git a/snapshot/mac/process_types.h b/snapshot/mac/process_types.h index 664b37b6..7274515f 100644 --- a/snapshot/mac/process_types.h +++ b/snapshot/mac/process_types.h @@ -27,13 +27,21 @@ namespace crashpad { namespace process_types { namespace internal { +// Some structure definitions have tail padding when built in a 64-bit +// environment, but will have less (or no) tail padding in a 32-bit environment. +// These structure’s apparent sizes will differ between these two environments, +// which is incorrect. Use this “Nothing” type to place an “end” marker after +// all of the real members of such structures, and use offsetof(type, end) to +// compute their actual sizes. +using Nothing = char[0]; + // Some structure definitions differ in 32-bit and 64-bit environments by having // additional “reserved” padding fields present only in the 64-bit environment. // These Reserved*_64Only* types allow the process_types system to replicate // these structures more precisely. -using Reserved32_64Only32 = char[0]; +using Reserved32_64Only32 = Nothing; using Reserved32_64Only64 = uint32_t; -using Reserved64_64Only32 = char[0]; +using Reserved64_64Only32 = Nothing; using Reserved64_64Only64 = uint64_t; } // namespace internal @@ -65,6 +73,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) using UIntPtr = internal::TraitsGeneric::UIntPtr; \ using Reserved32_64Only = internal::TraitsGeneric::Reserved32_64Only; \ using Reserved64_64Only = internal::TraitsGeneric::Reserved64_64Only; \ + using Nothing = internal::TraitsGeneric::Nothing; \ \ /* Initializes an object with data read from |process_reader| at \ * |address|, properly genericized. */ \ @@ -88,7 +97,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) /* Similar to Size(), but computes the expected size of a structure based \ * on the process’ bitness. This can be used prior to reading any data \ * from a process. */ \ - static size_t ExpectedSize(ProcessReader* process_reader); \ + static size_t ExpectedSize(ProcessReader* process_reader); #define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) \ member_type member_name __VA_ARGS__; @@ -155,6 +164,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) using UIntPtr = typename Traits::UIntPtr; \ using Reserved32_64Only = typename Traits::Reserved32_64Only; \ using Reserved64_64Only = typename Traits::Reserved64_64Only; \ + using Nothing = typename Traits::Nothing; \ \ /* Read(), ReadArrayInto(), and Size() are as in the generic user-visible \ * struct above. */ \ diff --git a/snapshot/mac/process_types/custom.cc b/snapshot/mac/process_types/custom.cc index 5fe92288..33e3a7ec 100644 --- a/snapshot/mac/process_types/custom.cc +++ b/snapshot/mac/process_types/custom.cc @@ -17,6 +17,8 @@ #include #include +#include + #include "base/logging.h" #include "snapshot/mac/process_types/internal.h" #include "util/mach/task_memory.h" @@ -62,46 +64,31 @@ bool ReadIntoVersioned(ProcessReader* process_reader, template size_t dyld_all_image_infos::ExpectedSizeForVersion( decltype(dyld_all_image_infos::version) version) { - if (version >= 14) { - return sizeof(dyld_all_image_infos); + const size_t kSizeForVersion[] = { + offsetof(dyld_all_image_infos, infoArrayCount), // 0 + offsetof(dyld_all_image_infos, libSystemInitialized), // 1 + offsetof(dyld_all_image_infos, jitInfo), // 2 + offsetof(dyld_all_image_infos, dyldVersion), // 3 + offsetof(dyld_all_image_infos, dyldVersion), // 4 + offsetof(dyld_all_image_infos, coreSymbolicationShmPage), // 5 + offsetof(dyld_all_image_infos, systemOrderFlag), // 6 + offsetof(dyld_all_image_infos, uuidArrayCount), // 7 + offsetof(dyld_all_image_infos, dyldAllImageInfosAddress), // 8 + offsetof(dyld_all_image_infos, initialImageCount), // 9 + offsetof(dyld_all_image_infos, errorKind), // 10 + offsetof(dyld_all_image_infos, sharedCacheSlide), // 11 + offsetof(dyld_all_image_infos, sharedCacheUUID), // 12 + offsetof(dyld_all_image_infos, infoArrayChangeTimestamp), // 13 + offsetof(dyld_all_image_infos, end), // 14 + }; + + if (version >= arraysize(kSizeForVersion)) { + return kSizeForVersion[arraysize(kSizeForVersion) - 1]; } - if (version >= 13) { - return offsetof(dyld_all_image_infos, infoArrayChangeTimestamp); - } - if (version >= 12) { - return offsetof(dyld_all_image_infos, sharedCacheUUID); - } - if (version >= 11) { - return offsetof(dyld_all_image_infos, sharedCacheSlide); - } - if (version >= 10) { - return offsetof(dyld_all_image_infos, errorKind); - } - if (version >= 9) { - return offsetof(dyld_all_image_infos, initialImageCount); - } - if (version >= 8) { - return offsetof(dyld_all_image_infos, dyldAllImageInfosAddress); - } - if (version >= 7) { - return offsetof(dyld_all_image_infos, uuidArrayCount); - } - if (version >= 6) { - return offsetof(dyld_all_image_infos, systemOrderFlag); - } - if (version >= 5) { - return offsetof(dyld_all_image_infos, coreSymbolicationShmPage); - } - if (version >= 3) { - return offsetof(dyld_all_image_infos, dyldVersion); - } - if (version >= 2) { - return offsetof(dyld_all_image_infos, jitInfo); - } - if (version >= 1) { - return offsetof(dyld_all_image_infos, libSystemInitialized); - } - return offsetof(dyld_all_image_infos, infoArrayCount); + + static_assert(std::is_unsigned::value, + "version must be unsigned"); + return kSizeForVersion[version]; } // static diff --git a/snapshot/mac/process_types/dyld_images.proctype b/snapshot/mac/process_types/dyld_images.proctype index eb8410aa..3470ee83 100644 --- a/snapshot/mac/process_types/dyld_images.proctype +++ b/snapshot/mac/process_types/dyld_images.proctype @@ -123,6 +123,13 @@ PROCESS_TYPE_STRUCT_BEGIN(dyld_all_image_infos) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_6) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_7) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_8) + + // The 32-bit version of the structure will have four extra bytes of tail + // padding when built for 64-bit systems than it does natively and when built + // for 32-bit systems. Instead of using sizeof(dyld_all_image_infos), use + // offsetof(dyld_all_image_infos, end) to avoid taking this tail padding into + // account. + PROCESS_TYPE_STRUCT_MEMBER(Nothing, end) PROCESS_TYPE_STRUCT_END(dyld_all_image_infos) #endif // ! PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO && diff --git a/snapshot/mac/process_types/traits.h b/snapshot/mac/process_types/traits.h index fec234d0..396799be 100644 --- a/snapshot/mac/process_types/traits.h +++ b/snapshot/mac/process_types/traits.h @@ -22,7 +22,8 @@ // DECLARE_PROCESS_TYPE_TRAITS_CLASS before #including this file again and after // the last #include of this file. // -// |Reserved*| are used for padding fields that may be zero-length, and thus +// |Reserved*| is used for padding fields that may be zero-length, and |Nothing| +// is always zero-length and is used solely as an anchor to compute offsets. // __VA_ARGS__, which is intended to set the alignment of the 64-bit types, is // not used for those type aliases. #define DECLARE_PROCESS_TYPE_TRAITS_CLASS(traits_name, lp_bits, ...) \ @@ -37,6 +38,7 @@ using UIntPtr = uint##lp_bits##_t __VA_ARGS__; \ using Reserved32_64Only = Reserved32_64Only##lp_bits; \ using Reserved64_64Only = Reserved64_64Only##lp_bits; \ + using Nothing = Nothing; \ }; \ } \ } \ diff --git a/snapshot/mac/process_types_test.cc b/snapshot/mac/process_types_test.cc index ce092fe1..8525c14e 100644 --- a/snapshot/mac/process_types_test.cc +++ b/snapshot/mac/process_types_test.cc @@ -24,6 +24,7 @@ #include "base/strings/stringprintf.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "snapshot/mac/process_types/internal.h" #include "test/mac/dyld.h" #include "util/mac/mac_util.h" #include "util/misc/implicit_cast.h" @@ -116,6 +117,42 @@ TEST(ProcessTypes, DyldImagesSelf) { process_types::dyld_all_image_infos::ExpectedSizeForVersion( &process_reader, kDyldAllImageInfosVersionInSDK)); + // Make sure that the computed sizes of various versions of this structure are + // correct at different bitnessses. + const struct { + uint32_t version; + size_t size_32; + size_t size_64; + } kVersionsAndSizes[] = { + {1, 17, 25}, + {2, 24, 40}, + {3, 28, 48}, + {5, 40, 72}, + {6, 44, 80}, + {7, 48, 88}, + {8, 56, 104}, + {9, 60, 112}, + {10, 64, 120}, + {11, 80, 152}, + {12, 84, 160}, + {13, 104, 184}, + {14, 164, 304}, + {15, 164, 304}, + }; + for (size_t index = 0; index < arraysize(kVersionsAndSizes); ++index) { + uint32_t version = kVersionsAndSizes[index].version; + SCOPED_TRACE(base::StringPrintf("index %zu, version %u", index, version)); + + EXPECT_EQ(kVersionsAndSizes[index].size_32, + process_types::internal::dyld_all_image_infos< + process_types::internal::Traits32>:: + ExpectedSizeForVersion(version)); + EXPECT_EQ(kVersionsAndSizes[index].size_64, + process_types::internal::dyld_all_image_infos< + process_types::internal::Traits64>:: + ExpectedSizeForVersion(version)); + } + process_types::dyld_all_image_infos proctype_image_infos; ASSERT_TRUE(proctype_image_infos.Read(&process_reader, dyld_info.all_image_info_addr)); diff --git a/snapshot/mac/system_snapshot_mac.cc b/snapshot/mac/system_snapshot_mac.cc index 3c39088f..b65a9e08 100644 --- a/snapshot/mac/system_snapshot_mac.cc +++ b/snapshot/mac/system_snapshot_mac.cc @@ -19,6 +19,8 @@ #include #include +#include + #include "base/logging.h" #include "base/strings/stringprintf.h" #include "build/build_config.h" @@ -349,34 +351,44 @@ void SystemSnapshotMac::TimeZone(DaylightSavingTimeStatus* dst_status, PCHECK(localtime_r(&snapshot_time_->tv_sec, &local)) << "localtime_r"; *standard_name = tzname[0]; + + bool found_transition = false; + long probe_gmtoff = local.tm_gmtoff; if (daylight) { // Scan forward and backward, one month at a time, looking for an instance // when the observance of daylight saving time is different than it is in - // |local|. - long probe_gmtoff = local.tm_gmtoff; - + // |local|. It’s possible that no such instance will be found even with + // |daylight| set. This can happen in locations where daylight saving time + // was once observed or is expected to be observed in the future, but where + // no transitions to or from daylight saving time occurred or will occur + // within a year of the current date. Arizona, which last observed daylight + // saving time in 1967, is an example. const int kMonthDeltas[] = { 0, 1, -1, 2, -2, 3, -3, 4, -4, 5, -5, 6, -6, 7, -7, 8, -8, 9, -9, 10, -10, 11, -11, 12, -12 }; - for (size_t index = 0; index < arraysize(kMonthDeltas); ++index) { - // Look at the 15th day of each month at local noon. Set tm_isdst to -1 to - // avoid giving mktime() any hints about whether to consider daylight - // saving time in effect. mktime() accepts values of tm_mon that are - // outside of its normal range and behaves as expected: if tm_mon is -1, - // it references December of the preceding year, and if it is 12, it + for (size_t index = 0; + index < arraysize(kMonthDeltas) && !found_transition; + ++index) { + // Look at a day of each month at local noon. Set tm_isdst to -1 to avoid + // giving mktime() any hints about whether to consider daylight saving + // time in effect. mktime() accepts values of tm_mon that are outside of + // its normal range and behaves as expected: if tm_mon is -1, it + // references December of the preceding year, and if it is 12, it // references January of the following year. tm probe_tm = {}; probe_tm.tm_hour = 12; - probe_tm.tm_mday = 15; + probe_tm.tm_mday = std::min(local.tm_mday, 28); probe_tm.tm_mon = local.tm_mon + kMonthDeltas[index]; probe_tm.tm_year = local.tm_year; probe_tm.tm_isdst = -1; if (mktime(&probe_tm) != -1 && probe_tm.tm_isdst != local.tm_isdst) { + found_transition = true; probe_gmtoff = probe_tm.tm_gmtoff; - break; } } + } + if (found_transition) { *daylight_name = tzname[1]; if (!local.tm_isdst) { *dst_status = kObservingStandardTime; diff --git a/snapshot/mac/system_snapshot_mac_test.cc b/snapshot/mac/system_snapshot_mac_test.cc index a02e94e0..aff773f3 100644 --- a/snapshot/mac/system_snapshot_mac_test.cc +++ b/snapshot/mac/system_snapshot_mac_test.cc @@ -14,11 +14,13 @@ #include "snapshot/mac/system_snapshot_mac.h" +#include #include #include #include +#include "base/strings/stringprintf.h" #include "build/build_config.h" #include "gtest/gtest.h" #include "snapshot/mac/process_reader.h" @@ -127,6 +129,39 @@ TEST_F(SystemSnapshotMacTest, MachineDescription) { EXPECT_FALSE(system_snapshot().MachineDescription().empty()); } +class ScopedSetTZ { + public: + ScopedSetTZ(const std::string& tz) { + const char* old_tz = getenv(kTZ); + old_tz_set_ = old_tz; + if (old_tz_set_) { + old_tz_.assign(old_tz); + } + + EXPECT_EQ(0, setenv(kTZ, tz.c_str(), 1)) << ErrnoMessage("setenv"); + tzset(); + } + + ~ScopedSetTZ() { + if (old_tz_set_) { + EXPECT_EQ(0, setenv(kTZ, old_tz_.c_str(), 1)) << ErrnoMessage("setenv"); + } else { + EXPECT_EQ(0, unsetenv(kTZ)) << ErrnoMessage("unsetenv"); + } + tzset(); + } + + private: + std::string old_tz_; + bool old_tz_set_; + + static constexpr char kTZ[] = "TZ"; + + DISALLOW_COPY_AND_ASSIGN(ScopedSetTZ); +}; + +constexpr char ScopedSetTZ::kTZ[]; + TEST_F(SystemSnapshotMacTest, TimeZone) { SystemSnapshot::DaylightSavingTimeStatus dst_status; int standard_offset_seconds; @@ -169,6 +204,68 @@ TEST_F(SystemSnapshotMacTest, TimeZone) { EXPECT_NE(standard_name, daylight_name); } + + // Test a variety of time zones. Some of these observe daylight saving time, + // some don’t. Some used to but no longer do. Some have uncommon UTC offsets. + const struct { + const char* tz; + bool observes_dst; + float standard_offset_hours; + float daylight_offset_hours; + const char* standard_name; + const char* daylight_name; + } kTestTimeZones[] = { + {"America/Anchorage", true, -9, -8, "AKST", "AKDT"}, + {"America/Chicago", true, -6, -5, "CST", "CDT"}, + {"America/Denver", true, -7, -6, "MST", "MDT"}, + {"America/Halifax", true, -4, -3, "AST", "ADT"}, + {"America/Los_Angeles", true, -8, -7, "PST", "PDT"}, + {"America/New_York", true, -5, -4, "EST", "EDT"}, + {"America/Phoenix", false, -7, -7, "MST", "MST"}, + {"Asia/Karachi", false, 5, 5, "PKT", "PKT"}, + {"Asia/Kolkata", false, 5.5, 5.5, "IST", "IST"}, + {"Asia/Shanghai", false, 8, 8, "CST", "CST"}, + {"Asia/Tokyo", false, 9, 9, "JST", "JST"}, + {"Australia/Adelaide", true, 9.5, 10.5, "ACST", "ACDT"}, + {"Australia/Brisbane", false, 10, 10, "AEST", "AEST"}, + {"Australia/Darwin", false, 9.5, 9.5, "ACST", "ACST"}, + {"Australia/Eucla", false, 8.75, 8.75, "ACWST", "ACWST"}, + {"Australia/Lord_Howe", true, 10.5, 11, "LHST", "LHDT"}, + {"Australia/Perth", false, 8, 8, "AWST", "AWST"}, + {"Australia/Sydney", true, 10, 11, "AEST", "AEDT"}, + {"Europe/Bucharest", true, 2, 3, "EET", "EEST"}, + {"Europe/London", true, 0, 1, "GMT", "BST"}, + {"Europe/Moscow", false, 3, 3, "MSK", "MSK"}, + {"Europe/Paris", true, 1, 2, "CET", "CEST"}, + {"Europe/Reykjavik", false, 0, 0, "UTC", "UTC"}, + {"Pacific/Auckland", true, 12, 13, "NZST", "NZDT"}, + {"Pacific/Honolulu", false, -10, -10, "HST", "HST"}, + {"UTC", false, 0, 0, "UTC", "UTC"}, + }; + + for (size_t index = 0; index < arraysize(kTestTimeZones); ++index) { + const auto& test_time_zone = kTestTimeZones[index]; + const char* tz = test_time_zone.tz; + SCOPED_TRACE(base::StringPrintf("index %zu, tz %s", index, tz)); + + { + ScopedSetTZ set_tz(tz); + system_snapshot().TimeZone(&dst_status, + &standard_offset_seconds, + &daylight_offset_seconds, + &standard_name, + &daylight_name); + } + + EXPECT_EQ(test_time_zone.observes_dst, + dst_status != SystemSnapshot::kDoesNotObserveDaylightSavingTime); + EXPECT_EQ(test_time_zone.standard_offset_hours * 60 * 60, + standard_offset_seconds); + EXPECT_EQ(test_time_zone.daylight_offset_hours * 60 * 60, + daylight_offset_seconds); + EXPECT_EQ(test_time_zone.standard_name, standard_name); + EXPECT_EQ(test_time_zone.daylight_name, daylight_name); + } } } // namespace diff --git a/snapshot/win/thread_snapshot_win.cc b/snapshot/win/thread_snapshot_win.cc index 8b86730c..d737c14b 100644 --- a/snapshot/win/thread_snapshot_win.cc +++ b/snapshot/win/thread_snapshot_win.cc @@ -14,6 +14,7 @@ #include "snapshot/win/thread_snapshot_win.h" +#include #include #include "base/logging.h" diff --git a/util/mach/exception_types.cc b/util/mach/exception_types.cc index 85bdf770..e133cb9a 100644 --- a/util/mach/exception_types.cc +++ b/util/mach/exception_types.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include "base/logging.h" #include "base/mac/mach_logging.h" @@ -200,16 +201,19 @@ int32_t ExceptionCodeForMetrics(exception_type_t exception, // one another. For the purposes of encoding these codes for metrics, // convert the flavor codes to their corresponding bit shift values. const uint32_t guard_flavor = (code_0 >> 32) & 0x1fffffff; - uint8_t metrics_guard_flavor = 0xff; - for (int bit = 0; bit < 29; ++bit) { - const uint32_t test_mask = 1 << bit; - if (guard_flavor & test_mask) { - // Make sure that no other bits are set. - DCHECK_EQ(guard_flavor, test_mask); + const int first_bit = ffs(guard_flavor); + uint8_t metrics_guard_flavor; + if (first_bit) { + metrics_guard_flavor = first_bit - 1; - metrics_guard_flavor = bit; - break; + const uint32_t test_guard_flavor = 1 << metrics_guard_flavor; + if (guard_flavor != test_guard_flavor) { + // Another bit is set. + DCHECK_EQ(guard_flavor, test_guard_flavor); + metrics_guard_flavor = 0xff; } + } else { + metrics_guard_flavor = 0xff; } metrics_code_0 = (guard_type << 8) | metrics_guard_flavor; @@ -305,15 +309,32 @@ bool IsExceptionNonfatalResource(exception_type_t exception, if (resource_type == RESOURCE_TYPE_MEMORY && resource_flavor == FLAVOR_HIGH_WATERMARK) { - // These exceptions are never fatal. See 10.10 + // These exceptions were never fatal prior to 10.12. See 10.10 // xnu-2782.1.97/osfmk/kern/task.c // THIS_PROCESS_CROSSED_HIGH_WATERMARK__SENDING_EXC_RESOURCE(). + // + // A superficial examination of 10.12 shows that these exceptions may be + // fatal, as determined by the P_MEMSTAT_FATAL_MEMLIMIT bit of the + // kernel-internal struct proc::p_memstat_state. See 10.12.3 + // xnu-3789.41.3/osfmk/kern/task.c task_footprint_exceeded(). This bit is + // not exposed to user space, which makes it difficult to determine whether + // the kernel considers a given instance of this exception fatal. However, a + // close read reveals that it is only possible for this bit to become set + // when xnu-3789.41.3/bsd/kern/kern_memorystatus.c + // memorystatus_cmd_set_memlimit_properties() is called, which is only + // possible when the kernel is built with CONFIG_JETSAM set, or if the + // kern.memorystatus_highwater_enabled sysctl is used, which is only + // possible when the kernel is built with DEVELOPMENT or DEBUG set. Although + // CONFIG_JETSAM is used on iOS, it is not used on macOS. DEVELOPMENT and + // DEBUG are also not set for production kernels. It therefore remains + // impossible for these exceptions to be fatal, even on 10.12. return true; } if (resource_type == RESOURCE_TYPE_IO) { - // These exceptions don’t ever appear to be fatal. See - // https://crashpad.chromium.org/bug/124. + // These exceptions are never fatal. See 10.12.3 + // xnu-3789.41.3/osfmk/kern/task.c + // SENDING_NOTIFICATION__THIS_PROCESS_IS_CAUSING_TOO_MUCH_IO(). return true; } diff --git a/util/mach/exception_types_test.cc b/util/mach/exception_types_test.cc index 7ecdd252..4175c637 100644 --- a/util/mach/exception_types_test.cc +++ b/util/mach/exception_types_test.cc @@ -57,7 +57,13 @@ TEST(ExceptionTypes, ExcCrashRecoverOriginalException) { {0x0700080, EXC_SYSCALL, 128, 0}, {0x0706000, EXC_SYSCALL, 0x6000, 0}, {0x3000000, 0, 0, SIGQUIT}, + {0x4000000, 0, 0, SIGILL}, + {0x5000000, 0, 0, SIGTRAP}, {0x6000000, 0, 0, SIGABRT}, + {0x7000000, 0, 0, SIGEMT}, + {0x8000000, 0, 0, SIGFPE}, + {0xa000000, 0, 0, SIGBUS}, + {0xb000000, 0, 0, SIGSEGV}, {0xc000000, 0, 0, SIGSYS}, {0, 0, 0, 0}, }; @@ -181,7 +187,13 @@ TEST(ExceptionTypes, ExceptionCodeForMetrics) { #define ENCODE_EXC_CRASH_SIGNAL(signal) \ { EXC_CRASH, (((signal) & 0xff) << 24), (EXC_CRASH << 16) | (signal) } ENCODE_EXC_CRASH_SIGNAL(SIGQUIT), + ENCODE_EXC_CRASH_SIGNAL(SIGILL), + ENCODE_EXC_CRASH_SIGNAL(SIGTRAP), ENCODE_EXC_CRASH_SIGNAL(SIGABRT), + ENCODE_EXC_CRASH_SIGNAL(SIGEMT), + ENCODE_EXC_CRASH_SIGNAL(SIGFPE), + ENCODE_EXC_CRASH_SIGNAL(SIGBUS), + ENCODE_EXC_CRASH_SIGNAL(SIGSEGV), ENCODE_EXC_CRASH_SIGNAL(SIGSYS), #undef ENCODE_EXC_CRASH_SIGNAL diff --git a/util/net/http_body.cc b/util/net/http_body.cc index b1bca19e..96a7cc40 100644 --- a/util/net/http_body.cc +++ b/util/net/http_body.cc @@ -20,7 +20,6 @@ #include #include "base/logging.h" -#include "base/stl_util.h" #include "util/misc/implicit_cast.h" namespace crashpad { @@ -89,7 +88,8 @@ CompositeHTTPBodyStream::CompositeHTTPBodyStream( } CompositeHTTPBodyStream::~CompositeHTTPBodyStream() { - base::STLDeleteContainerPointers(parts_.begin(), parts_.end()); + for (auto& item : parts_) + delete item; } FileOperationResult CompositeHTTPBodyStream::GetBytesBuffer(uint8_t* buffer, diff --git a/util/net/http_transport_test_server.py b/util/net/http_transport_test_server.py index 3f085a56..3d3c55a4 100755 --- a/util/net/http_transport_test_server.py +++ b/util/net/http_transport_test_server.py @@ -26,7 +26,7 @@ process one HTTP request, deliver the prearranged response to the client, and write the entire request to stdout. It will then terminate. This server is written in Python since it provides a simple HTTP stack, and -because parsing Chunked encoding is safer and easier in a memory-safe language. +because parsing chunked encoding is safer and easier in a memory-safe language. This could easily have been written in C++ instead. """ @@ -80,7 +80,7 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): RequestHandler.raw_request = self.rfile.buffer self.rfile.buffer = '' - if self.headers.get('Transfer-Encoding', '') == 'Chunked': + if self.headers.get('Transfer-Encoding', '').lower() == 'chunked': body = self.handle_chunked_encoding() else: length = int(self.headers.get('Content-Length', -1)) @@ -130,6 +130,10 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): return -1 return int(chunk_size_and_ext_line[:chunk_size_end], base=16) + def log_request(self, code='-', size='-'): + # The default implementation logs these to sys.stderr, which is just noise. + pass + def Main(): if sys.platform == 'win32': diff --git a/util/net/http_transport_win.cc b/util/net/http_transport_win.cc index 58ecc478..294048a9 100644 --- a/util/net/http_transport_win.cc +++ b/util/net/http_transport_win.cc @@ -15,17 +15,22 @@ #include "util/net/http_transport.h" #include +#include #include +#include #include +#include #include #include "base/logging.h" #include "base/numerics/safe_conversions.h" #include "base/scoped_generic.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "package.h" #include "util/file/file_io.h" +#include "util/numeric/safe_assignment.h" #include "util/net/http_body.h" namespace crashpad { @@ -34,7 +39,7 @@ namespace { // PLOG doesn't work for messages from WinHTTP, so we need to use // FORMAT_MESSAGE_FROM_HMODULE + the dll name manually here. -void LogErrorWinHttpMessage(const char* extra) { +std::string WinHttpMessage(const char* extra) { DWORD error_code = GetLastError(); char msgbuf[256]; DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | @@ -46,15 +51,13 @@ void LogErrorWinHttpMessage(const char* extra) { msgbuf, arraysize(msgbuf), NULL); - if (len) { - LOG(ERROR) << extra << ": " << msgbuf - << base::StringPrintf(" (0x%X)", error_code); - } else { - LOG(ERROR) << base::StringPrintf( - "Error (0x%X) while retrieving error. (0x%X)", - GetLastError(), - error_code); + if (!len) { + return base::StringPrintf("%s: error 0x%x while retrieving error 0x%x", + extra, + GetLastError(), + error_code); } + return base::StringPrintf("%s: %s (0x%x)", extra, msgbuf, error_code); } struct ScopedHINTERNETTraits { @@ -64,7 +67,7 @@ struct ScopedHINTERNETTraits { static void Free(HINTERNET handle) { if (handle) { if (!WinHttpCloseHandle(handle)) { - LogErrorWinHttpMessage("WinHttpCloseHandle"); + LOG(ERROR) << WinHttpMessage("WinHttpCloseHandle"); } } } @@ -97,7 +100,7 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { WINHTTP_NO_PROXY_BYPASS, 0)); if (!session.get()) { - LogErrorWinHttpMessage("WinHttpOpen"); + LOG(ERROR) << WinHttpMessage("WinHttpOpen"); return false; } @@ -107,7 +110,7 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { timeout_in_ms, timeout_in_ms, timeout_in_ms)) { - LogErrorWinHttpMessage("WinHttpSetTimeouts"); + LOG(ERROR) << WinHttpMessage("WinHttpSetTimeouts"); return false; } @@ -121,7 +124,7 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { // https://msdn.microsoft.com/en-us/library/aa384092.aspx if (!WinHttpCrackUrl( url_wide.c_str(), 0, 0, &url_components)) { - LogErrorWinHttpMessage("WinHttpCrackUrl"); + LOG(ERROR) << WinHttpMessage("WinHttpCrackUrl"); return false; } DCHECK(url_components.nScheme == INTERNET_SCHEME_HTTP || @@ -136,7 +139,7 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { ScopedHINTERNET connect(WinHttpConnect( session.get(), host_name.c_str(), url_components.nPort, 0)); if (!connect.get()) { - LogErrorWinHttpMessage("WinHttpConnect"); + LOG(ERROR) << WinHttpMessage("WinHttpConnect"); return false; } @@ -150,57 +153,152 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { url_components.nScheme == INTERNET_SCHEME_HTTPS ? WINHTTP_FLAG_SECURE : 0)); if (!request.get()) { - LogErrorWinHttpMessage("WinHttpOpenRequest"); + LOG(ERROR) << WinHttpMessage("WinHttpOpenRequest"); return false; } // Add headers to the request. + // + // If Content-Length is not provided, implement chunked mode per RFC 7230 + // §4.1. + // + // Note that chunked mode can only be used on Vista and later. Otherwise, + // WinHttpSendRequest() requires a real value for dwTotalLength, used for the + // Content-Length header. Determining that in the absence of a provided + // Content-Length would require reading the entire request body before calling + // WinHttpSendRequest(). + bool chunked = true; + size_t content_length = 0; for (const auto& pair : headers()) { - std::wstring header_string = - base::UTF8ToUTF16(pair.first) + L": " + base::UTF8ToUTF16(pair.second); - if (!WinHttpAddRequestHeaders( - request.get(), - header_string.c_str(), - base::checked_cast(header_string.size()), - WINHTTP_ADDREQ_FLAG_ADD)) { - LogErrorWinHttpMessage("WinHttpAddRequestHeaders"); - return false; + if (pair.first == kContentLength) { + chunked = !base::StringToSizeT(pair.second, &content_length); + DCHECK(!chunked); + } else { + std::wstring header_string = base::UTF8ToUTF16(pair.first) + L": " + + base::UTF8ToUTF16(pair.second) + L"\r\n"; + if (!WinHttpAddRequestHeaders( + request.get(), + header_string.c_str(), + base::checked_cast(header_string.size()), + WINHTTP_ADDREQ_FLAG_ADD)) { + LOG(ERROR) << WinHttpMessage("WinHttpAddRequestHeaders"); + return false; + } } } - // We need the Content-Length up front, so buffer in memory. We should modify - // the interface to not require this, and then use WinHttpWriteData after - // WinHttpSendRequest. - std::vector post_data; - - // Write the body of a POST if any. - const size_t kBufferSize = 4096; - for (;;) { - uint8_t buffer[kBufferSize]; - FileOperationResult bytes_to_write = - body_stream()->GetBytesBuffer(buffer, sizeof(buffer)); - if (bytes_to_write == 0) { - break; - } else if (bytes_to_write < 0) { - LOG(ERROR) << "GetBytesBuffer failed"; + DWORD content_length_dword; + if (chunked) { + const wchar_t kTransferEncodingHeader[] = L"Transfer-Encoding: chunked\r\n"; + if (!WinHttpAddRequestHeaders( + request.get(), + kTransferEncodingHeader, + base::checked_cast(wcslen(kTransferEncodingHeader)), + WINHTTP_ADDREQ_FLAG_ADD)) { + LOG(ERROR) << WinHttpMessage("WinHttpAddRequestHeaders"); return false; } - post_data.insert(post_data.end(), buffer, buffer + bytes_to_write); + + content_length_dword = WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH; + } else if (!AssignIfInRange(&content_length_dword, content_length)) { + content_length_dword = WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH; } if (!WinHttpSendRequest(request.get(), WINHTTP_NO_ADDITIONAL_HEADERS, 0, - &post_data[0], - base::checked_cast(post_data.size()), - base::checked_cast(post_data.size()), + WINHTTP_NO_REQUEST_DATA, + 0, + content_length_dword, 0)) { - LogErrorWinHttpMessage("WinHttpSendRequest"); + LOG(ERROR) << WinHttpMessage("WinHttpSendRequest"); return false; } + size_t total_written = 0; + FileOperationResult data_bytes; + do { + struct { + char size[8]; + char crlf0[2]; + uint8_t data[32 * 1024]; + char crlf1[2]; + } buf; + static_assert(sizeof(buf) == sizeof(buf.size) + + sizeof(buf.crlf0) + + sizeof(buf.data) + + sizeof(buf.crlf1), + "buf should not have padding"); + + // Read a block of data. + data_bytes = body_stream()->GetBytesBuffer(buf.data, sizeof(buf.data)); + if (data_bytes == -1) { + return false; + } + DCHECK_GE(data_bytes, 0); + DCHECK_LE(static_cast(data_bytes), sizeof(buf.data)); + + void* write_start; + DWORD write_size; + + if (chunked) { + // Chunked encoding uses the entirety of buf. buf.size is presented in + // hexadecimal without any leading "0x". The terminating CR and LF will be + // placed immediately following the used portion of buf.data, even if + // buf.data is not full, and not necessarily in buf.crlf1. + + unsigned int data_bytes_ui = base::checked_cast(data_bytes); + + // snprintf() would NUL-terminate, but _snprintf() won’t. + int rv = _snprintf(buf.size, sizeof(buf.size), "%08x", data_bytes_ui); + DCHECK_GE(rv, 0); + DCHECK_EQ(static_cast(rv), sizeof(buf.size)); + DCHECK_NE(buf.size[sizeof(buf.size) - 1], '\0'); + + buf.crlf0[0] = '\r'; + buf.crlf0[1] = '\n'; + buf.data[data_bytes] = '\r'; + buf.data[data_bytes + 1] = '\n'; + + // Skip leading zeroes in the chunk size. + unsigned int size_len; + for (size_len = sizeof(buf.size); size_len > 1; --size_len) { + if (buf.size[sizeof(buf.size) - size_len] != '0') { + break; + } + } + + write_start = buf.crlf0 - size_len; + write_size = base::checked_cast(size_len + sizeof(buf.crlf0) + + data_bytes + sizeof(buf.crlf1)); + } else { + // When not using chunked encoding, only use buf.data. + write_start = buf.data; + write_size = base::checked_cast(data_bytes); + } + + // write_size will be 0 at EOF in non-chunked mode. Skip the write in that + // case. In contrast, at EOF in chunked mode, a zero-length chunk must be + // sent to signal EOF. This will happen when processing the EOF indicated by + // a 0 return from body_stream()->GetBytesBuffer() above. + if (write_size != 0) { + DWORD written; + if (!WinHttpWriteData(request.get(), write_start, write_size, &written)) { + LOG(ERROR) << WinHttpMessage("WinHttpWriteData"); + return false; + } + + DCHECK_EQ(written, write_size); + total_written += written; + } + } while (data_bytes > 0); + + if (!chunked) { + DCHECK_EQ(total_written, content_length); + } + if (!WinHttpReceiveResponse(request.get(), nullptr)) { - LogErrorWinHttpMessage("WinHttpReceiveResponse"); + LOG(ERROR) << WinHttpMessage("WinHttpReceiveResponse"); return false; } @@ -214,7 +312,7 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { &status_code, &sizeof_status_code, WINHTTP_NO_HEADER_INDEX)) { - LogErrorWinHttpMessage("WinHttpQueryHeaders"); + LOG(ERROR) << WinHttpMessage("WinHttpQueryHeaders"); return false; } @@ -232,10 +330,10 @@ bool HTTPTransportWin::ExecuteSynchronously(std::string* response_body) { // which executes synchronously, is only concerned with reading until EOF. DWORD bytes_read = 0; do { - char read_buffer[kBufferSize]; + char read_buffer[4096]; if (!WinHttpReadData( request.get(), read_buffer, sizeof(read_buffer), &bytes_read)) { - LogErrorWinHttpMessage("WinHttpReadData"); + LOG(ERROR) << WinHttpMessage("WinHttpReadData"); return false; } diff --git a/util/stdlib/pointer_container.h b/util/stdlib/pointer_container.h index 576270df..7d2d5d06 100644 --- a/util/stdlib/pointer_container.h +++ b/util/stdlib/pointer_container.h @@ -18,30 +18,9 @@ #include #include "base/macros.h" -#include "base/stl_util.h" namespace crashpad { -//! \brief Allows a standard container to “own” pointer elements stored in it. -//! -//! When the container is destroyed, `delete` will be called on its pointer -//! elements. -//! -//! \note No attempt is made to `delete` elements that are removed from the -//! container by other means, such as replacement or `clear()`. -template -class PointerContainer : public ContainerType { - public: - PointerContainer() : ContainerType(), pointer_deleter_(this) {} - - ~PointerContainer() {} - - private: - base::STLElementDeleter pointer_deleter_; - - DISALLOW_COPY_AND_ASSIGN(PointerContainer); -}; - //! \brief Allows a `std::vector` to “own” pointer elements stored in it. //! //! When the vector is destroyed, `delete` will be called on its pointer @@ -50,7 +29,18 @@ class PointerContainer : public ContainerType { //! \note No attempt is made to `delete` elements that are removed from the //! vector by other means, such as replacement or `clear()`. template -using PointerVector = PointerContainer>; +class PointerVector : public std::vector { + public: + PointerVector() : std::vector() {} + + ~PointerVector() { + for (T* item : *this) + delete item; + } + + private: + DISALLOW_COPY_AND_ASSIGN(PointerVector); +}; } // namespace crashpad diff --git a/util/thread/thread_log_messages.cc b/util/thread/thread_log_messages.cc index dec111e7..60be08b5 100644 --- a/util/thread/thread_log_messages.cc +++ b/util/thread/thread_log_messages.cc @@ -16,7 +16,6 @@ #include -#include "base/lazy_instance.h" #include "base/logging.h" #include "base/threading/thread_local_storage.h" @@ -28,14 +27,24 @@ namespace { // handler. A thread may register its thread-specific log message list to // receive messages produced just on that thread. // -// Only one object of this class may exist in the program at a time. There must -// not be any log message handler in effect when it is created, and nothing else -// can be set as a log message handler while an object of this class exists. -// -// Practically, the only object of this class that might exist is managed by the -// g_master lazy instance, which will create it upon first use. +// Only one object of this class may exist in the program at a time as created +// by GetInstance(). There must not be any log message handler in effect when it +// is created, and nothing else can be set as a log message handler while an +// object of this class exists. class ThreadLogMessagesMaster { public: + void SetThreadMessageList(std::vector* message_list) { + DCHECK_EQ(logging::GetLogMessageHandler(), &LogMessageHandler); + DCHECK_NE(tls_.Get() != nullptr, message_list != nullptr); + tls_.Set(message_list); + } + + static ThreadLogMessagesMaster* GetInstance() { + static auto master = new ThreadLogMessagesMaster(); + return master; + } + + private: ThreadLogMessagesMaster() { DCHECK(!tls_.initialized()); tls_.Initialize(nullptr); @@ -45,20 +54,8 @@ class ThreadLogMessagesMaster { logging::SetLogMessageHandler(LogMessageHandler); } - ~ThreadLogMessagesMaster() { - DCHECK_EQ(logging::GetLogMessageHandler(), &LogMessageHandler); - logging::SetLogMessageHandler(nullptr); + ~ThreadLogMessagesMaster() = delete; - tls_.Free(); - } - - void SetThreadMessageList(std::vector* message_list) { - DCHECK_EQ(logging::GetLogMessageHandler(), &LogMessageHandler); - DCHECK_NE(tls_.Get() != nullptr, message_list != nullptr); - tls_.Set(message_list); - } - - private: static bool LogMessageHandler(logging::LogSeverity severity, const char* file_path, int line, @@ -84,18 +81,14 @@ class ThreadLogMessagesMaster { base::ThreadLocalStorage::StaticSlot ThreadLogMessagesMaster::tls_ = TLS_INITIALIZER; -base::LazyInstance::Leaky g_master = - LAZY_INSTANCE_INITIALIZER; - } // namespace -ThreadLogMessages::ThreadLogMessages() - : log_messages_() { - g_master.Get().SetThreadMessageList(&log_messages_); +ThreadLogMessages::ThreadLogMessages() : log_messages_() { + ThreadLogMessagesMaster::GetInstance()->SetThreadMessageList(&log_messages_); } ThreadLogMessages::~ThreadLogMessages() { - g_master.Get().SetThreadMessageList(nullptr); + ThreadLogMessagesMaster::GetInstance()->SetThreadMessageList(nullptr); } } // namespace crashpad diff --git a/util/win/process_info.cc b/util/win/process_info.cc index d05a23d1..2e285b8b 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -19,9 +19,12 @@ #include #include #include +#include #include #include "base/logging.h" +#include "base/memory/free_deleter.h" +#include "base/process/memory.h" #include "base/strings/stringprintf.h" #include "build/build_config.h" #include "util/numeric/safe_assignment.h" @@ -36,6 +39,16 @@ namespace crashpad { namespace { +using UniqueMallocPtr = std::unique_ptr; + +UniqueMallocPtr UncheckedAllocate(size_t size) { + void* raw_ptr = nullptr; + if (!base::UncheckedMalloc(size, &raw_ptr)) + return UniqueMallocPtr(); + + return UniqueMallocPtr(new (raw_ptr) uint8_t[size]); +} + NTSTATUS NtQueryInformationProcess(HANDLE process_handle, PROCESSINFOCLASS process_information_class, PVOID process_information, @@ -347,14 +360,20 @@ bool ReadMemoryInfo(HANDLE process, bool is_64_bit, ProcessInfo* process_info) { std::vector ProcessInfo::BuildHandleVector( HANDLE process) const { ULONG buffer_size = 2 * 1024 * 1024; - std::unique_ptr buffer(new uint8_t[buffer_size]); - // Typically if the buffer were too small, STATUS_INFO_LENGTH_MISMATCH would // return the correct size in the final argument, but it does not for // SystemExtendedHandleInformation, so we loop and attempt larger sizes. NTSTATUS status; ULONG returned_length; + UniqueMallocPtr buffer; for (int tries = 0; tries < 5; ++tries) { + buffer.reset(); + buffer = UncheckedAllocate(buffer_size); + if (!buffer) { + LOG(ERROR) << "UncheckedAllocate"; + return std::vector(); + } + status = crashpad::NtQuerySystemInformation( static_cast(SystemExtendedHandleInformation), buffer.get(), @@ -364,8 +383,6 @@ std::vector ProcessInfo::BuildHandleVector( break; buffer_size *= 2; - buffer.reset(); - buffer.reset(new uint8_t[buffer_size]); } if (!NT_SUCCESS(status)) { diff --git a/util/win/process_info.h b/util/win/process_info.h index 0bb8d735..be968f92 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -170,6 +170,7 @@ class ProcessInfo { bool is_64_bit, ProcessInfo* process_info); + // This function is best-effort under low memory conditions. std::vector BuildHandleVector(HANDLE process) const; pid_t process_id_;