Closed Bug 1212430 Opened 4 years ago Closed 4 years ago

Rename or remove CrashAtUnhandlableOOM

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jandem, Assigned: jonco)

Details

Attachments

(1 file)

AutoEnterOOMUnsafeRegion is the preferred way that works better when we're OOM testing.

We should probably remove CrashAtUnhandlableOOM so we don't add more calls to it.
Good plan.  There are just a couple of these remaining which I didn't convert to using AutoEnterOOMUnsafeRegion:

jit/arm/Assembler-arm.h - called when we have an out of range BOffImm, so not actually an OOM. jolesen said he had a plan for this.

jit/JitFrames.cpp - called when initInstructionResults() fails.  This is complicated and it wasn't clear to me that there were not other reasons why this could fail apart from OOM.

Probably we should just convert these to MOZ_CRASH.
(In reply to Jon Coppeard (:jonco) from comment #1)
> Good plan.  There are just a couple of these remaining which I didn't
> convert to using AutoEnterOOMUnsafeRegion:
> 
> jit/arm/Assembler-arm.h - called when we have an out of range BOffImm, so
> not actually an OOM. jolesen said he had a plan for this.

Yep. Bug 1207827. For ARM64, BOffImm has a much smaller range, so a solution is required there. We might as well share with ARM.

Feel free to replace this call with a MOZ_CRASH() or MOZ_RELEASE_ASSERT() if you want to get rid of CrashAtUnhandlableOOM() right away. Don't do a MOZ_ASSERT() since it is fairly trivial to trigger by compiling a large enough function.
Assignee: nobody → jcoppeard
Attachment #8670906 - Flags: review?(jdemooij)
Comment on attachment 8670906 [details] [diff] [review]
bug1212430-remove-CAUO

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

Great!

::: js/public/Utility.h
@@ -181,5 @@
>  
>  namespace js {
>  
> -MOZ_NORETURN MOZ_COLD void
> -CrashAtUnhandlableOOM(const char* reason);

Maybe add these MOZ_NORETURN MOZ_COLD annotations to crash().

::: js/src/jscntxt.cpp
@@ +1211,5 @@
>          check(frame.scopeChain());
>  }
>  #endif
>  
> +void AutoEnterOOMUnsafeRegion::crash(const char* reason)

Nit: \n after void
Attachment #8670906 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/1b6a1a820176
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.