Closed Bug 942552 Opened 8 years ago Closed 8 years ago

Unified sources causing build issues in SpiderMonkey with --disable-threadsafe

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
blocker

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox28 --- verified

People

(Reporter: gkw, Assigned: ehsan.akhgari)

References

Details

(Whiteboard: [fuzzblocker])

Attachments

(3 files)

Attached file build log
/var/folders/58/lf0k_v8n6jlg2h5gw3_crs3m0000gn/T/abtmp-ad6589ed742c-R0Upyn/compilePath/js/src/vm/PosixNSPR.cpp:267:13: note: candidate constructor (the implicit move constructor) not viable: cannot convert argument of incomplete type 'PRLock *' to 'nspr::CondVar'
class nspr::CondVar
            ^
/var/folders/58/lf0k_v8n6jlg2h5gw3_crs3m0000gn/T/abtmp-ad6589ed742c-R0Upyn/compilePath/js/src/vm/PosixNSPR.cpp:273:5: note: candidate constructor not viable: cannot convert argument of incomplete type 'PRLock *' to 'nspr::Lock *'
    CondVar(nspr::Lock *lock) : lock_(lock) {}

fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.


In the middle of the attached build log are these errors. Perhaps this is causing the build to fail somewhat?

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/3b9e118ded0f
user:        Ehsan Akhgari
date:        Fri Nov 22 00:16:31 2013 -0500
summary:     Bug 941424 - Build more of the JS engine in unified mode; r=djvj
Flags: needinfo?(ehsan)
Severity: normal → blocker
My configure arguments were:

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 --enable-more-deterministic --with-ccache --disable-threadsafe

Probably the key flag here is "--disable-threadsafe". I think threadsafe builds worked for me (and TBPL produces threadsafe builds too).
Summary: Unified sources causing build issues in SpiderMonkey → Unified sources causing build issues in SpiderMonkey with --disable-threadsafe
jslock.h doesn't try to use PosixNSPR.h if JS_THREADSAFE is not defined.  However, we still try to build PosixNSPR.cpp in non-threadsafe mode, and when that file tries to include PosixNSPR.h, its definition clashes with the PRThread etc typedefs in jslock.h.

Not sure what the desired semantics are here, but it seems to me like we should not be building PosixNSPR.cpp in non-threadsafe builds at all.
Blocks: 931151
Flags: needinfo?(ehsan) → needinfo?(wmccloskey)
Assignee: nobody → ehsan
Comment on attachment 8337396 [details] [diff] [review]
Don't build PosixNSPR.cpp in non-threadsafe mode; r=billm

Like this?  This seems to fix the build for me.  Gary, can you please check this patch to see if it fixes your build as well?
Attachment #8337396 - Flags: review?(wmccloskey)
Attachment #8337396 - Flags: feedback?(gary)
Attached patch posix-nspr-fixSplinter Review
I think I'd prefer this. JS_POSIX_NSPR shouldn't be enabled without JS_THREADSAFE.
Attachment #8337402 - Flags: review?(ehsan)
Flags: needinfo?(wmccloskey)
Comment on attachment 8337396 [details] [diff] [review]
Don't build PosixNSPR.cpp in non-threadsafe mode; r=billm

Yes, this does fix the issue I had.
Attachment #8337396 - Flags: feedback?(gary) → feedback+
Comment on attachment 8337402 [details] [diff] [review]
posix-nspr-fix

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

Looks good!
Attachment #8337402 - Flags: review?(ehsan) → review+
I helped land this to fix this breakage for Monday. Thanks everyone!

https://hg.mozilla.org/integration/mozilla-inbound/rev/48161187ac9a
https://hg.mozilla.org/mozilla-central/rev/48161187ac9a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment on attachment 8337396 [details] [diff] [review]
Don't build PosixNSPR.cpp in non-threadsafe mode; r=billm

Thanks Gary.
Attachment #8337396 - Flags: review?(wmccloskey)
Gary, can you confirm this is no longer an issue for you with today's builds?
Flags: needinfo?(gary)
This particular issue was fixed then. More recent build breakage comprised of bug 947683 and bug 948301.
Status: RESOLVED → VERIFIED
Flags: needinfo?(gary)
You need to log in before you can comment on or make changes to this bug.