gavinysj / google-breakpad Goto Github PK

Add servlet-api.jar, which was from Apache/Tomcat project. 
A servlet api is necessary to compile airbag/java/common. Although the
runtime library can be other implementations.

The binary file should have the same license (Apache 2.0) as Airbag.

The binary file is skipped by 'svn diff'.

CrashReportSender::SendCrashReport currently calls InternetOpen() with
INTERNET_OPEN_TYPE_DIRECT, which means that no proxy will be used.  This
means that even if a proxy is configured in IE, it won't be used, which
means the connection will probably fail.  I believe you can simply change
this to INTERNET_OPEN_TYPE_PRECONFIG and the code will check the settings
in the registry and attempt to do the right thing.

MSVC turns on /Oy (frame pointer omission) at /O1 and /O2.  Frame pointer
omission, oy!  The present StackwalkerX86 relies on %ebp as the frame
pointer and expects to be able to dereference %ebp to find the caller's
frame pointer.  stackwalker_x86.cc contains this comment:

  // If there is no frame pointer, determining the layout of the stack is
  // considerably more difficult, requiring debugging information.  This
  // stackwalker doesn't attempt to solve that problem (at this point).

Resolving this bug will require three things:

 - PDBSourceLineWriter (the dump_syms tool) will need to get stack frame
   data out of pdb files and write what we need in its output.  We can
   get this data from the pdb's FrameData table, which we access using
   the DIA IDiaFrameData and IDiaEnumFrameData APIs.
 - SourceLineResolver needs to interpret this information and make it
   available.  We can't use a RangeMap for this because of the way that
   the data is organized in a pdb: stack frame information is logically
   keyed by program counter, but blocks of stack frame information
   may enclose other blocks, possibly with identical start or end
   addresses.  (This supports different uses of the stack between
   different parts of the same function, when dictated by control
   structures or compiler optimizations.)  SourceLineResolver may want
   to be renamed.
 - StackwalkerX86 needs to find which module an instruction is in
   (which will require a slight reorganization between Stackwalker
   and the concrete StackwalkerX86) and pull stack frame information
   for an instruction out of that module's debugging information.
   When this information is available, it should be used to locate
   the position of the next stack frame.  When it is not available,
   StackwalkerX86 should continue to use %ebp as the frame pointer.

One open question is exactly how much stack frame information we need to
pull out of the pdb.  We have access to the sizes used on the stack for
locals, saved registers, and parameters.  We also have access to a "program
string," which is something we can feed to a stack machine along with a few
known bits of context and get some information out of at stackwalking time.
   The former is easy and correct when the compiler doesn't push extra
temps onto the stack outside of what we're told it's done in the debugging
info. If additional temps are pushed (i.e. if IDiaFrameData::get_maxStack
returns nonzero), it'll be wrong, though.  I haven't seen nonzero values
for maxStack yet, but I'm building a Firefox optimized build with vc8 now
to see if any turn up there.

Right now Minidump accepts an fd.  This should be changed to a const char*
or string filename.  We agreed that we probably don't need a file
abstraction here for now.

Initial import of a few Java files, asking for a code review.

In crash_report_sender.h:
// TODO(bryner): we should expose the response to the caller.

It would be nice to get a response so the server could tell the client an
ID assigned to the crash report, for example, and then provide a web link
to look it up.

As per 
http://groups.google.com/group/airbag-discuss/browse_thread/thread/ddae205aea494
f73/c1736d1d382e6e7a#c1736d1d382e6e7a

This patch gets rid of those warnings.

It'll be more flexible for library consumers if the report_id is a string
rather than a u_int64_t.  Callers can still use 64-bit ints if they want,
but also have the flexibility to use longer ids.

I've written the x86-specific parts of a stackwalker.  I can't provide a
clean patch yet because it depends on and changes parts of #6 (pending).

It would be cool if minidump_stackwalk would accept input from stdin, maybe
by specifying '-' for the input filename.  Since processing is likely to
occur either a) from a CGI or b) pulling the minidump from a database, it's
silly to have to write the dump data out to a tempfile just to have
minidump_stackwalk read it back in.

Attached is a patch removing the use of UuidToString and consequently
RPC_WSTR, which removes the dependency on rpcrt4.lib.  This makes life a
little easier for me, since Mozilla doesn't link with that lib by default.
 This patch basically just replaces the call to UuidToString with a call to
wsprintf.

Presently, the build system is slightly annoying in that objects,
executables, and libraries are all placed in the root directory of the
package instead of adjacent to the relevant sources.  The automake option
to fix this is "subdir-objects."

Also, maintaining the build system is slightly awkward because configure.ac
references macros that are defined outside the source tree.  We should add
the m4 macros we use to our own repository.

I'm skeptical about the STL_NAMESPACE and GOOGLE_NAMESPACE thing anyway,
because I don't really want to export config.h.  Maybe we should just get
rid of this stuff and assume std and google_airbag everywhere.

We can make it easier to implement an efficient SymbolSupplier if we only
call GetSymbolFile for modules that we haven't already loaded.

In ExceptionHandler::WriteMinidumpWithException, the code builds the full
path to the minidump file, but then passes just the minidump id to the
callback function, requiring me to build the full path myself in my
callback.  Since the full path is necessary to do anything useful anyway,
why not just pass the full path to the callback function?

Right now, the only test for minidump.cc runs minidump_dump on a minidump
file and compares the output to known output.  We should test the code
itself more directly (possibly by providing minidump files or fragments
that test limits?)

The processor should expose a better API to get relevant information about
modules.  MinidumpModule should implement this interface, and users of
modules should not need to bring in the whole Minidump family to use it. 
The API should provide some reasonable and consistent way to get an
identifier, or set of identifiers, for a module and its debug info. 
Specifically, it should consider the uuid if one exists, and the
module/debug file name and versioning info otherwise.  We may also want to
throw some other data into the mix.

We need a StackwalkerPPC implementation to handle ppc minidumps.

The current Windows exception handler runs on the same thread that took the
exception.  If stack space is nearly exhausted (for example, when a crash
is caused by infinite recursion), the exception will not be fully handled.
 MiniDumpWriteDump will not complete.

We want to handle this case.

When an exception occurs, we are able to execute a small amount of code on
the same thread and stack that took the exception.  This code should
transfer control to a background handler thread, or move the stack to some
preallocated buffer of sufficient size.  The former's probably cleaner, and
doable entirely with standard OS calls.  The latter requires CPU-specific asm.

Changing header paths in #includes and #ifdef guards to be relative to src/.

Right now, MinidumpProcessor only produces output for the crashed thread. 
It should process all threads.  I plan on handling this by defining a
struct like this:

struct ProcessState {
  // True if the process crashed, false if the dump was produced outside
  // of an exception handler.
  bool crashed;

  // If the process crashed, the type of crash.  OS- and possibly CPU-
  // specific.  For example, "EXCEPTION_ACCESS_VIOLATION" (Windows),
  // "EXC_BAD_ACCESS / KERN_INVALID_ADDRESS" (Mac OS X), "SIGSEGV"
  // (other Unix).
  string crash_reason;

  // If the process crashed, and if crash_reason implicates memory,
  // the memory address that caused the crash.
  u_int64_t crash_address;

  // If the process crashed, the index of the crashed thread's stack
  // in the threads vector.
  unsigned int crash_thread;

  // Stacks for each thread (except possibly the exception handler
  // thread) at the time of the crash.
  vector<StackFrames> threads;
};

ProcessState should possibly also contain values identifying the OS and
CPU, only because it may help to make sense out of crash_reason and the
context data I'm probably going to need to add to StackFrame or new
subclasses of StackFrame.

What steps will reproduce the problem?
1. go to "Source" tab
2. click on the link to browse svn repository

What is the expected output? What do you see instead?
expecting to see the source tree, but I got 'access denied' message.

The Windows sender should be able to submit crash reports via HTTPS; since
minidumps can contain sensitive information we'd like them to go across the
wire securely.

We should simplily the API for processing minidumps by getting rid of the
mostly-unused CrashReport.  Library consumers may have vastly different
requirements and it's not helpful to try to enforce this structure when we
don't even make use of the members.

I stuck a TODO in SourceLineResolver::Module::ParseStackInfo about
conflicting ranges.  My current thoughts on fixing this are to store
entries in the ContainedRangeMap with base (rva + code_size) and size
(code_size - prolog_size - epilog_size), and introduce the concept of a
"replacement key", which will be the rva.  In a ContainedRangeMap, one
entry may replace another (and inherit any preexisting children) if their
dimensions are identical and the new entry's replacement key is higher than
the existing entry's.  ContainedRangeMap::StoreRange should not return
false when inserting a new entry with identical dimensions to an existing
entry, it should return true but should only modify the map in accordance
with the replacement key.  This will also cover another case of duplication
I've found in dumped symbol files: when code is optimized such that
multiple functions compile to identical code and actually share the same
code, IDiaFrameData will still be present for each function, all completely
identical.

The current behavior of the ContainedRangeMap can be approximated under
this scheme by using a constant for the replacement key during each
StoreRange call.  The only difference is that the return value no longer
indicates whether an entry was stored.

This will be the first step toward processor-side ppc support.

What steps will reproduce the problem?
1. run "make check"

What is the expected output? What do you see instead?

I see:

--- ../src/processor/testdata/minidump1.out     2006-09-06
11:18:24.000000000 -0700
+++ -   2006-09-06 16:17:04.997440000 -0700
@@ -4,7 +4,7 @@
   stream_count         = 8
   stream_directory_rva = 0x20
   checksum             = 0x0
-  time_date_stamp      = 0x44172f15 2006-03-14 16:01:09
+  time_date_stamp      = 0x44172f15 2006-03-14 13:01:09
   flags                = 0x0
 mDirectory[0]
 MDRawDirectory

presumably because the expected result is local time in EDT, and I'm in PDT.

This cleans up some of the rough edges in RangeMap.

Per our discussion, we are not going to provide reporting and symbol server
implementation. This CL removes files under java directory.

Currently, passing a non-existent dump file to SendCrashReport will send a
request anyway, but it's malformed.  There's a check in GenerateRequestBody
for this, but the return value of GenerateRequestBody is ignored in
SendCrashReport.  The simple fix for this is to check the return value
there, and return false out of SendCrashReport if it fails.  This was
biting me because I was mangling my path on the way to the sender.  :-/

Most of the TODO(mmentovai) messages in minidump.cc are about checking
memory allocations.  The Minidump family of classes should provide some
(user-configurable) allocation limits, to avoid over-allocating in the
event of a huge or (worse) malicious minidump.

This is the beginning of airbag's server-side public API.

We can't fill out the StackFrame::function_base field at the moment because
RangeMap::RetrieveRange doesn't tell its caller what the range's base
address is.  This means that code offsets within functions can't be computed.

The range map already stores the base address with each range and can
return it easily.  The size is an arithmatic operation away, and that can
be returned too.  This is only needed for RangeMap, but since that shares
an API with ContainedRangeMap, and it's just as easy to fix
ContainedRangeMap, it should probably be updated as well.

Moving license from Apache 2 to BSD, to allow for inclusion in products
that require GPL-compatibility (sigh).  Google is the sole rights holder of
the code being relicensed.

It does not reproduce!

We've got the types we need to support ppc minidumps now.  Even before the
OS X-specific stuff is ready (we may need to add or change a few
OS-specific structures), we can get the processor-side minidump reader
prepared to read ppc state.

Initial import of a few Java files, asking for a code review.

This is an opaque entity like the report id, but can uniquely identify the
client who sent the report.

Currently the Windows client only builds against the dynamic CRT, we should
provide build configurations which use the static CRT as well.

This patch does that, and also gets rid of the README file, which I decided
was pretty redundant with the documentation in the headers (and less likely
to be kept current).

I've gotten the minidump reader ready for import.  It's tremendous.

- The patch contains only the change to Makefile.am; run automake before
  testing or checking in.
- I couldn't figure out how to get script-based unit tests to run within
  this framework.  "make check" runs "make MinidumpDump_unittest" which
  does nothing because the script already exists, but doesn't include
  MinidumpDump_unittest in the list of tests to run.  If you add it to
  $(TESTS), automake sets up rules to build it from .c source which of
  course does not exist.  So the test is here, but it's never run.
- This isn't done, but it works and the interfaces are roughly where I'd
  like them to be, so it's suitable to get into the tree.

bryner, when you get a chance, could you give me whatever type of review is
appropriate for a huge import?

What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?


What version of the product are you using? On what operating system?


Please provide any additional information below.

I've clarified some of the fields in minidump_format.h and added a couple
more enums.  This should come in handy as we get minidump-writing clients
ready and write processor components for other OSes and CPUs.

SourceLineInfo duplicates some fields of StackFrame, and it should be
possible for the lookup method (which I'd like to rename to
FillSourceLineInfo) to just take a StackFrame as an in/out parameter and
fill in those missing fields.

I was thinking about different ways to test Stackwalk.  One idea I had was
to walk the test process' own stack.  I threw this together, which makes a
bunch of recursive calls to verify that Stackwalker returns the right
number of frames on x86.

SourceLineResolver presently uses its own memory map (MemAddrMap), but #6
introduced a more generic RangeMap.  We can migrate SourceLineResolver to
the new class.

One caveat: currently, MemAddrMap and the file format used by
SourceLineResolver and the Windows dump_syms tool don't take the size/high
address of a range into account, where RangeMap does.

This patch provides a utility to simplify extracting and uploading symbols
from an exe/dll and pdb file on Windows.

MinidumpProcessor should get stacks from all threads from a minidump, not
just the crashed thread.

While integrating airbag into Mozilla, we're setting the exception handler
as early as possible in the startup process, before XPCOM is started, so
we're just using a temp location for the dump path.  We'd like to change it
to the user's profile later once we have that location.  A SetDumpPath()
method on ExceptionHandler would serve nicely.

Need to free the program string after writing a stack info line.

Initial commit of Windows client code for collecting and sending minidumps.