Closed Bug 1036780 Opened 6 years ago Closed 6 years ago

Replace MOZ_ASSUME_UNREACHABLE with MOZ_CRASH in js/src/gc

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox32 --- wontfix
firefox33 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

Details

Attachments

(1 file)

Replace calls to the deprecated MOZ_ASSUME_UNREACHABLE macro with MOZ_CRASH.

Calls to MOZ_NOT_REACHED were replaced with MOZ_ASSUME_UNREACHABLE which, instead of crashing unconditionally, invokes undefined behavior in the name of dubious and dangerous compiler optimizations. See bug 990764 for details. This patch reverts to the original crashing behavior.

(MOZ_ASSUME_UNREACHABLE will eventually be removed or renamed in bug 990764.)
Attachment #8453542 - Flags: review?(terrence)
Comment on attachment 8453542 [details] [diff] [review]
replace-js-gc-MOZ_ASSUME_UNREACHABLE.patch

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

Thanks! A couple of these sites are hot on Sunspider, so keep an eye on talos-ss after landing.
Attachment #8453542 - Flags: review?(terrence) → review+
Which functions are hot on Sunspider? GetGCKindSlots() in jsgc.h or FinalizeArenas() in jsgc.cpp?

If you prefer, I can replace MOZ_ASSUME_UNREACHABLE with MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE (the __builtin_unreachable optimization hint) instead of MOZ_CRASH in those hot functions.
Flags: needinfo?(terrence)
(In reply to Chris Peterson (:cpeterson) from comment #2)
> Which functions are hot on Sunspider? GetGCKindSlots() in jsgc.h or
> FinalizeArenas() in jsgc.cpp?
> 
> If you prefer, I can replace MOZ_ASSUME_UNREACHABLE with
> MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE (the __builtin_unreachable
> optimization hint) instead of MOZ_CRASH in those hot functions.

|toHandle| actually. Don't worry about it unless you actually see a slowdown in practice though.
Flags: needinfo?(terrence)
https://hg.mozilla.org/mozilla-central/rev/5a2cd9e13de1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.