Replace MOZ_ASSUME_UNREACHABLE with MOZ_CRASH in js/src/gc

RESOLVED FIXED in Firefox 33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

unspecified
mozilla33
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox32 wontfix, firefox33 fixed)

Details

Attachments

(1 attachment)

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: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.