1444604 - Assertion failure: !frames->empty(), at js/src/vm/SavedStacks.cpp:143 or AddressSanitizer: heap-buffer-overflow [@ mozilla::Variant<..., js::wasm::DebugFrame*>::operator==] with READ of size 1

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 0817a733d45a+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

var evalInFrame = (function (global) {
  var dbgGlobal = newGlobal();
  var dbg = new dbgGlobal.Debugger();
  return function evalInFrame(upCount, code) {
    dbg.addDebuggee(global);
    var frame = dbg.getNewestFrame().older;
    for (var i = 0; i < upCount; i++) {
    }
    var completion = frame.eval(code);
  };
})(this);
enableTrackAllocations();
evalInFrame(1, "print(a)");


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000bf382c in js::LiveSavedFrameCache::find (this=this@entry=0x7fffffffcf80, cx=0x7ffff5f16000, framePtr=..., pc=0x7ffff480a2c8 ":\001", frame=...) at js/src/vm/SavedStacks.cpp:143
#0  0x0000000000bf382c in js::LiveSavedFrameCache::find (this=this@entry=0x7fffffffcf80, cx=0x7ffff5f16000, framePtr=..., pc=0x7ffff480a2c8 ":\001", frame=...) at js/src/vm/SavedStacks.cpp:143
#1  0x0000000000bf8322 in js::SavedStacks::insertFrames(JSContext*, js::FrameIter&, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=this@entry=0x7ffff5f3e100, cx=cx@entry=0x7ffff5f16000, iter=..., frame=..., capture=capture@entry=<unknown type in /mnt/LangFuzz/work/builds/debug64/dist/bin/js, CU 0x569a2d9, DIE 0x58e7259>) at js/src/vm/SavedStacks.cpp:1370
#2  0x0000000000bf9446 in js::SavedStacks::saveCurrentStack(JSContext*, JS::MutableHandle<js::SavedFrame*>, mozilla::Variant<JS::AllFrames, JS::MaxFrames, JS::FirstSubsumedFrame>&&) (this=this@entry=0x7ffff5f3e100, cx=cx@entry=0x7ffff5f16000, frame=..., capture=capture@entry=<...>) at js/src/vm/SavedStacks.cpp:1225
#3  0x0000000000bf97fa in js::SavedStacks::MetadataBuilder::build (this=<optimized out>, cx=0x7ffff5f16000, target=..., oomUnsafe=...) at js/src/vm/SavedStacks.cpp:1703
#4  0x0000000000b3f74b in JSCompartment::setNewObjectMetadata (this=0x7ffff5f3e000, cx=0x7ffff5f16000, obj=obj@entry=...) at js/src/vm/JSCompartment.cpp:1055
#5  0x0000000000b3fcab in js::SetNewObjectMetadata<JSObject> (obj=0x7ffff4d00580, cx=0x7ffff5f16000) at js/src/vm/JSObject-inl.h:389
#6  js::AutoSetNewObjectMetadata::~AutoSetNewObjectMetadata (this=0x7fffffffc250, __in_chrg=<optimized out>) at js/src/vm/JSCompartment.cpp:1423
#7  0x0000000000bd99a1 in js::ProxyObject::New (cx=0x7ffff5f16000, handler=<optimized out>, priv=..., proto_=..., options=...) at js/src/vm/ProxyObject.cpp:86
#8  0x0000000000a2b8c5 in js::NewProxyObject (cx=<optimized out>, handler=<optimized out>, priv=..., proto_=<optimized out>, options=...) at js/src/proxy/Proxy.cpp:828
#9  0x0000000000a2cefd in js::Wrapper::New (cx=cx@entry=0x7ffff5f16000, obj=<optimized out>, handler=0x2058820 <js::CrossCompartmentWrapper::singleton>, options=...) at js/src/proxy/Wrapper.cpp:322
#10 0x0000000000a2d042 in js::TransparentObjectWrapper (cx=0x7ffff5f16000, existing=..., obj=...) at js/src/proxy/Wrapper.cpp:441
#11 0x0000000000b45038 in JSCompartment::getOrCreateWrapper (this=this@entry=0x7ffff5f3e000, cx=cx@entry=0x7ffff5f16000, existing=..., existing@entry=..., obj=...) at js/src/vm/JSCompartment.cpp:430
#12 0x0000000000b45312 in JSCompartment::wrap (this=this@entry=0x7ffff5f3e000, cx=0x7ffff5f16000, obj=obj@entry=...) at js/src/vm/JSCompartment.cpp:476
#13 0x00000000004820f7 in JSCompartment::wrap (this=0x7ffff5f3e000, cx=0x7ffff5f16000, vp=...) at js/src/vm/JSCompartment-inl.h:155
#14 0x0000000000a24df7 in js::CrossCompartmentWrapper::call (this=0x2058820 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff5f16000, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:362
#15 0x0000000000a35285 in js::Proxy::call (cx=0x7ffff5f16000, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:510
#16 0x0000000000a3534d in js::proxy_Call (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/proxy/Proxy.cpp:769
#17 0x000000000057b1dd in js::CallJSNative (cx=0x7ffff5f16000, native=0xa352d0 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:290
[...]
#31 0x00000000004440a2 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9408
rax	0x0	0
rbx	0x7fffffffcf80	140737488342912
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffb4d0	140737488336080
rsp	0x7fffffffb460	140737488335968
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7fffffffb610	140737488336400
r13	0x7ffff5f16000	140737319624704
r14	0x7ffff480a2c8	140737295459016
r15	0x7fffffffb480	140737488336000
rip	0xbf382c <js::LiveSavedFrameCache::find(JSContext*, js::LiveSavedFrameCache::FramePtr&, unsigned char const*, JS::MutableHandle<js::SavedFrame*>) const+508>
=> 0xbf382c <js::LiveSavedFrameCache::find(JSContext*, js::LiveSavedFrameCache::FramePtr&, unsigned char const*, JS::MutableHandle<js::SavedFrame*>) const+508>:	movl   $0x0,0x0
   0xbf3837 <js::LiveSavedFrameCache::find(JSContext*, js::LiveSavedFrameCache::FramePtr&, unsigned char const*, JS::MutableHandle<js::SavedFrame*>) const+519>:	ud2



This is at most sec-moderate because it requires the debugger to be involved.

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, failed due to error (try manually).

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/f54675bcf897
user:        Jim Blandy
date:        Sat Feb 24 15:39:43 2018 -0800
summary:     Bug 1438121: Final Part 2: Fix interaction between async parents and the LiveSavedFrameCache in SavedStacks::insertFrames. r=fitzgen

Jim, is bug 1438121 a likely regressor?

Comment 3

[Jim Blandy :jimb]

Yes, Bug 1438121 is almost certainly the regressor. In that bug I introduced much stricter assertions about the behavior of the LiveSavedFrameCache; one of those is failing.

I can reproduce this bug locally.

Comment 4

[Jim Blandy :jimb]

I can't say yet whether this is SS; it may not depend on Debugger.

Comment 5

[Christian Holler (:decoder)]

(In reply to Jim Blandy :jimb from comment #3)
> Yes, Bug 1438121 is almost certainly the regressor. In that bug I introduced
> much stricter assertions about the behavior of the LiveSavedFrameCache; one
> of those is failing.

Note that this bug also reproduces on non-debug builds and either crashes or triggers an ASan heap-buffer-overflow error (so just new assertions alone can't be the problem).

Comment 6

[Christian Holler (:decoder)]

This is also crashing in various ways, marking as fuzzblocker based on that.

Comment 7

[Jim Blandy :jimb]

The problem seems to be that my changes in 1438121 didn't take into account js::InterpreterFrame::evalInPrevLink_. Since I used MOZ_ASSERT to check for running off the end of the cache, this would account for the non-debug crashes as well.

Here's a more direct way to reproduce the problem:

// LiveSavedFrameCache should not be confused by eval-in-frame-prev links.

const g = newGlobal();
const dbg = new Debugger();
const gDO = dbg.addDebuggee(g);

g.evaluate(`
  function inner() { debugger; }
  function outer() { inner(); }
`);

dbg.onDebuggerStatement = function handler(frame) {
  // Capture the JS stack, putting frames for handler, inner, and outer in the
  // cache. Perform all captures in g's compartment, to avoid flushing the cache
  // for compartment mismatches.
  frame.eval('print(`in inner: ${saveStack()}`)');

  // Carry out a capture in outer's context, following the eval-in-frame prev
  // link and thus skipping over inner, removing it from the cache.
  frame.older.eval('print(`in outer: ${saveStack()}`)');

  // Capture again. inner's hasCachedSavedFrame bit should be set, but its entry
  // has been flushed.
  frame.eval('print(`in inner: ${saveStack()}`)');
};

g.outer();

Comment 8

[Jim Blandy :jimb]

This is not S-S, because it only occurs when an InterpreterFrame has a non-null evalInFramePrev_ member, and the only way to introduce such a frame is via the Debugger API.

Comment 9

[Jim Blandy :jimb]

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=827d4114f9e90c2c25226f4159b46ac9d24351eb

Comment 10

[Jim Blandy :jimb]

Attachment: Check for overrunning the LiveSavedFrameCache even in release builds.

Comment 11

[Jim Blandy :jimb]

Attachment: Clear the hasCachedSavedFrame flag on frames we skip following evalInFramePrev links.

Comment 12

[Jim Blandy :jimb]

Attachment: Simplify condition in FrameIter::operator++.

Comment 13

[Jason Orendorff [:jorendorff]]

Comment on attachment 8958342 [details] [diff] [review]
Check for overrunning the LiveSavedFrameCache even in release builds.

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

r=me assuming my comment is right; if not, I should re-review.

::: js/src/vm/SavedStacks.cpp
@@ +139,5 @@
>          // If the frame's bit was set, the frame should always have an entry in
>          // the cache. (If we purged the entire cache because its SavedFrames had
>          // been captured for a different compartment, then we would have
>          // returned early above.)
> +        MOZ_ALWAYS_TRUE(!frames->empty());

I think you want MOZ_RELEASE_ASSERT.

`MOZ_ALWAYS_TRUE(expr)` is like `auto tmp = expr; MOZ_ASSERT(tmp)`.

Comment 14

[Jason Orendorff [:jorendorff]]

Comment on attachment 8958343 [details] [diff] [review]
Clear the hasCachedSavedFrame flag on frames we skip following evalInFramePrev links.

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

Clearing review -- we discussed this at some length and found one concern with this approach, and a new approach we both like better.

The concern: SavedStacks::insertFrames asserts that if we've seen any cached frame, all subsequent frames we see are cached. We weren't able to convince ourselves that this invariant holds when evalInFramePrev links are present, particularly when multiple frames on the stack have them (nested frame.eval calls).

Bonus concern: I didn't like FrameIter mutating frames. It just seemed really astonishing, even if the scope of the mutation is pretty limited.

New approach: Change SavedStacks::insertFrames to use a non-evalInFramePrev-following iterator when walking the stack. Thus the algorithm and all the cache invariants trivially hold regardless of any evalInFramePrev links, because we're ignoring them altogether. Since we do want the output SavedStack objects to follow those links, we implement that by explicitly following the link when creating SavedStack objects, in the second half of SavedStacks::insertFrames.

::: js/src/vm/Stack.cpp
@@ +863,5 @@
>  
>              popInterpreterFrame();
>  
>              while (!hasUsableAbstractFramePtr() || abstractFramePtr() != eifPrev) {
> +

Style nit: please remove this blank line; the rule is to have blank lines before comments except at the top of a block.

Comment 15

[Jim Blandy :jimb]

Attachment: Part 3: Let SavedStacks::insertFrames construct its own FrameIter.

Comment 16

[Jim Blandy :jimb]

Attachment: Part 4: Construct FramePtrs from AbstractFramePtrs.

Comment 17

[Jim Blandy :jimb]

Attachment: Part 5: Downcast FramePtrs to InterpreterFrames.

Comment 18

[Jim Blandy :jimb]

Attachment: Part 6: Add LiveSavedFrameCache::findWithoutInvalidation.

Comment 19

[Jim Blandy :jimb]

Attachment: bug 1444604: Part 7: Reconcile LiveSavedFrameCache with evalInFramePrev by tweaking SavedFrame parent links.

Comment 20

[Jim Blandy :jimb]

Attachment: Part 7: Reconcile LiveSavedFrameCache with evalInFramePrev by tweaking SavedFrame parent links.

Comment 21

[Jim Blandy :jimb]

What's missing here is a test case that uses Debugger.Frame.prototype.eval to show why my original patch was wrong. I'll try to put that together tomorrow.

Comment 22

[Jim Blandy :jimb]

I'm having a hard time constructing a test case that demonstrates the concern described in comment 14.

I'm taking as given that the assertion we're afraid of breaking is the one in insertFrames that says that once we encounter a cached frame, all older frames in that traversal are also cached.

For this assertion to fail, as we walk, there must be a transition from a cached frame to an uncached frame.

Debugger eval frames' `evalInFramePrev` links mean that, from the point of view of stack capture, frames form a tree, not a stack.

Stack capture always starts in a frame at the top of the stack, and clears the bits of all frames that it skips. That means that every frame that is not a parent of the current leaf has its bit cleared; only frames that are parents of the current leaf have their bits set. Any stack capture that reaches a frame with its bit set will follow the same `evalInFramePrev` links from that point onward as the capture that set the bit, so it can never encounter a frame with its bit clear.

We needn't worry about `evalInFramePrev` links pointing at frames whose bits are clear, because the leafward frames of the link were also not part of the most recent capture path, and thus had their bits cleared too.

So I think the original plan of clearing the bit on skipped frames would not have triggered that assertion. I'm giving up on constructing the test case.

Comment 23

[Jim Blandy :jimb]

The upshot of that is, if these patches pass muster, that's all that's needed to close this bug.

Comment 24

[Jim Blandy :jimb]

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=67264704c8af2c3617b02e3f4bc14a104faff8ec

Comment 25

[Jason Orendorff [:jorendorff]]

Comment on attachment 8959387 [details] [diff] [review]
Part 4: Construct FramePtrs from AbstractFramePtrs.

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

I like reviewing small patches, but for this one, I did search through the rest of the stack to find out where the new method is used.

I think the FramePtr constructor template should assert that the argument (whatever it is) is not null, for good measure.

::: js/src/vm/Stack-inl.h
@@ +997,5 @@
>      MOZ_CRASH("unexpected frame type");
>  }
>  
> +/* static */ inline LiveSavedFrameCache::FramePtr
> +LiveSavedFrameCache::FramePtr::create(const AbstractFramePtr& afp)

Pass AbstractFramePtr by value, please. It's pointer-sized.

@@ +1008,5 @@
> +        return FramePtr(afp.asInterpreterFrame());
> +    if (afp.isWasmDebugFrame())
> +        return FramePtr(afp.asWasmDebugFrame());
> +    if (afp.isRematerializedFrame())
> +        return FramePtr(afp.asRematerializedFrame());

LiveSavedFrameCache::FramePtr and AbstractFramePtr seem to be the same type, except AbstractFramePtr is smaller. If so, all this code could be deleted and things would be a touch more efficient. Some other day, maybe.

Comment 26

[Jason Orendorff [:jorendorff]]

Comment on attachment 8959389 [details] [diff] [review]
Part 6: Add LiveSavedFrameCache::findWithoutInvalidation.

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

::: js/src/vm/Stack.h
@@ +1358,5 @@
>  
> +    // Search the cache for a frame matching |framePtr|, without removing any
> +    // entries. Return the matching saved frame, or nullptr if none is found.
> +    // This is used for resolving |evalInFramePrev| links.
> +    void findWithoutInvalidation(const FramePtr& framePtr, MutableHandleSavedFrame frame);

I think this should be private and const, if possible.

Comment 27

[Jason Orendorff [:jorendorff]]

Comment on attachment 8959392 [details] [diff] [review]
Part 7: Reconcile LiveSavedFrameCache with evalInFramePrev by tweaking SavedFrame parent links.

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

::: js/src/vm/SavedStacks.cpp
@@ +1382,5 @@
> +    // link.
> +    FrameIter iter(cx,
> +                   capture.is<JS::AllFrames>()
> +                   ? FrameIter::IGNORE_DEBUGGER_EVAL_PREV_LINK
> +                   : FrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK);

Nice comment.

Is `capture.is<JS::AllFrames>()` the right way to spell "the LiveSavedFrameCache is enabled for this stack capture"? I thought there were other conditions that affected whether the cache could be used, but I don't see them now.

::: js/src/vm/Stack.h
@@ +1262,5 @@
> +//   the entire stack, invalidating and populating the cache in the usual way.
> +//   Instead, when we construct a SavedFrame for a debugger eval frame, we
> +//   select the appropriate parent at that point: rather than the next-older
> +//   frame, we find the SavedFrame for the eval's target frame. The skip appears
> +//   in the SavedFrame chains, even as the traversal covers all the frames.

Great.

Comment 28

[Jim Blandy :jimb]

(In reply to Jason Orendorff [:jorendorff] from comment #25)
> I think the FramePtr constructor template should assert that the argument
> (whatever it is) is not null, for good measure.

Done.

> Pass AbstractFramePtr by value, please. It's pointer-sized.

Done. (mozilla::Variant types cannot be passed as arguments by value [see the comments in mfbt/Variant.h], and I think I was just in that state of mind...)

> LiveSavedFrameCache::FramePtr and AbstractFramePtr seem to be the same type,
> except AbstractFramePtr is smaller. If so, all this code could be deleted
> and things would be a touch more efficient. Some other day, maybe.

They're not quite the same type. FramePtr can also point to physical Ion frames, which cannot support all the operations that AbstractFramePtr does (without rematerialization).

Comment 29

[Jim Blandy :jimb]

Comment on attachment 8959389 [details] [diff] [review]
Part 6: Add LiveSavedFrameCache::findWithoutInvalidation.

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

::: js/src/vm/Stack.h
@@ +1358,5 @@
>  
> +    // Search the cache for a frame matching |framePtr|, without removing any
> +    // entries. Return the matching saved frame, or nullptr if none is found.
> +    // This is used for resolving |evalInFramePrev| links.
> +    void findWithoutInvalidation(const FramePtr& framePtr, MutableHandleSavedFrame frame);

I made it const. It can't be private, since it's used by SavedStacks.

Comment 30

[Jim Blandy :jimb]

(In reply to Jason Orendorff [:jorendorff] from comment #27)
> Is `capture.is<JS::AllFrames>()` the right way to spell "the
> LiveSavedFrameCache is enabled for this stack capture"? I thought there were
> other conditions that affected whether the cache could be used, but I don't
> see them now.

Yes. There are further conditions regarding which kinds of frames can be cached, but the capture is the only thing that can disable the cache altogether.

Comment 31

[Jim Blandy :jimb]

Pushed to inbound:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=1a7cb8857de44f7f0bdf665c07fbc2097bf93e93

Comment 32

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 6d0f84afc194).

Comment 33

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 34

[Jim Blandy :jimb]

Attachment: Part 1: Check for overrunning the LiveSavedFrameCache even in release builds. r=jorendorff

Comment 35

[Jim Blandy :jimb]

Attachment: Part 2: Simplify condition in FrameIter::operator++. r=jorendorff

Comment 36

[Jim Blandy :jimb]

Attachment: Part 3: Let SavedStacks::insertFrames construct its own FrameIter. r=jorendorff

Comment 37

[Jim Blandy :jimb]

Attachment: Part 4: Construct FramePtrs from AbstractFramePtrs. r=jorendorff

Comment 38

[Jim Blandy :jimb]

Attachment: Part 5: Downcast FramePtrs to InterpreterFrames. r=jorendorff

Comment 39

[Jim Blandy :jimb]

Attachment: Part 6: Add LiveSavedFrameCache::findWithoutInvalidation. r=jorendorff

Comment 40

[Jim Blandy :jimb]

Attachment: Part 7: Reconcile LiveSavedFrameCache with evalInFramePrev by tweaking SavedFrame parent links. r=jorendorff

Comment 41

[Jim Blandy :jimb]

Comment on attachment 8964061 [details] [diff] [review]
Part 1: Check for overrunning the LiveSavedFrameCache even in release builds. r=jorendorff

Approval Request Comment
[Feature/Bug causing the regression]: 1444604
[User impact if declined]: Using the JS debugger might cause crashes.
[Is this code covered by automated tests?]: Yes, added by patches in bug
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: medium risk
[Why is the change risky/not risky?]: This changes the way JS stack capture works, and JS stack capture happens frequently in ordinary browser operation. I believe the change has no effect unless the JS debugger is evaluating code, but I could be mistaken.
[String changes made/needed]: None

Comment 42

[Jim Blandy :jimb]

Comment on attachment 8964062 [details] [diff] [review]
Part 2: Simplify condition in FrameIter::operator++. r=jorendorff

Approval Request Comment: See above

Comment 43

[Jim Blandy :jimb]

Comment on attachment 8964067 [details] [diff] [review]
Part 3: Let SavedStacks::insertFrames construct its own FrameIter. r=jorendorff

Approval Request Comment: See above

Comment 44

[Jim Blandy :jimb]

Comment on attachment 8964068 [details] [diff] [review]
Part 4: Construct FramePtrs from AbstractFramePtrs. r=jorendorff

Approval Request Comment: See above

Comment 45

[Jim Blandy :jimb]

Comment on attachment 8964069 [details] [diff] [review]
Part 5: Downcast FramePtrs to InterpreterFrames. r=jorendorff

Approval Request Comment: See above

Comment 46

[Jim Blandy :jimb]

Comment on attachment 8964070 [details] [diff] [review]
Part 6: Add LiveSavedFrameCache::findWithoutInvalidation. r=jorendorff

Approval Request Comment: See above

Comment 47

[Jim Blandy :jimb]

Comment on attachment 8964071 [details] [diff] [review]
Part 7: Reconcile LiveSavedFrameCache with evalInFramePrev by tweaking SavedFrame parent links. r=jorendorff

Approval Request Comment: See above

Comment 48

[Jim Blandy :jimb]

The patches flagged for beta above are identical to the changeset range da9491adea85:1a7cb8857de4 on central.

Comment 49

[Ryan VanderMeulen [:RyanVM]]

For posterity, this did in fact get merged to m-c too.
https://hg.mozilla.org/mozilla-central/rev/da9491adea8584c9780cb57fad6022ba7b871e2e
https://hg.mozilla.org/mozilla-central/rev/27993b0b6e51ce8d6ad22ed4d0d1471e286b5872
https://hg.mozilla.org/mozilla-central/rev/9579abbc85f6e39f94b85d44bb604181b24b2636
https://hg.mozilla.org/mozilla-central/rev/54f35c7183e4db5b8a6262e8d566a9cbcde5bcfe
https://hg.mozilla.org/mozilla-central/rev/0bf319672ac38bf3cec262bfe97d8b4fdc21b15f
https://hg.mozilla.org/mozilla-central/rev/00c6c3c10a3aed6e3dacc35727ce97628c91a4bd
https://hg.mozilla.org/mozilla-central/rev/1a7cb8857de44f7f0bdf665c07fbc2097bf93e93

Comment 50

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8964061 [details] [diff] [review]
Part 1: Check for overrunning the LiveSavedFrameCache even in release builds. r=jorendorff

Fix a sec issue, approved for 60.0b9.

Comment 51

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6d530e6cb06d
https://hg.mozilla.org/releases/mozilla-beta/rev/2d48365acd9f
https://hg.mozilla.org/releases/mozilla-beta/rev/4518879e2140
https://hg.mozilla.org/releases/mozilla-beta/rev/4e8e98727140
https://hg.mozilla.org/releases/mozilla-beta/rev/1e6efa9a8b0b
https://hg.mozilla.org/releases/mozilla-beta/rev/68469fba2454
https://hg.mozilla.org/releases/mozilla-beta/rev/c31d3d320dbd

Comment 52

[Andrei Vaida [:avaida]]

(In reply to Jim Blandy :jimb from comment #41)
> [Is this code covered by automated tests?]: Yes, added by patches in bug
> [Has the fix been verified in Nightly?]: Yes
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Jim's assessment on manual testing needs and the fact that this fix has automated tests.

Comment 53

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.