between classic and extension structures.
Previosly, each MinidumpModuleCrashpadInfo structure contained a
minidump_module_list_index field referencing a module in the
MINIDUMP_MODULE_LIST stream by index. This layout was discovered to
cause a problem for the new minidump reader in ModuleSnapshotMinidump.
Instead, the module list index for linkage should be contained in the
MinidumpModuleCrashpadInfoList alongside the LOCATION_DESCRIPTORs
pointing to each MinidumpModuleCrashpadInfo.
The organizational difference is small, but this enables a better design
for ModuleSnapshotMinidump. When initializing a ModuleSnapshotMinidump
with the new layout, it is possible for the caller to have access to the
location descriptor for the MinidumpModuleCrashpadInfo corresponding to
a MINIDUMP_MODULE_LIST. Previously, the caller would not have had this
data without interpreting each MinidumpModuleCrashpadInfo, which
ModuleSnapshotMinidump would have to do anyway.
MinidumpModuleCrashpadInfoListWriter was the only user of
MinidumpLocationDescriptorListWriter, which is obsoleted and removed in
this change. Its functionality is moving directly into
MinidumpModuleCrashpadInfoListWriter, but it is no longer generic enough
to maintain as a distinct class.
TEST=minidump_test \
MinidumpModuleCrashpadInfoWriter.*,MinidumpCrashpadInfoWriter.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/978463003
the Snapshot family.
For the time being, only ProcessSnapshotMinidump::AnnotationsSimpleMap()
is implemented. In order to complete the initial uploader for Crashpad,
ModuleSnapshotMinidump::AnnotationsSimpleMap() is also needed, to be
accessed by ProcessSnapshotMinidump::Modules().
TEST=snapshot_test ProcessSnapshotMinidump.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/932153003
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
Some annotations will exist at a broader scope than per-module, which is
the only place that annotations can currently be stored. The product
name and version are not under the control of any module, but are
established when the first Crashpad client establishes a handler. These
annotations will be stored in a minidump’s MinidumpCrashpadInfo
structure, which applies to the entire minidump.
Within the snapshot interface, this data is carried within the
“process” snapshot because it is the top-level structure in that family.
Note that the data may not correspond directly with a process, however.
TEST=minidump_test MinidumpCrashpadInfoWriter.*
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/924673003
Upload isn’t actually hooked up yet, but this establishes the upload
thread and provides all of the plumbing to process pending reports. For
the time being, SkipReportUpload() is called for all pending reports.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/918743002
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
The PUSH/POP are less noisy for sure. SUPPRESS is a little more
subtle -- it's correctly documented as "for this line and the next"
but that doesn't work well with our coding style.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/898133002
external defined symbols.
Indirect symbols remain unsupported.
Xcode 5.1 ld64-236.3/src/ld/LinkEditClassic.hpp
ld::tool::SymbolTableAtom<>::addGlobal() is responsible for producing
symbols found in the extdef portion of the symbol table. It always sets
the type to N_ABS, N_SECT, or N_INDR, each with the N_EXT bit set. The
N_PEXT bit is never set for non-object files, and we’re not concerned
with reading object files. With this change to tolerate N_INDR, I think
we’re covering all of the symbol types that we might find ld writing to
the extdef portion of the symbol table.
https://groups.google.com/a/chromium.org/forum/#!topic/crashpad-dev/k7QkLwO71ZoR=rsesek@chromium.org
Review URL: https://codereview.chromium.org/901463004
- Stack object instantiated for sizeof to avoid
d:\src\crashpad\crashpad\minidump\minidump_system_info_writer_test.cc(43) : error C2597: illegal reference to non-static member '_MINIDUMP_STRING::Buffer'
Could also just be sizeof(WCHAR) if that feels less ugly.
- narrowing cast
- potentially uninitialize variable warning
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/886143004
MINIDUMP_MISC_INFO and MINIDUMP_MISC_INFO_2, etc. are not
derived from each other in Windows' dbghelp.h, so need
a reinterpret_cast.
arraysize fails on Struct::Member with a big mess (below)
but works ok on a local stack instance.
d:\src\crashpad\crashpad\minidump\minidump_misc_info_writer_test.cc(405) : error C2664: 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>::basic_string(std::initializer_list<_Elem>,const std::allocator<char> &)' : cannot convert argument 1 from 'char' to 'const std::basic_string<char,std::char_traits<char>,std::allocator<char>> &'
with
[
_Elem=char
]
Reason: cannot convert from 'char' to 'const std::basic_string<char,std::char_traits<char>,std::allocator<char>>'
No constructor could take the source type, or constructor overload resolution was ambiguous
d:\src\crashpad\crashpad\minidump\minidump_misc_info_writer_test.cc(408) : error C2784: 'char (&ArraySizeHelper(const T (&)[N]))[N]' : could not deduce template argument for 'const T (&)[N]' from 'unknown'
d:\src\crashpad\crashpad\third_party\mini_chromium\mini_chromium\base\basictypes.h(39) : see declaration of 'ArraySizeHelper'
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/899163004
test_process_snapshot.h apparently requires the full inclusion,
not a forward declaration otherwise it claims to be deleting
undefined types.
And, some more potentially uninitialized variables.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/902803002
Needed on Windows for sys/time.h.
d:\src\crashpad\crashpad\snapshot\test\test_process_snapshot.h(19) : fatal error C1083: Cannot open include file:
'sys/time.h': No such file or directory
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/897973003
Had to move this one out to a scoped_ptr too, otherwise when
it's instantiated in test code on the stack,
d:\src\crashpad\crashpad\minidump\minidump_simple_string_dictionary_writer_test.cc(45) : warning C4815: 'dictionary_writer' : zero-sized array in stack object will have no elements (unless the object is an aggregate that has been aggregate initialized)
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/895313004
A (somewhat cursory) inspection leads me to believe that there's
no particular alignment requirements for this object at this location,
so this warning can be ignored.
d:\src\crashpad\crashpad\minidump\minidump_context_writer.cc(43) : error C2220: warning treated as error - no 'object' file generated
d:\src\crashpad\crashpad\minidump\minidump_context_writer.cc(43) : warning C4316: 'crashpad::MinidumpContextAMD64Writer' : object allocated on the heap may not be aligned 16
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/893393002
- dbghelp.h requires windows.h to be included before it (ick!).
Add a stub one for non_win to make this work.
- convert __attribute__ -> macro that can work work with MSVC;
- a handful of narrowing casts.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/883773005
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
A crash handler needs a way to clean up after itself it it calls
CrashReportDatabase::PrepareCrashReport() to begin writing a new crash
report, but then encounters an error that renders the crash report
unusable. The new ErrorWritingCrashReport() method allows it to
communicate to the database that a previously-prepared crash report
should be removed without ever being promoted to a completed report
pending upload.
TEST=client_test CrashReportDatabaseTest.ErrorWritingCrashReport
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/904533002
- 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
- Dependency on compat required for sys/types.h inclusion for ssize_t.
- Test impl of stat to avoid #error
- FileHandle isn't int on Windows.
client_test no longer links though, as it's still lacking an
implementation of CrashReportDatabase of course.
R=rsesek@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/875043004
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 adds an Info.plist to these two tools that use task_for_pid().
Since they’re flat unbundled executables, the Info.plist must be placed
as a __TEXT,__info_plist section.
By setting SecTaskAccess to allowed and signing these tools with a
certificate trusted by the system, they will be allowed to use
task_for_pid() without requiring the invoking user to be root. This
provides an alternative to installing these tools as setuid root.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/822533002
run_with_crashpad runs a program with a Crashpad exception handler. It
allows a Crashpad exception-handling server to be started and to direct
crashes to it from programs that are not themselves clients of the
Crashpad client library.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/810423004
CrashpadClient is the primary interface for an application to have
Crashpad monitor it for crashes. It contains StartHandler(), which
starts a Crashpad handler process, and UseHandler(), which configures
the process to direct its crashes to a handler process.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/785233011
crashpad_handler is Crashpad’s exception handler server.
Currently, it runs a loop to receive exceptions, and exits when it no
longer has any clients. In the future, this will be extended to write
and potentially upload dumps.
The handler is expected to be started by its initial client via the
CrashpadClient interface.
R=rsesek@chromium.org
Review URL: https://codereview.chromium.org/789693005
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
This picks up:
222feaf3d302 Update README
e38cddb83571 Lock: don’t require NativeThreadType to be a pointer type
8c599712d10c Include What You Use: <string.h> for memmove() and memcpy()
ef991f44ed63 Linux: always build in C++11 mode
BUG=crashpad:6
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/812133002
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
Just avoid the Mac-specific __attribute__ tagging for now. There will need
to be some Windows-specific pragmas added here once the reader has been
written.
R=mark@chromium.org
BUG=crashpad:1
Review URL: https://codereview.chromium.org/808623002