1033020 - Make BaslineFrame::returnValue match InterpreterFrame::returnValue

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Terrence Cole [:terrence]]

Attachment: missing_hasReturnValue_check-v0.diff

Right now, if you access BaselineFrame::returnValue and !hasReturnValue, you get random stack data (note, this should at least have had an assertion). On the other hand, it is safe to call InterpreterFrame::returnValue whenever: it will just give back UndefinedValue. This makes AbstractFramePointer's wrapper have inconsistent behavior. In particular, Debugger::slowPathOnLeaveFrame was not checking hasReturnValue, causing exception aborts in the debugger with jit frames on stack to read random memory from the stack as a Value.

I think the right fix here is to make both frame classes behave the same.

Comment 1

[Terrence Cole [:terrence]]

Attachment: missing_hasReturnValue_check-v1.diff

Missed one subtlety: in setReturnValue, we need to set the value before the flag so that HandleValue doesn't see garbage in the constructor.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8449027 [details] [diff] [review]
missing_hasReturnValue_check-v1.diff

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

Good find!

Comment 3

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5e0699e4b50

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c5e0699e4b50

Comment 5

[Matt Wobensmith [:mwobensmith][:matt:]]

Looks like this came from a code audit. If a test case appears, let me know so I can verify this bug. Otherwise, marking [qe-verify-].