Closed Bug 1002564 Opened 6 years ago Closed 2 months ago

All crashes with reason EXC_BAD_ACCESS / 0x0000000d have a 32-bit crash address

Categories

(Toolkit :: Crash Reporting, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1035892
mozilla32

People

(Reporter: smichaud, Assigned: smichaud)

References

Details

Attachments

(3 files, 1 obsolete file)

This bug is spun off from bug 997908, where I noticed a case of Socorro reporting a "crash address" of '0' for a crash stack where this can't possibly be true.  See particularly bug 997908 comment #35 and bug 997908 comment #36.

Search as follows in Socorro for all crashes whose reason is "EXC_BAD_ACCESS / 0x0000000d", then click on the "Address facet" tab.  You'll notice that there are no values for "crash address" that are larger than the maximum size for a 32-bit unsigned integer.

https://crash-stats.mozilla.com/search/?reason=%3DEXC_BAD_ACCESS+%2F+0x0000000d&cpu_name=amd64&_facets=signature&_facets=address&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=address

Now search as follows for all crashes whose reason is "EXC_BAD_ACCESS / KERN_INVALID_ADDRESS" and click on the "Address facet" tab.  Notice that some of the address are large enough that they can only be 64-bit addresses.

https://crash-stats.mozilla.com/search/?reason=%3DEXC_BAD_ACCESS+%2F+KERN_INVALID_ADDRESS&cpu_name=amd64&_facets=signature&_facets=address&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=address

I don't know whose fault this is -- it may be Apple's.

I first thought that we might be storing the "crash address" for this kind of crash in a 32-bit unsigned integer.  A quick look through the Breakpad code in the Mozilla tree shows that we always store the "crash address" in a 64-bit unsigned integer.  But I *have* found one place in the code where we might be constraining it to 32-bits.  (More about this in a later comment.)
"EXC_BAD_ACCESS / 0x0000000d" should really be "EXC_BAD_ACCESS / KERN_ABORTED".  KERN_ABORTED is defined in mach/kern_return.h, and has the following comment:

  The operation was aborted.  Ipc code will catch this and reflect it as a message error.

From this comment and from a couple of code samples I've been able to find (as follows), this error seems to have to do with mach messaging and mach ports, and particularly with an operation being interrupted.

http://opensource.apple.com/source/libdispatch/libdispatch-84.5.1/src/semaphore.c
http://golang.org/src/pkg/runtime/os_darwin.c

I can't find any reason why the "crash addresses" associated with these failures should be 32-bit only.  But it's possible there's some underlying Apple bug.
But ...

Here's code where (on the Mac) Breakpad sets the "exception address" to the "exception subcode" when the "exception type" is EXC_BAD_ACCESS:

https://hg.mozilla.org/mozilla-central/annotate/cfde3603b020/toolkit/crashreporter/google-breakpad/src/client/mac/handler/minidump_generator.cc#l1033

And here's code where the "exception subcode" is stored in an int32_t:

https://hg.mozilla.org/mozilla-central/annotate/cfde3603b020/toolkit/crashreporter/google-breakpad/src/client/mac/crash_generation/crash_generation_server.h#l54
There are a few places where this could break down:
* Breakpad storing the data in the minidump
* minidump_stackwalk formatting the data for output

The former happens here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/google-breakpad/src/client/mac/handler/minidump_generator.cc#1033

The latter happens here:
https://code.google.com/p/google-breakpad/source/browse/trunk/src/processor/minidump_stackwalk.cc#722

and the accessor it's using is defined here:
https://code.google.com/p/google-breakpad/source/browse/trunk/src/google_breakpad/processor/process_state.h#102

The latter both look sensible, it's using a 64-bit format string and passing a 64-bit int.
Note that crash_generation_server is the out-of-process dump implementation.
> Note that crash_generation_server is the out-of-process dump implementation.

Do we ever use that on the Mac?
When content processes are in play, so for out-of-process plugins right now (and e10s in the future).
Thanks!

Content processes *are* in play at bug 997908 (for the Flash and Silverlight plugins).
Attached patch Possible fixSplinter Review
So this might help.

I've started a tryserver build:
https://tbpl.mozilla.org/?tree=Try&rev=5b73f60ace92
Attachment #8413873 - Flags: review?(ted)
Comment on attachment 8413873 [details] [diff] [review]
Possible fix

Review of attachment 8413873 [details] [diff] [review]:
-----------------------------------------------------------------

Well that's certainly a tiny fix! If this fixes the issue let me know and I'll land it upstream.
Attachment #8413873 - Flags: review?(ted) → review+
Comment on attachment 8413873 [details] [diff] [review]
Possible fix

Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab6da1212288

Let's see what happens with the supposedly null-dereference Mac crash stacks for bug 997908.  I suspect that none of the recent ones (those that have increased recently) really are null-dereferences.  Since bug 997908 is a topcrasher, we should be able to tell fairly quickly -- within a few days.
https://hg.mozilla.org/mozilla-central/rev/ab6da1212288
Assignee: nobody → smichaud
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Should I just preemptively land this upstream or did you want to wait and see if it actually fixes the issue? It looks trivially correct on its face.
Flags: needinfo?(smichaud)
It *does* look trivially correct ... but it doesn't seem to have fixed the issue with the Socorro stacks for bug 997908:  All the recent Mac crashes (those in builds containing the patch) are still reported as NULL-dereferences, even though they can't possibly be (as best I can tell).

I've now thought of more tests I can run to try to get to the bottom of this (for example to find out if this is an Apple bug).  Let me do them and get back before we decide either way.
Flags: needinfo?(smichaud)
There seems to be a bewildering variety of bugs here :-(

1) An Apple bug that zeros out all crash addresses > 0x7fffffffffff.
2) A Firefox Breakpad bug (or Socorro bug?) that truncates crash addresses to the lowest 32 bits, but (under some circumstances) sign-extends them.
3) A Google Chrome bug that truncates crash addresses to the lowest 32 bits.

I mention Chrome's behavior because its crash reporter may be related to Breakpad (though it uses Apple's own crash report dialog to display crash information).

So the problems here are much deeper than I first thought, and I'm not sure when (or if) we'll be able to fix them.  But let me play a bit more with Breakpad code to see what I can accomplish.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ted, let's not upstream my patch for the time being.  But let's also leave it on trunk (for the time being).
Here's the interpose library I used to do my tests.  It has instructions for building and use.  But you also need to know the following:

The interpose library "interposes" a handler for an unused signal (the WINCH signal, '28') that causes a crash by deliberately dereferencing a bad pointer.  Once a process has loaded the library, you can get it to crash by doing "kill -28 [pid]" from a Terminal prompt.

Play with the bad pointer values to see what happens in different apps.  I tested with today's m-c nightly, Google Chrome, Safari and the Calendar app (the last of which I'm hoping is an example of a plain-vanilla Cocoa app).
> the Calendar app

No, it was the Calculator app.
Just now I did a "super search" of *all* crashes over the last week with a crash address "facet":

https://crash-stats.mozilla.com/search/?_facets=signature&_facets=address&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform

I couldn't find any, on any platform, where the address was longer than 32-bits (save where it was sign-extended with 0xfffffffff in the top 32 bits).

Could this be a Socorro problem, as opposed to a Breakpad one?  Ted, do you know where the Socorro code lives (the server-side stuff)?
Flags: needinfo?(ted)
(Following up comment #14)

> 3) A Google Chrome bug that truncates crash addresses to the lowest 32 bits.

Oops, forgot that the Google Chrome main process is 32-bit only on the Mac.
> I couldn't find any, on any platform, where the address was longer than 32-bits (save where
> it was sign-extended with 0xfffffffff in the top 32 bits).

Oops, not true.  Here are some examples on 64-bit Linux of crash addresses that are larger than 32 bits *without* sign extending:

https://crash-stats.mozilla.com/search/?address=%3D0x500000090&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform
https://crash-stats.mozilla.com/search/?address=%3D0x100000022&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform

So this is presumably *not* a Socorro bug.

Still, though, I'd like to know where the Socorro code lives.
(In reply to Steven Michaud from comment #14)
> There seems to be a bewildering variety of bugs here :-(
> 
> 1) An Apple bug that zeros out all crash addresses > 0x7fffffffffff.
> 2) A Firefox Breakpad bug (or Socorro bug?) that truncates crash addresses
> to the lowest 32 bits, but (under some circumstances) sign-extends them.
> 3) A Google Chrome bug that truncates crash addresses to the lowest 32 bits.
> 
> I mention Chrome's behavior because its crash reporter may be related to
> Breakpad (though it uses Apple's own crash report dialog to display crash
> information).


FWIW, Chrome does use Breakpad for crash reporting.

Socorro grabs the crash address out of the stackwalker output here:
https://github.com/mozilla/socorro/blob/master/socorro/processor/hybrid_processor.py#L1007

The stackwalker code that prints that line is here:
https://github.com/mozilla/socorro/blob/master/minidump-stackwalk/stackwalker.cc#L610
Flags: needinfo?(ted)
While trolling around the web for clues, I found this:
http://people.mozilla.org/~vladimir/misc/breakpad-64-bit-exc.patch

Vlad, by any chance is this ready to go? :-)
Flags: needinfo?(vladimir)
From an IRC log in 2013-03:

# [18:01] <vlad> ted: passed?
# [18:01] <@ted> vlad: yeah
# [18:01] <vlad> ted: yay!
# [18:02] <@ted> vlad: certainly comforting
# [18:02] <@ted> vlad: so i think i'd still like a second opinion on this from mento
# [18:02] <@ted> and also i think you should just default to using these codepaths on x86-64
# [18:03] <@ted> (if we could write a unittest that shows that we're currently broken that would be ideal)
# [18:03] <vlad> ted: I could probably create one easily
# [18:03] <vlad> if I could run them :/
# [18:03] <@ted> vlad: yeah :-/
# [18:03] <vlad> ted: just deref something with an address > 32-bits
# [18:03] <@ted> vlad: seems pretty plausible
# [18:03] <vlad> the bad address it gets currently will be truncated
# [18:03] <vlad> I was doing that to test, just in gecko
# [18:03] <vlad> howeve!
# [18:03] <vlad> er however!
# [18:03] <vlad> I think such a test will keep failing
# [18:04] <vlad> because I don't think the rest of the infrastructure handles 64-bit values
# [18:04] <@ted> vlad: rest of what infrastructure?
# [18:05] <vlad> ted: WriteMinidumpWithException just uses int for exception_code/subcode
# [18:05] <vlad> and passes an int down to the directCallback_
# [18:05] <@ted> vlad: ah
# [18:05] <@ted> vlad: yeah, that's crappy
# [18:05] <vlad> which also only takes ints
# [18:05] <@ted> i guess we could fix those to be int64_t
# [18:05] <@ted> a little more invasive
# [18:06] <vlad> yeah
# [18:06] <vlad> but necessary, really
# [18:06] <vlad> ted: can you contact mento? or shall I? (if so what's the right way to do so?)
# [18:07] <@ted> vlad: can you rejigger your patch to default to this on x86-64? if you do that then i can push it for upstream review
# [18:07] <vlad> ted: sure
# [18:08] <@ted> ok, then we can see what he thinks
# [18:11] <vlad> ted: http://people.mozilla.com/~vladimir/misc/breakpad-64-bit-exc.patch

Sooo... Ted, did you see what mark thought? :)  Also note that issues back then about the rest of breakpad infrastructure only handling 32-bit (int) values.  So this patch was correct, but insufficient.
Here's a patch for this bug that (in my tests) fixes problem #2 from comment #14.

I based it on your patch, Vlad, plus whatever I could find out from other sources (more below).  But I didn't include your unrelated fixes for ExceptionHandler::WaitForMessage() (I'll put those in a separate patch).  And I made Breakpad (and the OS) use 64-bit "subcodes" in both 64-bit and 32-bit mode.  This works just fine in 32-bit mode (which I tested), and it makes the code simpler.

The "other sources" I consulted are the following plus Apple's source for their implementation of xnu (available at http://opensource.apple.com/).

http://stackoverflow.com/questions/2824105/handling-mach-exceptions-in-64bit-os-x-application

Turns out the only real documentation for a lot of the stuff used by Breakpad to make the OS feed it exceptions (task_set_exception_ports() and friends) is in Apple's xnu source code.  It even has some html-format man pages.

Ted and Vlad, what do you think of this approach?  Please let me know, and please feel free to turn my "needinfos" into requests for review.

I'd like to get this (or something like it) landed on the trunk as soon as possible.  Upstreaming it will take longer, and I'm willing to spend time on that myself (to open bugs with Google) -- if you guys will tell me where I need to do that.  Whatever bugs I open, I'll be sure to CC both of you and Mark Mentovai.

I've started a tryserver build for this patch:
https://tbpl.mozilla.org/?tree=Try&rev=b93830179c78

This patch doesn't fix problem #1 from comment #14.  That's clearly an Apple bug (since it also effects Safari and other apps that use Apple's crash reporting mechanism).  But the bug is probably in xnu code (for which we have the source) -- so I'll likely be able to figure it out.  And I may even be able to find a way to work around it (in Breakpad).  If nothing else, though, I'll be able to report the bug to Apple.
Attachment #8425010 - Flags: feedback?(vladimir)
Attachment #8425010 - Flags: feedback?(ted)
One possible problem:

I don't know if my patch for Breakpad works in IOS, and I have no way of testing it.

But I also don't know if Google really cares about making Breakpad work on IOS.

(Of course that doesn't matter for us.)
Flags: needinfo?(vladimir)
I've started another tryserver build, though with the same patch as from comment #6:
https://tbpl.mozilla.org/?tree=Try&rev=2c238fd53d91

Apparently bug 1010783 has been causing all kinds of grief on recent m-c nightlies and my previous tryserver build.  But that bug has now been fixed on trunk.
> though with the same patch as from comment #6

comment #26
No longer blocks: 997908
> 1) An Apple bug that zeros out all crash addresses > 0x7fffffffffff.

This is what I've been calling "problem #1 from comment #14".  It appears to be even deeper than an Apple bug -- to be the consequence of how Intel processors work.  So it's going to be difficult to fix, and maybe impossible.

I think I've found where in xnu code (in the kernel) the crash address gets NULLed out before it gets passed to Breakpad.  Visit the following page and search on "case EXC_I386_GPFLT:".

http://opensource.apple.com/source/xnu/xnu-2050.48.11/bsd/dev/i386/unix_signal.c

You'll see that sinfo64.si_addr gets NULLed out in the case of a SIGSEGV signal whose "code" is EXC_I386_GPFLT (as opposed to KERN_PROTECTION_FAILURE or KERN_INVALID_ADDRESS).  This code relies on the crash address being in the CR2 register (http://en.wikipedia.org/wiki/Control_register#CR2).  But a comment says that "CR2 is meaningless after GP fault".

Testing with an app that (like Safari or Calculator) uses Apple's crash reporting system, you'll see that every time the crash address is > 0x7fffffffffff, you'll get an exception of type "EXC_BAD_ACCESS (SIGSEGV)" whose code is EXC_I386_GPFLT.  With "lower" crash addresses the code is always something else (for example KERN_INVALID_ADDRESS).
Also visit the following URL and search on "code = EXC_I386_GPFLT":

http://opensource.apple.com/source/xnu/xnu-2050.48.11/osfmk/i386/trap.c

The comment above this code gives us some hope:

  * The trouble is cr2 doesn't contain the faulting address;
  * we'd need to decode the faulting instruction to really
  * determine this. We'll leave that to debuggers.

But frankly I don't yet know how to do this.
(In reply to Steven Michaud from comment #30)
> This is what I've been calling "problem #1 from comment #14".  It appears to
> be even deeper than an Apple bug -- to be the consequence of how Intel
> processors work.  So it's going to be difficult to fix, and maybe impossible.

Hmm, if there's something there deeply rooted in how Intel processors work, does that mean that bug 974420 is related?
> does that mean that bug 974420 is related?

Yes, I think it does.
See Also: → 974420
Attached patch Yet another possible fix (obsolete) — Splinter Review
Here's a variation on my take2 patch that (indirectly) does something about problem #1 from comment #14:  It gets (from the kernel) the "thread state" as of a crash (the current values of all the user-level registers).

Though Breakpad minidumps currently store *some* register values (which aren't displayed in Socorro, but can be read using minidump_stackwalk), they don't store them *all*.  And that's not enough to work around "problem #1 from comment #14".

If (on an Intel CPU) you try to dereference an "address" > 0x7fffffffffff, you get a "general protection fault" (as opposed to a "page fault").  The reason is that such a value isn't actually a valid address (because the address space for Intel 64-bit CPUs isn't actually 64-bits wide).  Only with a "page fault" does the CPU store a "crash address" in the CR2 register (and does the kernel pass it to Breakpad).

But almost certainly one of the "thread state" registers contains the "address" that, when dereferenced, caused the GPF.  So seeing them all may allow you to guess/infer which one it was.

Currently, Breakpad has no way to store this information in a minidump.  So this patch just logs the "thread state" to the console (if you're running from Terminal) and to the syslog (/var/log/system.log).  It's a proof-of-concept patch, and one which I hope will be useful to figuring out bug 997908.

Here's a tryserver build made from this patch:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-6b8002b98fd5/try-macosx64/firefox-32.0a1.en-US.mac.dmg
(In reply to Steven Michaud from comment #34)
> Created attachment 8430360 [details] [diff] [review]
> Yet another possible fix
> 
> Here's a variation on my take2 patch that (indirectly) does something about
> problem #1 from comment #14:  It gets (from the kernel) the "thread state"
> as of a crash (the current values of all the user-level registers).
> 
> Though Breakpad minidumps currently store *some* register values (which
> aren't displayed in Socorro, but can be read using minidump_stackwalk), they
> don't store them *all*.  And that's not enough to work around "problem #1
> from comment #14".

I'm curious, what registers is Breakpad missing? It calls thread_get_state to get the state of each thread:
https://code.google.com/p/google-breakpad/source/browse/trunk/src/client/mac/handler/minidump_generator.cc#941

I take it the data from mach_exc_server is more reliable? (I'm not sure why Breakpad wasn't already using that, unless perhaps it wasn't available previously or wasn't documented.)
(In reply to comment #35)

> I'm curious, what registers is Breakpad missing?

I just checked, by adding logging to MinidumpGenerator::GetThreadState(), and found that the register containing the > 0x7fffffffffff crash "address" *isn't* missing.  But it's definitely not present in the output from minidump_stackwalk.  However this time I also noticed that the "address" itself *is* present, here:

basic_code_modules.cc:88: INFO: No module at 0x123456789abcdef
So, Ted, Breakpad minidumps seem already to have the contents of all user-level registers for every thread in the crashing process ... though tell me if I'm wrong.

If I'm right, we seriously need to get Socorro to display all of them for the crashing thread.  Any idea how we should go about doing this?  Or to put this another way:  Where should I open the bug, and who should I CC on it to get it expedited? :-)
Flags: needinfo?(ted)
(In reply to Steven Michaud from comment #36)
> (In reply to comment #35)
> 
> > I'm curious, what registers is Breakpad missing?
> 
> I just checked, by adding logging to MinidumpGenerator::GetThreadState(),
> and found that the register containing the > 0x7fffffffffff crash "address"
> *isn't* missing.  But it's definitely not present in the output from
> minidump_stackwalk.  However this time I also noticed that the "address"
> itself *is* present, here:
> 
> basic_code_modules.cc:88: INFO: No module at 0x123456789abcdef

Apparently minidump_stackwalk just doesn't try very hard to print all the x86-64 registers:
https://code.google.com/p/google-breakpad/source/browse/trunk/src/processor/minidump_stackwalk.cc#204

That'd be trivial to fix.

(In reply to Steven Michaud from comment #37)
> So, Ted, Breakpad minidumps seem already to have the contents of all
> user-level registers for every thread in the crashing process ... though
> tell me if I'm wrong.
> 
> If I'm right, we seriously need to get Socorro to display all of them for
> the crashing thread.  Any idea how we should go about doing this?  Or to put
> this another way:  Where should I open the bug, and who should I CC on it to
> get it expedited? :-)

This would be a two-part Socorro bug: one to add it to the output of Soccoro's minidump_stackwalk (fairly easy, it has JSON output now: https://github.com/mozilla/socorro/blob/master/minidump-stackwalk/stackwalker.cc ) and one to actually display those values. The tricky bit would be determining if we can safely put this information in the public report or whether it needs to be hidden behind authentication. I'm honestly not completely sure about that.

I'd also be a little concerned about bloating the JSON output, but if we only put in register state for the top frame of the crashing thread it wouldn't be so bad, and that's probably the sweet spot in terms of usefulness.
Flags: needinfo?(ted)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #38)
> Apparently minidump_stackwalk just doesn't try very hard to print all the
> x86-64 registers:
> https://code.google.com/p/google-breakpad/source/browse/trunk/src/processor/
> minidump_stackwalk.cc#204
> 
> That'd be trivial to fix.

So trivial I have a patch up for it:
http://breakpad.appspot.com/7654002/
Ted, could you post a copy of your patched minidump_stackwalk somewhere, so that I can download it and test it?  (No, I don't yet know how to build minidump_stackwalk.)
> I don't yet know how to build minidump_stackwalk

Never mind.  Now I do.  For the record here's how:

1) Install a reasonably recent XCode (I have 4.5.2) and the latest XCode commandline tools.
2) Visit https://github.com/mozilla/socorro and click "download zip".
3) CC=clang CXX=clang++ make breakpad
4) CC="clang -Wno-switch" CXX="clang++ -Wno-switch" make stackwalker
(In reply to comment #39)

I've tested your patch, Ted (which seems already to have landed in
Socorro).  It does exactly what I want:  It prints the current values
of all user-level registers for the top frame of each thread's stack.

(In reply to comment #38)

> if we only put in register state for the top frame of the crashing
> thread it wouldn't be so bad, and that's probably the sweet spot in
> terms of usefulness.

This is exactly what I want -- the contents of all user level
registers for the top frame of the crashing thread.  I can't see how
this information could be sensitive -- I can't see how it could be
used to identify the user that submitted the minidump that contains
it.  But I'm willing to listen to contrary arguments.

So you've already finished part 1.

Where should I open a bug for part 2?
Flags: needinfo?(ted)
Flags: needinfo?(ted)
(In reply to Steven Michaud from comment #42)
> (In reply to comment #39)
> 
> I've tested your patch, Ted (which seems already to have landed in
> Socorro).  It does exactly what I want:  It prints the current values
> of all user-level registers for the top frame of each thread's stack.

So, I landed that patch in upstream Breakpad. For future reference you can build a stock minidump_stackwalk just by checking out Breakpad from SVN and running "configure && make".

> This is exactly what I want -- the contents of all user level
> registers for the top frame of the crashing thread.  I can't see how
> this information could be sensitive -- I can't see how it could be
> used to identify the user that submitted the minidump that contains
> it.  But I'm willing to listen to contrary arguments.
> 
> So you've already finished part 1.

That's not really true. I modified upstream Breakpad's minidump_stackwalk, but that's not what we use in Socorro nowadays--we use the stackwalk.cc that's in the github repository.

> 
> Where should I open a bug for part 2?

File a bug under Socorro, but we'll need to actually fix part 1 for it to go anywhere.
I've opened bug 1018360 for part 2.  Should I open another bug for part 1?
This bug has a lot of existing content (including patches), so a new bug is probably a good idea. The patch should be fairly small.
Comment on attachment 8430360 [details] [diff] [review]
Yet another possible fix

(In reply to comment #45)

Though there are some extraneous comments, the underlying bug here is pretty clear -- issue #2 from comment #14.

That issue also has a fix -- in comment #26 (attachment 8425010 [details] [diff] [review]).

I can open another bug on that, if you'd like.  But in the meantime please comment on my patch from comment #26 :-)
Attachment #8430360 - Attachment is obsolete: true
Comment on attachment 8425010 [details] [diff] [review]
Another possible fix

The mach goop looks right to me, from what I remember.
Attachment #8425010 - Flags: feedback?(vladimir) → feedback+
I've opened bug 1035892 to get my patch from comment #26 reviewed.
Attachment #8425010 - Flags: feedback?(ted)

Marking as dup of bug 1035892 since all the work to fix this is happening there.

Status: REOPENED → RESOLVED
Closed: 6 years ago2 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1035892
You need to log in before you can comment on or make changes to this bug.