This test was added in https://codereview.chromium.org/1052813002. It
was previously checking the timestamp from in-memory module traversal
vs. the disk mtime. This is flaky (of course) because it depends on
the linker writing the header and closing the file during the same time
quantum. So the bots occasionally failed with:
[ RUN ] ProcessInfo.Self
e:\b\build\slave\chromium_win_dbg\build\crashpad\util\win\process_info_test.cc(86): error: Value of: GetTimestampForModule(GetModuleHandleW(nullptr))
Actual: 1431650338
Expected: modules[0].timestamp
Which is: 1431650337
Instead, use imagehlp to pull the timestamp out of the header so that
it matches the header value that will be the in-memory timestamp.
R=cpu@chromium.orgTBR=mark@chromium.org
Review URL: https://codereview.chromium.org/1139103003
Retrieve context and save to thread context. NtQueryInformationThread
is no longer required (right now?) because to retrieve the CONTEXT, the
thread needs to be Suspend/ResumeThread'd anyway, and the return value
of SuspendThread is the previous SuspendCount.
I haven't handle the x86 case yet -- that would ideally be via
Wow64GetThreadContext (I think) but unfortunately that's Vista+, so I'll
likely need to to a bit of fiddling to get that sorted out. (It's actually
likely going to be NtQueryInformationThread again, but one thing at a
time for now.)
R=cpu@chromium.org, rsesek@chromium.orgTBR=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/1133203002
The next big piece of functionality in snapshot. There's a bit more
grubbing around in the NT internals than would be nice, and it has
made me start to question the value avoiding MinidumpWriteDump. But
this seems to extract most of the data we need (I haven't pulled
the cpu context yet, but I hope that won't be too hard.)
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/1131473005
At the moment the LOGs print something unhelpful like:
[19912:21888:20150501,145958.098:ERROR file_io_win.cc:122] CreateFile 000000C9F8FDE7F0: The system cannot find the file specified. (0x2)
(where the hex string ought to be a file name)
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/1117393002
The main goal was to get the beginnings of module iteration and retrieval
of CrashpadInfo in snapshot. The main change for that is to move
crashpad_info_client_options[_test] down out of mac/.
This also requires adding some of the supporting code of snapshot in
ProcessReaderWin, ProcessSnapshotWin, and ModuleSnapshotWin. These are
partially copied from Mac or stubbed out with lots of TODO annotations.
This is a bit unfortunate, but seemed like the most productive way to
make progress incrementally. That is, it's mostly placeholder at the
moment, but hopefully has the right shape for things to come.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/1052813002
The warnings are emitted when a translation unit attempts to reference
a function whose availability is newer than the deployment target.
BUG=471823
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1109273002
Patch from Erik Chen <erikchen@chromium.org>.
This adds IsExceptionNonfatalResource() and its test, and uses it in
crashpad_handler. When non-fatal resource exceptions are encountered, no
crash report is generated. crashpad_handler swallows these exceptions.
Alternatively, it could allow them to be sent to the system’s host-level
resource exception handler, normally com.apple.ReportCrash.root, which
would allow them to be processed in the same way as when Crashpad is not
in use. I’m not sure which option is better. I chose to swallow them
because there doesn’t appear to be much value in letting
com.apple.ReportCrash.root and spindump look at them.
This also moves ExcCrashRecoverOriginalException() to the new file as a
sibling of IsExceptionNonfatalResource(). This provides better
organization.
BUG=crashpad:35, chromium:474163, chromium:474326
TEST=crashpad_util_test ExceptionTypes.IsExceptionNonfatalResource
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1066243002
ExcServerCopyState() properly sets the new_state and new_state_count
out-parameters for exception handler routines that may deal with
state-carrying exceptions.
This used to exist inline in catch_exception_tool, but that
implementation had a bug caught by the new test.
TEST=crashpad_util_test ExcServerVariants.ExcServerCopyState and others
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1049023003
After 9e79ea1da719, it no longer makes sense for crashpad_util_test_lib
to “hide” in util/util_test.gyp. All of util/test is moved to its own
top-level directory, test, which all other test code is allowed to
depend on. test, too, is allowed to depend on all other non-test code.
In a future change, when crashpad_util_test_lib gains a dependency on
crashpad_client, it won’t look so weird for something in util (even
though it’s in util/test) to depend on something in client, because the
thing that needs to depend on client will live in test, not util.
BUG=crashpad:33
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1051533002
In a future change, crashpad_util_test_lib will gain a dependency on
crashpad_client. This would violate GYP’s prohibition on circular
dependencies between .gyp files, although there would be no circular
relationship between the targets themselves. To overcome this problem,
all test-related targets are moved into their own first-class .gyp
files.
BUG=crashpad:33
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1045173004
Add MapInsertOrReplace<>() to insert a key-value pair into a map if the
key is not already present, or replace the existing value for key if the
key is present. The original value can optionally be returned to the
caller in this case.
Map insertions now use either MapInsertOrReplace<>() or
std::map<>::insert() directly.
Use MapInsertOrReplace<>() when the map should be updated to contain a
mapping from a key to a value regardless of whether the key is already
present.
Use std::map<>::insert() to insert a mapping from a key to a value
without replacing any existing mapping from a key, if present. If it is
important to know whether an existing mapping from a key was present,
use the returned std::pair<>.second. If it is important to know the
existing value, use the returned std::pair<>.first->second.
This change has a slight positive impact on performance.
TEST=crashpad_util_test MapInsert.MapInsertOrReplace and others
BUG=
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1044273002
The test wasn’t strictly broken, but an <= written where a < was
intended caused some threads to log more messages than intended.
BUG=crashpad:26
TEST=crashpad_util_test ThreadLogMessages.Multithreaded
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1047813002
This updates mini_chromium to 91ea4908ffd74d9c886bd2f8ccbfae6d31c499af.
The last five commits listed here are required to support this change.
The mini_chromium update includes:
c1745a924c5c Fix paths to atomicops files in base.gyp from 2f02dcc73536
108e9247189c Add #include of <unistd.h> to close_nocancel.cc
6e4f98a9edf8 Add logging::SetLogMessageHandler()
4063fcb8f460 Add base::ThreadLocalStorage
4870f18a33a6 Add base::LazyInstance
0d31b1f3a289 Fix base/memory/aligned_memory.h for MSVC
91ea4908ffd7 base/logging.h: DCHECK() should always reference its
condition
BUG=crashpad:26
TEST=crashpad_util_test ThreadLogMessages.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1041643003
Towards removing use of open() with O_EXLOCK/O_SHLOCK in code
used on non-BSD.
Adds simple Thread abstraction to util/test.
Includes mini_chromium roll with:
56dd2883170d0df0ec89af0e7862af3f9aaa9be6 Fix import of atomicops for Windows
886592fd6677615c54c4156bb2f2edb5d547ba6c Export SystemErrorCodeToString
R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:1, crashpad:13
Review URL: https://codereview.chromium.org/1001673002
The new call is also used in
CrashReportDatabaseWin::PrepareNewCrashReport(). Previously, that method
used the UUID::InitializeFromBytes() constructor. That actually caused
various fields of the UUID to be byte-swapped so that the ::UUID and
crashpad::UUID would be different UUIDs. Although a UUID is mostly
random, the version field in data_3 is used as a namespace and should be
4 for random UUIDs, and this was not the case under swapping.
TEST=crashpad_util_test UUID.FromSystem
BUG=crashpad:1
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1004913004
This makes it easier for clients to start the Crashpad handler, instead
of requiring them to know how to construct arguments for the handler
themselves. Note in the TEST that -a is no longer required.
TEST=run_with_crashpad --handler crashpad_handler \
--database=/tmp/crashpad_db \
--url=https://clients2.google.com/cr/staging_report \
--annotation=prod=crashpad \
--annotation=ver=0.7.0 \
crashy_program
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1001993002
origin.
This adds AuditPIDFromMachMessageTrailer() to get the process ID of a
Mach message’s sender. Exception messages are considered suspicious when
not sent by the kernel or the exception process.
TEST=crashpad_util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1001943002
disabled.
ClientInfo::set_system_crash_reporter_forwarding() can be used to
disable forwarding. The first module that is found with a non-default
value in this field will dictate whether forwarding is enabled or
disabled. It is possible to enable or disable reporting with this call,
as well as reset it to default, which will allow later modules a chance
to influence the behavior.
ClientInfo::set_crashpad_handler_behavior() is also provided, which can
be used to disable Crashpad’s handling of the exception. Most users
should not call this, but should use Settings::SetUploadsEnabled()
instead.
TEST=crashpad_snapshot_test \
CrashpadInfoClientOptions.*:MachOImageReader.Self_DyldImages; \
run_with_crashpad --handler crashpad_handler \
-a --database=/tmp/crashpad_db \
-a --url=https://clients2.google.com/cr/staging_report \
-a --annotation=prod=crashpad \
-a --annotation=ver=0.7.0 \
crashy_program
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/997713002
NSInputStream requires overriding and implementing private methods in order to
use it with NSURLConnection [1]. It is cleaner to use the private but stable
and open source CFStreamAbstract.h header from CF-Lite to implement a
CFReadStream. Since CFReadStream is toll-free bridged to NSInputStream, the
remainder of the HTTPTransport code can remain unchanged.
[1] http://lists.apple.com/archives/macnetworkprog/2007/May/msg00055.html
BUG=crashpad:15
R=mark@chromium.org
Review URL: https://codereview.chromium.org/993413003
Child exits were using exit(), which caused crashes on 10.10 for
Multiprocess-based tests that ran after HTTPTransport tests. The crashes
occurred while running exit-time destructors. exit() was never correct
in this situation, this should have used _exit() all along.
TEST=crashpad_util_test
BUG=crashpad:17
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/1000653002
optimization.
Newer versions of clang (in this case, trunk r231191) can see through
the pointless division by zero and optimize it away. This caused the
test to hang in release mode.
A 50ms timeout is added to each test to transform the hang into a
failure. The test was split into 12 tests to provide better feedback and
control.
To fix the bug, the division by zero is replaced by __builtin_trap().
TEST=crashpad_util_test ExceptionPorts.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/993613004
This only works 64->64, 32->32, and 64->32. We shouldn't have a
need for 32->64. It will also currently not work if the crash service
is running on Wow64 itself (that is, 32->32, but on an x64 OS). We
should also be able to avoid needing that.
Primarily, this change templatizes the winternl.h process structure
types on word size, so the PEB can be read in a foreign bitsize process.
This also happens to resolve using void* as pointer values into foreign
processes, as they're now all either DWORD or DWORD64 depending on which
traits class is used.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/981393003
Likewise for EXPECT_DEATH_CHECK() and EXPECT_DEATH().
In the in-Chromium build configured for official builds in Release mode,
CHECK() throws away its condition string and stream parameters without
ever printing them, although it still evaluates the condition and
triggers death appropriately. {ASSERT,EXPECT}_DEATH(statement, regex)
will not work correctly for any regex that attempts to match what
CHECK() prints. In these build configurations,
{ASSERT,EXPECT}_DEATH_CHECK() use a match-all regex (""). In other build
configurations, they transparently wrap {ASSERT,EXPECT}_DEATH().
BUG=crashpad:12
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/992693003
When building in the Chromium tree, this swaps out Crashpad’s copies of
mini_chromium, gtest, and gmock for the equivalents provided by
Chromium. A GYP variable, crashpad_in_chromium, is used to determine the
behavior.
gclient doesn’t sync sub-DEPS, so when doing an in-Chromium build,
Crashpad’s copies of mini_chromium, gtest, and gmock are not available.
BUG=crashpad:12
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/986033002
Test code that requires test data should call Paths::TestDataRoot() to
obtain the test data root. This will use the CRASHPAD_TEST_DATA_ROOT
environment variable if set. Otherwise, it will look for test data at
known locations relative to the executable path. If the test data is not
found in any of these locations, it falls back to using the working
directory, the same as the current behavior.
BUG=crashpad:4
TEST=crashpad_util_test Paths.TestDataRoot and others
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/992503002
In Chromium, many targets are built, sharing a single output directory.
Collisions are likely. When integrating Crashpad into Chromium, the
ui/snapshot library and Crashpad’s snapshot library were found to
conflict.
This change gives most Crashpad targets a “crashpad_” prefix to avoid
conflicts. All library and test targets are given a target_name with
this prefix. Existing tools are not likely to conflict with anything
else and are not given a prefix.
BUG=crashpad:12
R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/990553003
When building in the Chromium tree, chromium_code is necessary to apply
Chromium’s build/filename_rules.gypi. Crashpad’s build depends on these
rules. chromium_code also enables a high warning level, which is
desirable for Crashpad.
BUG=crashpad:12
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/986873002
A few stragglers were missing from bc401f6aac22. I really don’t know how
that’s possible, since I thought I was testing it with the correct SDK.
I guess I wasn’t.
BUG=crashpad:11
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/978883007
The 10.10 SDK deprecates <launch.h>, but this file is still useful and
is used by util/mac/launchd.* and util/mac/service_management.*. Wrap
the <launch.h> functions in versions that ignore the deprecation
warnings.
BUG=crashpad:10
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/989733003
Mostly size_t <-> unsigned int warnings, but I also had a mistake in
PROCESS_BASIC_INFORMATION, the pids are 32-on-32 and 64-on-64.
The Windows build is still x86 until https://codereview.chromium.org/981333002/.
I don't think I'll bother maintaining the x86 build for now, though we will probably
need it for x86 OSs in the future. It should be straightforward to revive it once we
need it, and have bots to support it.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/983103004
A couple of the problems related to not having a C++11 library:
- You can’t put const elements into a std::vector<>, so
CrashReportDatabase::GetPendingReports() and
CrashReportDatabase::GetCompletedReports() need to change. There was
no data-safety benefit to const elements.
- std::string::pop_back() does not exist, another mechanism must be
used to trim strings in BreakpadHTTPFormParametersFromMinidump().
One relates to a feature that does not exist in 10.6:
- The O_CLOEXEC flag to open() was introduced in 10.7. Although it
would be possible to use fcntl(..., F_SETFD, FD_CLOEXEC) on 10.6, the
O_CLOEXEC behavior is just removed from
CrashReportDatabaseMac::ObtainReportLock(), in line with other open()
calls in Crashpad.
And one was a real bug:
- #define __STDC_FORMAT_MACROS before #including <inttypes.h> to get
format macros like SCNx32, used in UUID::InitializeFromString().
TEST=* (gyp_crashpad.py -Dmac_sdk=10.6 -Dmac_deployment_target=10.6)
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/987693004
This is as a precursor to ProcessReader. Some basic functionality
is included for now, with more to be added later as necessary.
The PEB code is pretty icky -- walking the doubly-linked list
in the target's address space is cumbersome. The alternative
is to use EnumProcessModules. That would work but:
1) needs different APIs for XP and Vista 64+
2) retrieves modules in memory-location order, rather than
initialization order. I felt retrieving them in initialization order
might be useful when detecting third party DLL injections. In the
end, we may want to make both orders available.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/977003003
The handler is now capable of uploading crash reports from the database.
At present, only one upload attempt is made, and the report will be
moved to “completed” in the database after the attempt, regardless of
whether it succeeded or failed.
The handler also has support to push annotations from its command line
into the process annotations map of each crash report it writes. This is
intended to set basic information about each crash report, such as the
product ID and version. Each potentially crashy process can’t be relied
on to maintain this information on their own.
With this change, Crashpad is now 100% capable of running a handler that
maintains a database and uploads crash reports to a Breakpad-type server
such that Breakpad properly interprets the reports. This is all possible
from the command line.
TEST=run_with_crashpad --handler crashpad_handler \
-a --database=/tmp/crashpad_db \
-a --url=https://clients2.google.com/cr/staging_report \
-a --annotation=prod=crashpad \
-a --annotation=ver=0.6.0 \
crashy_program
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/982613002
implement the new interface.
The upcoming minidump reader will get minidump data from a
FileReaderInterface. For ease of testing, a string-based implementation
is provided. There wasn’t a good reason to have a separate
StringFileReader and StringFileWriter, so I combined them into a single
StringFile.
TEST=util_test StringFile.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/936153002
As there are no extended file attributes available on all Windows file
systems (NTFS supports alternate data streams, but Chrome still supports
running on FAT), instead of using metadata attached to the file, metadata
is stored separately in a simple record-based file and keyed by UUID.
Initially, I attempted a metadata file beside each report, each locked
separately more closely mirroring the Mac implementation. But given the
expected number of of active reports (in the 10s to 100s range?) and the
size of the metadata for each, simply storing it all in one file is much
less complicated when considering error situations.
If the serialization/deserialization becomes a measurable problem, it
could be optimized at some complexity by reading/writing only as
necessary, or optimizing the storage.
R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/867363003
deals solely with a weak FileHandle.
CrashReportDatabase::PrepareNewCrashReport() provides its caller with
both a FileHandle and a FilePath. While it’s possible to create a
FileWriter from the FilePath, it’s not necessary to have two FileHandles
open to the same file. Also, there’s no FileWriteMode::kReuseOrFail
option because it didn’t seem necessary[1], and although it would
actually be the most desirable option for a FileWriter here, allowing
the FileHandle to be used directly without reopening the file sidesteps
the problem entirely.
FileWriter is adapted to use WeakFileHandleFileWriter to minimize
duplication.
[1] https://codereview.chromium.org/818433002/diff/80001/util/file/file_io.h#newcode138R=rsesek@chromium.org, scottmg@chromium.org
Review URL: https://codereview.chromium.org/871193010
- Various "FD" to "Handle"
- Existing Multiprocess implementation moves to _posix.
- Stub implementation for _win.
At the moment, multiprocess_exec_win.cc contains implementations of both
Multiprocess methods and MultiprocessExec functions. This will need more
work in the future, but reflects the idea that all tests should be in
terms of MultiprocessExec eventually.
Currently, this works sufficiently to have util_test succeed (including
multiprocess_exec_test, and the recently ported HTTPTransport tests.)
R=mark@chromium.org
BUG=crashpad:1, crashpad:7
Review URL: https://codereview.chromium.org/880763002
off_t exists on Windows, but Seek is implemented in terms of
SetFilePointerEx which expects a LONGLONG, so FileOffset is LONGLONG.
So, use FileOffset in the test code so that it wraps at the expected
value.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/854883002
d:\src\crashpad\crashpad\util\file\string_file_writer_test.cc(367) : warning C4244: 'initializing' : conversion from 'const uint64_t' to 'const off_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/836413004
This function will be useful in upcoming non-test code. Because the
first Crashpad client that wants a Crashpad handler will now be
responsible for starting the handler process, this will prevent file
descriptors from leaking to the handler process.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/819483002
There will be no reason to leave the handler process connected to its
invoker’s stdin or stdout.
On the other hand, I’m currently leaving it connected to the original
and stderr, as these may be useful for diagnostics.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/818573002
I started (https://codereview.chromium.org/812403002/) emulating oflag values
on Windows in FileWriter, but it seemed awkward. On the assumption that we're
only likely to need "read a file" and "write a file" this seemed simpler, and
sufficient (but I don't know if that's necessarily true).
Users of open are not yet switched.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/818433002
Otherwise, when assigning to a smaller type, MSVC warns e.g.
d:\src\crashpad\crashpad\util\numeric\safe_assignment.h(38) : error C2220: warning treated as error - no 'object' file generated
d:\src\crashpad\crashpad\util\file\string_file_writer.cc(127) : see reference to function template instantiation 'bool crashpad::AssignIfInRange<size_t,FileOffset>(Destination *,Source)' being compiled
with
[
Destination=size_t
, Source=FileOffset
]
d:\src\crashpad\crashpad\util\numeric\safe_assignment.h(38) : warning C4244: '=' : conversion from 'FileOffset' to 'size_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/809303003
Rename fd_io to file_io, and ReadFD to ReadFile, etc.
file_io.cc is the higher level versions that call the basic ReadFile/WriteFile
and then file_io_posix.cc and file_io_win.cc are the implementations of
those functions.
The Windows path is as yet untested, lacking the ability to link the test binary.
R=cpu@chromium.org, mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/811823003
There were many call sites that wasted a few lines on
mach_port_allocate() and sticking the result into a scoper. I was about
to add three more, so I took the opportunity to simplify things.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/809103002
Else,
d:\src\crashpad\crashpad\util\net\http_multipart_builder.cc(50) : warning C4244: 'initializing' : conversion from 'uint64_t' to 'int', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/796643006
This DCHECK fails for me locally as
[----------] 3 tests from ProcessInfo
[ RUN ] ProcessInfo.Self
[70989:10546846:20141216,112509.948519:FATAL process_info_mac.cc:114] Check failed: static_cast<size_t>(ngroups) < (sizeof(ArraySizeHelper(kern_proc_info_.kp_eproc.e_ucred.cr_groups))) (16 vs. 16).
Abort trap: 6
It doesn't seem to happen on the waterfall, so maybe I'm building against
an incorrect header? I don't particularly understand the code, but assuming
it's normal 0-based array, perhaps it should be a DCHECK_LE in any case.
R=mark@chromium.org
Review URL: https://codereview.chromium.org/813473002
CXX_LIBRARY_VERSION is irrelevant, because the only C++11 library
feature of any concern is whether numeric_limits’ min() and max() are
declared constexpr.
Crashpad is C++11-only as far as the language is concerned, and the
comment doesn’t need to call it out explicitly because static_assert()
is always available.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/809833002
e.g.
FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC @obj\util\misc\util_test.clock_test.obj.rsp /c ..\..\util\misc\clock_test.cc /Foobj\util\misc\util_test.clock_test.obj /Fdobj\util\util_test.cc.pdb
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(49) : error C2220: warning treated as error - no 'object' file generated
d:\src\crashpad\crashpad\util\misc\clock_test.cc(72) : see reference to function template instantiation 'To implicit_cast<uint64_t,double>(const From &)' being compiled
with
[
To=uint64_t
, From=double
]
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(49) : warning C4244: 'return' : conversion from 'const double' to 'uint64_t', possible loss of data
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/807653002
e.g.
d:\src\crashpad\crashpad\util\numeric\in_range_cast.h(35) : warning C4244: 'return' : conversion from 'unsigned __int64' to 'uint32_t', possible loss of data
d:\src\crashpad\crashpad\util\numeric\in_range_cast.h(35) : warning C4244: 'return' : conversion from '__int64' to 'int32_t', possible loss of data
d:\src\crashpad\crashpad\util\numeric\in_range_cast_test.cc(54) : see reference to function template instantiation 'Destination crashpad::InRangeCast<int32_t,__int64>(Source,Destination)' being compiled
with
[
Destination=int32_t
, Source=__int64
]
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/800073003
Unfortunately VS2013's support of C++11 is partial. It supports the
extended union definition, but does not fully support constexpr.
So, update some locations where CXX_LIBRARY_VERSION is used where
toolchain support is lacking. It works correctly in the locations
where std::is_standard_layout is used.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/803273002
When run from the wrong location and test data or other test programs
can’t be found, the tests should fail with gtest assertions. The test
executable should not crash.
BUG=crashpad:4
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/799083003
The implementations for the exc and mach_exc subsystems were nearly
identical, and were a good target for templatization. The existing
split between exc and mach_exc was a good candidate for unification
based on CompositeMachMessageServer instead of the custom unification
previously done in UniversalMachMessageServer.
TEST=util_test ExcServerVariants.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/766193006
MachMessageServer::Run()’s distinct |nonblocking| parameter is removed.
The information it formerly conveyed is now implied by the |timeout_ms|
parameter, which can accept two special values,
kMachMessageTimeoutNonblocking and kMachMessageTimeoutWaitIndefinitely.
TEST=client_test, snapshot_test, util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/777993002
UniversalMachExcServer provided both an interface and an implementation,
contrary to the other classes in the exc_server_variants family. This
was mostly done for reasons of economy in an already-large class family.
Unfortunately, this decision meant that it was impossible for other code
to use UniversalMachExcServer, which required that CatchMachException()
be implemented, and also extend another class without violating the
style guide’s prohibition of multiple implementation inheritance. This
became a problem in a lot of test code, which extended MachMultiprocess
and UniversalMachExcServer.
UniversalMachExcServer is now given its own nested Interface class,
which is a pure interface. All users of UniversalMachExcServer are
changed from “is-a” UniversalMachExcServer to “has-a”
UniversalMachExcServer and “is-a” UniversalMachExcServer::Interface.
TEST=client_test, snapshot_test, util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/775943005
This exposed a bug in the ExcClientVariants test, which was expecting
the memory used for new_state to be initialized with zeroes. In reality,
no guarantee of initialization is made. MIG “out” parameters are
strictly “out” and may contain garbage at function entry.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/779633004
The F_SETNOSIGPIPE fcntl() command is not available on 10.6. Use
socketpair() instead of pipe(), so that the SO_NOSIGPIPE socket option
can be used.
TEST=util_test ChildPortHandshake.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/777573002
MachMessageWithDeadline() is a mach_msg() wrapper that deals with
deadlines instead of timeouts. It is a slight simplification of the
mach_msg() interface because the deadline parameter implies the timeout
option bits, and because the caller does not need to specify send_size
during sends as the message itself already carries this information.
TEST=util_test MachMessage.MachMessageDeadlineFromTimeout
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/773943002
A subsequent change will add MachMessageWithDeadline(), a mach_msg()
wrapper. Conceptually, it makes sense to include that function in this
file family. Since this file family now contains a mach_msg() wrapper,
it makes sense to rename it mach_message and lose the _util suffix.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/772133004
MachMessageServer was wasteful with allocations for request and reply
messages. It allocated new memory for each request receive and for each
reply send, and if it needed to resize an allocation for a request, it
would maintain two request allocations simultaneously. The new behavior
allocates memory for a new request only if it needs a different size
than for the previous request, and it never maintains two request
allocations simultaneously. Memory for a reply is allocated once per
method invocation and maintained, since this never needs to be resized.
One pass of the loop is now guaranteed, even if a caller specifies a
very small timeout that expires before attempting to receive a message.
An infinite looping bug that could occur when ignoring large messages
has also been fixed.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/759023004
The buffer sizing logic was correct to start with. I don’t know why I
misread it. It should say “if this would resize to receive a large
message, use the entire allocation rounded up to full page size,
otherwise, only use the space expected for a message.”
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/760573003
These port dispositions were naïvely taken from excUser.c and
mach_excUser.c, but the local and remote portions were not swapped as
they would be upon receipt in a server. This swaps them to match how
they’d be visible in a server, and uses the port disposition name
aliases expected to be used in servers: MACH_MSG_TYPE_PORT_* instead of
MACH_MSG_TYPE_{MAKE,COPY,MOVE}_*.
TEST=util_test ExcServerVariants.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/755323002
The existing implementation used the same logic as is found in
mach_msg_server(), but that logic seems incorrect. When the caller wants
to retry a mach_msg() receive of a too-large message that returns
MACH_RCV_TOO_LARGE, there’s no harm in attempting the receive with a
larger buffer initially. On the other hand, if the caller does not want
to retry such mach_msg() receive attempts, it’s an indication that the
caller is expecting to be intolerant of too-large messages, and there’s
no need to attempt the receive with a buffer any larger than requested.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/753363003
As documented, MACH_SEND_TRAILER would allow a sender to provide its own
message trailer instead of having the kernel append its own
kernel-generated trailer. This is a Mach feature that supports a network
of multiple Mach hosts, but even in that environment, the option is
restricted to use by privileged callers. In reality, MACH_SEND_TRAILER
has never been implemented in OS X.
The system’s mach_msg_server() family does consider the value of
MACH_SEND_TRAILER, but this is pointless. Any purported trailer set by a
server function would be ignored.
Maintaining this code gives the illusion that it’s functional, so it’s
being removed.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/736493007
ChildPortHandshake is the most generic system yet to allow child
processes to provide their parents with Mach rights. These are
ordinarily expected to be send rights to the children’s own task ports,
or send rights to servers that the children hold receive rights to.
This updates DEPS to pull mini_chromium 1d3523dbda93, which includes
base::mac::ScopedMachPortSet.
TEST=util_test ChildPortHandshake.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/756603003
Previously, MachMessageServer::Run() only provided two strategies for
dealing with large messages, indicated by mach_msg() returning
MACH_RCV_TOO_LARGE: the receive buffer could be reallocated and the
message received, or the entire function could return MACH_RCV_TOO_LARGE
to the caller. There are situations where an intermediate behavior might
be desirable. This intermediate behavior would allow the function to
continue waiting for another message without returning an error to the
caller or attempting to receive the large message. This is desirable
when dealing with fixed-sized messages and a receiver that might be sent
messages by unknown, possibly-malicious callers. This can happen when
the corresponding send right is published with the bootstrap server, for
example.
Existing users continue to request their existing behavior, typically
receiving an error when encountering a large message.
catch_exception_tool will use the new “ignore” behavior when running in
persistent mode.
TEST=util_test MachMessageServer.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/756803002
the child_port subsystem.
Common routines shared with the ExcServer family of classes have been
moved to a new file, where they can be shared between different
MachMessageServer::Interface implementations.
TEST=util_test ChildPortServer.*:MachMessageUtil.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/754123002
DropPrivileges() is used in exception_port_tool, so that when it is
installed as a setuid executable, it only uses elevated privileges to
obtain a task port for its -p option, and then relinquishes those
privileges.
It is difficult to provide a test for this function, because it must be
running setuid or setgid in order to do anything interesting. However,
the function contains its own CHECKs to verify that it behaves properly.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/727053002
This also transitions exception_port_tool to use TaskForPID(), so that
it can be safely used as a setuid executable without giving permission
to operate on any process on the system.
It is difficult to provide a test for this function, because it must be
running setuid root in order to do anything interesting.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/728973002
Also, move ProcessArgumentsForPID() into ProcessInfo.
This change prepares for a TaskForPID() implementation that’s capable of
operating correctly in a setuid root executable. TaskForPID() belongs in
util/mach, but for its permission checks, it must access some process
properties that were previously fetched by ProcessReader in snapshot.
util can’t depend on snapshot. The generic util-safe process information
bits (Is64Bit(), ProcessID(), ParentProcessID(), and StartTime()) are
moved from ProcessReader to ProcessInfo (in util), where the current
ProcessReader can use it (as it’s OK for snapshot to depend on util),
and the future TaskForPID() in util can also use it. ProcessInfo also
contains other methods that TaskForPID() will use, providing access to
the credentials that the target process holds. ProcessArgumentsForPID()
is related, and is also now a part of ProcessInfo.
TEST=snapshot_test, util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/727973002
implicit_cast<> only performs a cast in cases where an implicit
conversion would be possible. It’s even safer than static_cast<> It’s an
“explicit implicit” cast, which is not normally necsesary, but is
frequently required when working with the ?: operator, functions like
std::min() and std::max(), and logging and testing macros.
The public style guide does not mention implicit_cast<> only because it
is not part of the standard library, but would otherwise require it in
these situations. Since base does provide implicit_cast<>, it should be
used whenever possible.
The only uses of static_cast<> not converted to implicit_cast<> are
those that require static_cast<>, such as those that assign an integer
constant to a variable of an enum type.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/700383007
This only came up in one location, PointerVector.
A template alias is superior to inheritance, which doesn’t provide full
type equivalence and doesn’t automatically inherit non-default
constructors.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/683753005
Not doing this causes gtest to issue these warnings:
[WARNING] ../../third_party/gtest/gtest/src/gtest-death-test.cc:825::
Death tests use fork(), which is unsafe particularly in a threaded
context. For this test, Google Test detected 4 threads.
The gtest documentation recommends giving the test case a name ending in
DeathTest. Test cases named according to this convention run before all
other tests. Other death tests in Crashpad also follow this convention.
https://code.google.com/p/googletest/wiki/AdvancedGuide#Death_Tests_And_Threads
TEST=util_test HTTPMultipartBuilderDeathTest.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/694963002
This change was generated mechanically by running:
find . \( -name \*.cc -or -name \*.mm -or -name \*.h \) \
-and -not -path ./third_party/\* -and -not -path ./out/\* \
-exec sed -i '' -E -e 's/(^|[^_])NULL/\1nullptr/g' {} +
Further manual fix-ups were applied to remove casts of nullptr to other
pointer types where possible, to preserve the intentional use of NULL
(as a short form of MACH_PORT_NULL) in exception_port_tool, and to fix
80-column violations.
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/4mijeJHzxLg/discussion
TEST=*_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/656703002
This uses THREAD_NULL, TASK_NULL, and HOST_NULL in preference to
MACH_PORT_NULL and kMachPortNull. These constants are correctly-typed
(thread_t, task_t, and host_t) and result in more readable source code,
especially where thread and task parameters appear together as they do
in exc_*_variants.
TEST=util_test
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/649713002
