Closed Bug 721321 Opened 11 years ago Closed 10 years ago

Minidump writer process ends with SIGILL after writing the minidump

Categories

(Toolkit :: Crash Reporting, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

I ended up reliably triggering this when applying part 2 from bug 686805, but I suspect this may happen under some circumstances, which effectively makes us miss crash reports.

It appears this was fixed upstream: http://codereview.chromium.org/8165010
Assignee: nobody → mh+mozilla
Attachment #591726 - Flags: review?(ted.mielczarek)
Blocks: 686805
In fact, it happens after the minidump is written, not before, but it'd still be better to do the right thing here.
Summary: Minidump writer process may end with SIGILL before writing a minidump → Minidump writer process may end with SIGILL after writing the minidump
Summary: Minidump writer process may end with SIGILL after writing the minidump → Minidump writer process ends with SIGILL after writing the minidump
I really don't feel confident reviewing this. Can you get someone who actually understands ARM assembly to review?
Attachment #591726 - Flags: review?(ted.mielczarek) → review?(Jacob.Bramley)
Comment on attachment 591726 [details] [diff] [review]
Fix sys_clone() on ARM to work reliably in Thumb-2

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

::: toolkit/crashreporter/google-breakpad/src/common/linux/linux_syscall_support.h
@@ +2196,5 @@
> +                              */
> +                           #ifdef __thumb2__
> +                             "ldr   r7,[sp]\n"
> +                             "blx   r7\n"
> +                           #else

Yep, this is fine.

I'd normally argue for using BLX in the ARM case too (except on ARMv4T, where it doesn't exist), but this doesn't exactly look like performance-critical code. I'd probably make it "__thumb2__ || !__armv4t__" (or whatever the macro is called), but it's up to you.
Attachment #591726 - Flags: review?(Jacob.Bramley) → review+
Can you upload a patch against the linux-syscall-support repo to codereview.chromium.org and ask for review from markus@chromium.org ?
(In reply to Ted Mielczarek [:ted, :luser] from comment #7)
> Can you upload a patch against the linux-syscall-support repo to
> codereview.chromium.org and ask for review from markus@chromium.org ?

The patch comes from there.
https://hg.mozilla.org/mozilla-central/rev/04a179077054
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.