Bug 20876 - REGRESSION (r36417, r36427): fast/js/exception-expression-offset.html fails
Summary: REGRESSION (r36417, r36427): fast/js/exception-expression-offset.html fails
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Cameron Zwarich (cpst)
Depends on:
Reported: 2008-09-15 22:08 PDT by Cameron Zwarich (cpst)
Modified: 2008-09-17 23:22 PDT (History)
2 users (show)

See Also:

Proposed patch for 'instanceof' case (1.27 KB, patch)
2008-09-17 20:59 PDT, Cameron Zwarich (cpst)
no flags Details | Formatted Diff | Diff
Proposed patch for 'instanceof' case (5.18 KB, patch)
2008-09-17 21:00 PDT, Cameron Zwarich (cpst)
oliver: review-
Details | Formatted Diff | Diff
Proposed patch (9.55 KB, patch)
2008-09-17 22:54 PDT, Cameron Zwarich (cpst)
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron Zwarich (cpst) 2008-09-15 22:08:29 PDT
Recent changes to op_instanceof and op_construct in r36417 and r36427 mean that exception data is not correctly emitted for op_instanceof and op_construct. An op_get_by_id is emitted to get the prototype property ahead of time, but there is no exception data for these new opcodes. This shouldn't be too hard to fix: just make these opcodes use ThrowableSubExpressionData and emit the right information.
Comment 1 Cameron Zwarich (cpst) 2008-09-17 20:59:50 PDT
Created attachment 23519 [details]
Proposed patch for 'instanceof' case

Here's a patch for the 'instanceof' case. The constructor case requires actual subexpression data, but I should be able to do it easily after figuring out this case.
Comment 2 Cameron Zwarich (cpst) 2008-09-17 21:00:36 PDT
Created attachment 23520 [details]
Proposed patch for 'instanceof' case

Oops. Here is the version with the layout test changes as well.
Comment 3 Oliver Hunt 2008-09-17 21:07:43 PDT
Comment on attachment 23520 [details]
Proposed patch for 'instanceof' case

r- as this reduces the fidelity of the error messages, and the error range no longer covers the entire instanceof
Comment 4 Cameron Zwarich (cpst) 2008-09-17 22:54:35 PDT
Created attachment 23521 [details]
Proposed patch

This patch fixes both cases, and it does not change any of the exception information at all.
Comment 5 Maciej Stachowiak 2008-09-17 23:16:36 PDT
Comment on attachment 23521 [details]
Proposed patch

Comment 6 Cameron Zwarich (cpst) 2008-09-17 23:22:08 PDT
Landed in r36604.