Open
Bug 862657
Opened 11 years ago
Updated 2 years ago
Fix failures on Mac SpiderMonkey jobs
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
NEW
People
(Reporter: philor, Unassigned)
References
Details
(Whiteboard: [leave open])
Attachments
(4 files)
5.47 KB,
patch
|
Details | Diff | Splinter Review | |
1.36 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
2.08 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
1.04 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
Because I'm not shy about just taking them away from you if you leave them busted too long. warnaserr: https://tbpl.mozilla.org/php/getParsedLog.php?id=21886474&tree=Mozilla-Inbound dtrace: https://tbpl.mozilla.org/php/getParsedLog.php?id=21886262&tree=Mozilla-Inbound jsgc.cpp In file included from /builds/slave/m-in_osx64-d_sm-warnaserrdebug/src/js/src/jsgc.cpp:50: In file included from ../../src/js/src/jsatom.h:22: In file included from ../../src/js/src/gc/Barrier.h:12: ../../src/js/src/gc/Heap.h:664:8: error: cannot initialize a parameter of type 'void *' with an rvalue of type 'volatile uintptr_t (*)[2016]' struct ChunkBitmap ^~~~~~~~~~~ /builds/slave/m-in_osx64-d_sm-warnaserrdebug/src/js/src/jsgc.cpp:3099:9: note: in instantiation of function template specialization 'js::Swap<js::gc::ChunkBitmap>' requested here js::Swap(*entry, *bitmap); ^ /builds/slave/m-in_osx64-d_sm-warnaserrdebug/objdir/dist/include/js/Utility.h:737:7: note: implicit default copy assignment operator for 'js::gc::ChunkBitmap' first required here t = Move(u); ^ 1 error generated. They've been busted for a long long time, so there may well be others below that one, though.
Reporter | ||
Comment 1•11 years ago
|
||
No takers? Shut them off now?
Comment 2•11 years ago
|
||
I'll take a look, sigh.
Comment 3•11 years ago
|
||
As far as I can tell, I can't test these specific builds on try, but at least I can test that the patch compiles on everything else, then if it does I can push and see what happens. This adds pod-operation gnarliness to allow volatile to flow places, and may be enough to fix things. We'll find out. https://tbpl.mozilla.org/?tree=Try&rev=98f21cac994d
Comment 4•11 years ago
|
||
Attachment #745452 -
Flags: review?(nfroyd)
Updated•11 years ago
|
Assignee: general → jwalden+bmo
Comment 5•11 years ago
|
||
You can't memset, memcpy, etc. volatile objects. The compiler has to be able to see the actual expressions/operations used to modify that memory. That motivates these overloads that work on volatile data, but don't include any memset or equivalent "optimizations".
Attachment #745454 -
Flags: review?(nfroyd)
Comment 6•11 years ago
|
||
This might or might not work, but I believe the change at least won't hurt anything, so it seems worth trying to see what happens.
Attachment #745455 -
Flags: review?(wmccloskey)
Comment 7•11 years ago
|
||
Comment on attachment 745454 [details] [diff] [review] Make PodCopy work copying to volatile addresses (when operator= works), add PodArrayCopy for copying fixed-size arrays Luke, your comments here would also be appreciated, I think. I think eventually we probably want to get rid of volatile entirely for this junk, and use actual atomic classes/arrays of them, maybe. But in the meantime, this is maybe the most reasonable thing, I think.
Attachment #745454 -
Flags: feedback?(luke)
Comment 8•11 years ago
|
||
I would think the normal overload resolution rules would allow you to just add the volatile overload without requiring the EnableIf; is that not the case?
Comment 9•11 years ago
|
||
Hmm. It seems to be! Possibly I added that at some intermediate state when it might have been needed, but in the final state it was superfluous.
Comment 10•11 years ago
|
||
Comment on attachment 745452 [details] [diff] [review] Implement mozilla::IsConst and mozilla::IsVolatile, needed for later patches Review of attachment 745452 [details] [diff] [review]: ----------------------------------------------------------------- I think this is fine to go in as-is, even though it sounds like IsVolatile isn't needed for part 2.
Attachment #745452 -
Flags: review?(nfroyd) → review+
Comment 11•11 years ago
|
||
Comment on attachment 745454 [details] [diff] [review] Make PodCopy work copying to volatile addresses (when operator= works), add PodArrayCopy for copying fixed-size arrays Review of attachment 745454 [details] [diff] [review]: ----------------------------------------------------------------- r=me with removing the #include of TypeTraits.h and the EnableIf bit for the non-volatile PodCopy if that all compiles as expected.
Attachment #745454 -
Flags: review?(nfroyd) → review+
Comment on attachment 745455 [details] [diff] [review] Speculative fix for Swap<ChunkBitmap>(), by giving ChunkBitmap a Move constructor Review of attachment 745455 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Jeff.
Attachment #745455 -
Flags: review?(wmccloskey) → review+
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/33a510f4ae92 https://hg.mozilla.org/integration/mozilla-inbound/rev/75f047d8b347 https://hg.mozilla.org/integration/mozilla-inbound/rev/83ed83e58666 Unfortunately that seems to have not affected the error. I'll look some more, I guess.
Whiteboard: [leave open]
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/33a510f4ae92 https://hg.mozilla.org/mozilla-central/rev/75f047d8b347 https://hg.mozilla.org/mozilla-central/rev/83ed83e58666
Updated•11 years ago
|
Attachment #745454 -
Flags: feedback?(luke)
Comment 15•11 years ago
|
||
It sounds like this may just be due to using the wrong version of clang for the osx spidermonkey builds.
Depends on: 895208
Comment 16•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: jwalden → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•