BrowserWorks / Waterfox-Classic

The Waterfox Classic repository, for legacy systems and customisation.
https://classic.waterfox.net
Mozilla Public License 2.0
175 stars 34 forks source link

Avoid Ion crash due to logical assignment operators #164

Closed buttercookie42 closed 1 year ago

buttercookie42 commented 1 year ago

The Mozilla implementation of the logical assignment operators (see #138) uses a JSOP_GOTO, which in our version of SpiderMonkey still requires some special handling in IonControlFlow.cpp.

Until somebody understands that logic well enough to shoehorn in a proper solution, we need to disable Ion compilation when hitting a logical assignment operator in order to avoid triggering the diagnostic crash here.

Note for posterity if somebody wants to take a closer look and get Ion compilation working again for this case: This code for example

(e._jswReactRootContainer ||= (0, react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot)(e)).render(t);

gets compiled into this bytecode

# from ifeq @ 00188
00261:   1  jumptarget                  #
    00262:   1  zero                        # 0
    00263:   1  pop                         #
    00264:   1  getaliasedvar "react__WEBPACK_IMPORTED_MODULE_1__" (hops = 2, slot = 3) # react__WEBPACK_IMPORTED_MODULE_1__
    00269:   1  getprop "isValidElement"    # react__WEBPACK_IMPORTED_MODULE_1__.isValidElement
    00274:   1  undefined                   # react__WEBPACK_IMPORTED_MODULE_1__.isValidElement undefined
    00275:   1  getaliasedvar "t" (hops = 0, slot = 3) # react__WEBPACK_IMPORTED_MODULE_1__.isValidElement undefined t
    00280:   1  call 1                      # react__WEBPACK_IMPORTED_MODULE_1__.isValidElement(...)
    00283:   1  ifeq 363 (+80)              #
    00288:   1  jumptarget                  #
    00289:   1  getaliasedvar "e" (hops = 0, slot = 2) # e
    00294:   1  dup                         # e e
    00295:   1  getprop "_jswReactRootContainer" # e e._jswReactRootContainer
    00300:   1  or 338 (+38)                # e e._jswReactRootContainer
    00305:   1  jumptarget                  # e e._jswReactRootContainer
    00306:   1  pop                         # e
    00307:   1  zero                        # e 0
    00308:   1  pop                         # e
    00309:   1  getaliasedvar "react_dom_client__WEBPACK_IMPORTED_MODULE_0__" (hops = 2, slot = 2) # e react_dom_client__WEBPACK_IMPORTED_MODULE_0__
    00314:   1  getprop "createRoot"        # e react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot
    00319:   1  undefined                   # e react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot undefined
    00320:   1  getaliasedvar "e" (hops = 0, slot = 2) # e react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot undefined e
    00325:   1  call 1                      # e react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)
    00328:   1  strict-setprop "_jswReactRootContainer" # react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)
    00333:   1  goto 341 (+8)               # react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)

# from or @ 00300
00338:   1  jumptarget                  # e e._jswReactRootContainer
    00339:   1  swap                        # e._jswReactRootContainer e
    00340:   1  pop                         # e._jswReactRootContainer

# from goto @ 00333
00341:   1  jumptarget                  # merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>
    00342:   1  dup                         # merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)> merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>
    00343:   1  callprop "render"           # merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)> merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>.render
    00348:   1  swap                        # merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>.render merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>
    00349:   1  getaliasedvar "t" (hops = 0, slot = 3) # merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>.render merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)> t
    00354:   1  call-ignores-rv 1           # merged<react_dom_client__WEBPACK_IMPORTED_MODULE_0__.createRoot(...)>.render(...)
    00357:   1  pop                         #
    00358:   1  goto 1299 (+941)            #

So whereas the regular short-circuiting operators (&&, ||, ??) roughly resemble a

…
if (!shortCircuitingConditionOnLeftHandSide) {
    evaluateRightHandSide()
}
…

with the logical assignment operators we now have something like

…
if (!shortCircuitingConditionOnLeftHandSide) {
    evaluateRightHandSideAndAssign()
} else {
    doSomeCleanup()
}
…

As noted here, the bytecode for an if-then-else looks roughly like

    //    IFEQ X     ; src note (IF_ELSE, COND)
    //    ...
    //    GOTO Z
    // X: JUMPTARGET ; else/else if
    //    ...
    // Z: JUMPTARGET ; join

which is basically what we have here above between 00300: and 00341:, except that of course instead of IFEQ we're starting out with a AND/OR/COALESCE operator at the beginning.

So it should probably be possible to cobble something together by looking at the existing processIf… and processShortCircuit… routines in IonControlFlow.cpp.

adamp01 commented 1 year ago

I'll get this merged and push a release once #162 is resolved.