Closed Bug 1435219 Opened 3 years ago Closed 3 years ago

Missing WasmSignalHandler platform definitions on Linux sparc64

Categories

(Core :: JavaScript Engine: JIT, defect)

59 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: glaubitz, Assigned: glaubitz)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20180103231032

Steps to reproduce:

Firefox 59 is crashing with MOZ_CRASH() on Linux sparc64 due to missing platform definitions on Linux sparc64 (and sparc):

Thread 1 "firefox" received signal SIGSEGV, Segmentation fault.
0xfff8000109aea030 in ContextToPC (context=0x7feffffa790)
    at /srv/glaubitz/firefox-59.0~b4/js/src/wasm/WasmSignalHandlers.cpp:434
434         MOZ_CRASH();
(gdb) bt
#0  0xfff8000109aea030 in ContextToPC (context=0x7feffffa790)
    at /srv/glaubitz/firefox-59.0~b4/js/src/wasm/WasmSignalHandlers.cpp:434
#1  0xfff8000109aea030 in RedirectJitCodeToInterruptCheck (context=0x7feffffa790, cx=<optimized out>)
    at /srv/glaubitz/firefox-59.0~b4/js/src/wasm/WasmSignalHandlers.cpp:1510
#2  0xfff8000109aea030 in JitInterruptHandler(int, siginfo_t*, void*) (signum=<optimized out>, info=0x7feffffa790, context=0x7feffffa790)
    at /srv/glaubitz/firefox-59.0~b4/js/src/wasm/WasmSignalHandlers.cpp:1561
#3  0xfff800010013bf5c in <signal handler called> ()
    at ../sysdeps/unix/sysv/linux/sparc/sparc64/sigaction.c
#4  0xfff80001009af594 in __GI___poll (fds=0x7feffffab68, nfds=1, timeout=<optimized out>) at ../sysdeps/unix/sysv/linux/poll.c:29
#5  0xfff8000103c53bd0 in  () at /usr/lib/sparc64-linux-gnu/libxcb.so.1
(gdb)

The code in question is:

static uint8_t**
ContextToPC(CONTEXT* context)
{
#ifdef KNOWS_MACHINE_STATE
    return reinterpret_cast<uint8_t**>(&PC_sig(context));
#else
    MOZ_CRASH();
#endif
}

from js/src/wasm/WasmSignalHandlers.cpp
Component: Untriaged → JavaScript Engine: JIT
Product: Firefox → Core
Comment on attachment 8947788 [details] [diff] [review]
0001-Bug-1435219-Missing-WasmSignalHandler-platform-defin.patch

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

Stealing. Thanks for the patch!

Can you format it according to our commit message conventions [1], and upload it again with r?bbouvier, please?

[1] https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#Commit_Message_Conventions
Attachment #8947788 - Flags: review?(mh+mozilla) → feedback+
(In reply to Benjamin Bouvier [:bbouvier] from comment #2)
> Stealing. Thanks for the patch!
> 
> Can you format it according to our commit message conventions [1], and
> upload it again with r?bbouvier, please?

Whoops, did too many OpenJDK upstream commits recently and apparently adopted the commit message style :).

Patch comes in a second.
Attachment #8947788 - Attachment is obsolete: true
Attachment #8947872 - Flags: review?(bbouvier)
Comment on attachment 8947872 [details] [diff] [review]
0001-Bug-1435219-Add-missing-WasmSignalHandler-platform-d.patch

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

Hmm, I still see the email format (in the splinter review screen):

From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: Fri, 2 Feb 2018 14:09:36 +0100
Subject: [PATCH] Bug 1435219 - Add missing WasmSignalHandler platform
 definitions on Linux sparc64

If you're using git, there are tools to convert from one format to the other, mentioned in the page I linked to earlier.

Usually, we'd have something like:

# HG changeset patch
# User Benjamin Bouvier <benj@benj.me>
# Date 1517423158 -3600
#      Wed Jan 31 19:25:58 2018 +0100
# Node ID 6ad018b23ceebbbf2efe0ae315ab91f937e439a8
# Parent  db8d5186d53e30578a571c27268bdba194bf8e99

where the node ids are just referring to hg changesets.
Attachment #8947872 - Flags: review?(bbouvier)
Hmm, I have always used git for Mozilla upstream changes in the past.

Has the policy changed now?

I can whip up a Mercurial changeset, but that will take some time more.
Oops nevermind, I'm being told on irc that we can import git formatted patches. Sorry, you were right :)
Checkin-needed; not a tier 1 platform, so no try build. Thanks!
Keywords: checkin-needed
Assignee: nobody → glaubitz
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6123659a0676
Add missing WasmSignalHandler platform definitions on Linux sparc64 r=bbouvier
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6123659a0676
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.