Closed Bug 947683 Opened 6 years ago Closed 6 years ago

Unable to compile non-threadsafe js shells using --disable-threadsafe

Categories

(Core :: JavaScript Engine, defect, blocker)

x86_64
macOS
defect
Not set
blocker

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox28 --- fixed
firefox29 --- fixed

People

(Reporter: gkw, Assigned: jandem)

References

Details

(Keywords: regression, Whiteboard: [fuzzblocker][qa-])

Attachments

(2 files)

Bug 915735 may have broken compiling js shells and bug 947299 may have partially fixed some issues, but shells compiled with --disable-threadsafe still don't seem to work.

This isn't the first failure involving --disable-threadsafe (see bug 942552), please test with this flag in the future!

Setting needinfo? from Ehsan as he wrote the patch in bug 915735.
Flags: needinfo?(ehsan)
Configure parameters:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --disable-threadsafe
Additionally, I also seem to be having trouble compiling with --enable-threadsafe on Windows (32-bit and 64-bit), though that might be related to bug 947299. (also on rev edac8cba9f78)
As you could have probably guessed form the build failure, this has nothing to do with bug 915735.

$ git bisect good
3b2fc4c5d744113e6c677dd86a4df7806d02450a is the first bad commit
commit 3b2fc4c5d744113e6c677dd86a4df7806d02450a
Author: Jan de Mooij <jdemooij@mozilla.com>
Date:   Fri Dec 6 21:03:27 2013 +0100

    Bug 946883 - Use NSPR thread for AsmJSMachExceptionHandler on OS X, so that it works with PosixNSPR. r=luke

    --HG--
    extra : rebase_source : 34a82b93197c14ab237df23ceb8646499049cbf8

:040000 040000 c72d0fd18fb42d08fcf80299209f85e18e9bd8c9 42da86449ed59158aca1197433e8a116154443a0 M	js
Blocks: 946883
No longer blocks: 915735
Flags: needinfo?(ehsan)
Sigh. Sorry for the mixup.

Turns out that bug 946883 landed in the middle of the landing of bug 915735 and bug 947299's changesets (these broke & fixed Mac shells in some configurations), plus the build failure being in the middle rather than the end of the logfile does not help.
Flags: needinfo?(jdemooij)
Luke, do you think it's ok to disable signal handlers (and therefore Odin) on OS X with --disable-threadsafe?
Flags: needinfo?(luke)
Yeah, just have EnsureIsSignalHandlingBroken() return 'false' in that case and Odin will turn off.
Flags: needinfo?(luke)
Attached patch PatchSplinter Review
This fixes the build for me with --disable-threadsafe.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #8344675 - Flags: review?(luke)
Flags: needinfo?(jdemooij)
Attachment #8344675 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/f6d5a48271b6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
This will need to land on Aurora, I think. The regressor made the merge but not this fix.
Flags: needinfo?(jdemooij)
Blocks: 948301
Comment on attachment 8344675 [details] [diff] [review]
Patch

This patch is basically NPOTB. I'll request approval just to be sure, but we never build the browser with --disable-threadsafe so it has no effect on it.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 946883.
User impact if declined: Unable to build a --disable-threadsafe shell.
Testing completed (on m-c, etc.): On m-c.
Risk to taking this patch (and alternatives if risky): None.
String or IDL/UUID changes made by this patch: None.
Attachment #8344675 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jdemooij)
NPOTB changes do not need approval.
Comment on attachment 8344675 [details] [diff] [review]
Patch

what Ehsan said, use a=NPTOB in your commit, thanks for checking though.
Attachment #8344675 - Flags: approval-mozilla-aurora?
Whiteboard: [fuzzblocker] → [fuzzblocker][checkin-needed-aurora]
https://hg.mozilla.org/releases/mozilla-aurora/rev/423edfffe821
Whiteboard: [fuzzblocker][checkin-needed-aurora] → [fuzzblocker]
If this needs extra QA testing please remove the [qa-] whiteboard tag and add the verifyme keyword.
Whiteboard: [fuzzblocker] → [fuzzblocker][qa-]
This is working.
Status: RESOLVED → VERIFIED
Blocks: 951587
No longer blocks: 951587
You need to log in before you can comment on or make changes to this bug.