Assertion decompiling try-catch(guard)-finally statements

VERIFIED FIXED in mozilla1.8.1beta2

Status

()

Core
JavaScript Engine
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

({assertion, verified1.8.1})

Trunk
mozilla1.8.1beta2
assertion, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
This is a spin-off from bug 346494. See especially: bug 346494 comment 4.
(Assignee)

Comment 1

12 years ago
Created attachment 233169 [details] [diff] [review]
Fix

The code generator wasn't accounting for the fact that though the JSOP_GOSUB had +1 stack accountability, the matching JSOP_RETSUB was -1, giving a net of 0 for that opcode. Therefore, we have to manually adjust cg->stackDepth to the right thing.
Attachment #233169 - Flags: review?(brendan)

Updated

12 years ago
Attachment #233169 - Flags: review?(brendan) → review+
This is a serious bug, not only for its effect on decompilation (Function.prototype.toString) but for the crash, which might be controllable.

/be
Flags: blocking1.8.1?
Comment on attachment 233169 [details] [diff] [review]
Fix

Correct by inspection and supported by assertions -- I don't think this needs much bake-time.

The js testsuite does need cases covering all the combos.  mrbkap has one in his copy buffer.

/be
Attachment #233169 - Flags: approval1.8.1?
(In reply to comment #2)
> This is a serious bug, not only for its effect on decompilation
> (Function.prototype.toString) but for the crash, which might be controllable.

Oops, this really applies to bug 346494.  This bug, thanks to the decompiler's use of runtime-safe LOCAL_ASSERT, does not crash release builds.  It does assertbotch debug builds, and it should go into 1.8.1.

/be
Flags: blocking1.8.1? → blocking1.8.1+
Comment on attachment 233169 [details] [diff] [review]
Fix

a=drivers, 'cause brendan says so.
Attachment #233169 - Flags: approval1.8.1? → approval1.8.1+
Fixed on the 1.8 branch too (this landed on the trunk on 8/10.

/be
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
(In reply to comment #3)
> 
> The js testsuite does need cases covering all the combos.  mrbkap has one in
> his copy buffer.

mrbkap? 
(Assignee)

Comment 8

12 years ago
(In reply to comment #7)
> mrbkap? 

Ah, that copy buffer is long gone. Sorry.
Checking in regress-346494.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-346494.js,v  <--  regress-346494.js
initial revision: 1.1
Flags: in-testsuite+
(In reply to comment #9)

oops, wrong bug.
verified fixed 1.8, 1.9 20060818 windows/mac*/linux since the testcases in Bug 346494 covers this.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
You need to log in before you can comment on or make changes to this bug.