Bug 1893554 - Ensure Ion frames with rematerialized frames are invalidated. r=iain

When the debugger creates a `RematerializedFrame` for an Ion frame, we need to invalidate
the Ion frame to ensure the rematerialized frame is destroyed in `FinishBailoutToBaseline`
(or the exception handler).

The debugger code could fail with OOM before doing this invalidation and in this case we
left a rematerialized frame in the activation's map after returning from the Ion frame.

Differential Revision: https://phabricator.services.mozilla.com/D208774
This commit is contained in:
Jan de Mooij 2024-04-30 12:21:34 +00:00
parent 536e415070
commit fd012729c2
4 changed files with 56 additions and 23 deletions

View file

@ -0,0 +1,31 @@
let g = newGlobal({ newCompartment: true });
g.parent = this;
g.eval(
"(" +
function () {
Debugger(parent).onExceptionUnwind = function (frame) {
frame.older;
};
} +
")()"
);
function f(x, y) {
try {
Object.setPrototypeOf(
y,
new Proxy(Object.getPrototypeOf(y), {
get(a, b, c) {
return undefined;
},
})
);
} catch (e) {}
}
function h(x, y) {
f(x, y);
}
oomTest(function () {
h("", undefined);
h("", "");
"".replaceAll();
});

View file

@ -228,13 +228,8 @@ static void OnLeaveIonFrame(JSContext* cx, const InlineFrameIterator& frame,
RematerializedFrame* rematFrame = nullptr;
{
JS::AutoSaveExceptionState savedExc(cx);
// We can run recover instructions without invalidating because we're
// already leaving the frame.
MaybeReadFallback::FallbackConsequence consequence =
MaybeReadFallback::Fallback_DoNothing;
rematFrame = act->getRematerializedFrame(cx, frame.frame(), frame.frameNo(),
consequence);
IsLeavingFrame::Yes);
if (!rematFrame) {
return;
}

View file

@ -13,6 +13,7 @@
#include <utility> // std::move
#include "debugger/DebugAPI.h" // js::DebugAPI
#include "jit/Invalidation.h" // js::jit::Invalidate
#include "jit/JSJitFrameIter.h" // js::jit::InlineFrameIterator
#include "jit/RematerializedFrame.h" // js::jit::RematerializedFrame
#include "js/AllocPolicy.h" // js::ReportOutOfMemory
@ -58,7 +59,9 @@ js::jit::JitActivation::~JitActivation() {
// Traps get handled immediately.
MOZ_ASSERT(!isWasmTrapping());
clearRematerializedFrames();
// Rematerialized frames must have been removed by either the bailout code or
// the exception handler.
MOZ_ASSERT_IF(rematerializedFrames_, rematerializedFrames_->empty());
}
void js::jit::JitActivation::setBailoutData(
@ -82,20 +85,9 @@ void js::jit::JitActivation::removeRematerializedFrame(uint8_t* top) {
}
}
void js::jit::JitActivation::clearRematerializedFrames() {
if (!rematerializedFrames_) {
return;
}
for (RematerializedFrameTable::Enum e(*rematerializedFrames_); !e.empty();
e.popFront()) {
e.removeFront();
}
}
js::jit::RematerializedFrame* js::jit::JitActivation::getRematerializedFrame(
JSContext* cx, const JSJitFrameIter& iter, size_t inlineDepth,
MaybeReadFallback::FallbackConsequence consequence) {
IsLeavingFrame leaving) {
MOZ_ASSERT(iter.activation() == this);
MOZ_ASSERT(iter.isIonScripted());
@ -117,6 +109,14 @@ js::jit::RematerializedFrame* js::jit::JitActivation::getRematerializedFrame(
// preserve identity. Therefore, we always rematerialize an uninlined
// frame and all its inlined frames at once.
InlineFrameIterator inlineIter(cx, &iter);
// We can run recover instructions without invalidating if we're always
// leaving the frame.
MaybeReadFallback::FallbackConsequence consequence =
MaybeReadFallback::Fallback_Invalidate;
if (leaving == IsLeavingFrame::Yes) {
consequence = MaybeReadFallback::Fallback_DoNothing;
}
MaybeReadFallback recover(cx, this, &iter, consequence);
// Frames are often rematerialized with the cx inside a Debugger's
@ -124,6 +124,14 @@ js::jit::RematerializedFrame* js::jit::JitActivation::getRematerializedFrame(
// be in the script's realm.
AutoRealmUnchecked ar(cx, iter.script()->realm());
// The Ion frame must be invalidated to ensure the rematerialized frame will
// be removed by the bailout code or the exception handler. If we're always
// leaving the frame, the caller is responsible for cleaning up the
// rematerialized frame.
if (leaving == IsLeavingFrame::No && !iter.checkInvalidation()) {
jit::Invalidate(cx, iter.script());
}
if (!RematerializedFrame::RematerializeInlineFrames(cx, top, inlineIter,
recover, frames)) {
return nullptr;

View file

@ -40,6 +40,8 @@ namespace jit {
class BailoutFrameInfo;
enum class IsLeavingFrame { No, Yes };
// A JitActivation is used for frames running in Baseline or Ion.
class JitActivation : public Activation {
// If Baseline, Ion or Wasm code is on the stack, and has called into C++,
@ -94,8 +96,6 @@ class JitActivation : public Activation {
// purposes. Wasm code can't trap reentrantly.
mozilla::Maybe<wasm::TrapData> wasmTrapData_;
void clearRematerializedFrames();
#ifdef CHECK_OSIPOINT_REGISTERS
protected:
// Used to verify that live registers don't change between a VM call and
@ -156,8 +156,7 @@ class JitActivation : public Activation {
// The inlineDepth must be within bounds of the frame pointed to by iter.
RematerializedFrame* getRematerializedFrame(
JSContext* cx, const JSJitFrameIter& iter, size_t inlineDepth = 0,
MaybeReadFallback::FallbackConsequence consequence =
MaybeReadFallback::Fallback_Invalidate);
IsLeavingFrame leaving = IsLeavingFrame::No);
// Look up a rematerialized frame by the fp. If inlineDepth is out of
// bounds of what has been rematerialized, nullptr is returned.