Closed
Bug 681092
Opened 13 years ago
Closed 10 years ago
Add script size assertions
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: billm, Assigned: billm)
References
Details
Attachments
(2 files)
(deleted),
patch
|
dmandelin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
This patch tries to track down the issues identified in bug 670702 comment 18.
Attachment #554976 -
Flags: review?(dmandelin)
Assignee | ||
Comment 1•13 years ago
|
||
Comment on attachment 554976 [details] [diff] [review]
patch
Never mind with the review. This is tripping on tryserver. Either I messed up or else the problem shouldn't be too hard to fix.
Attachment #554976 -
Flags: review?(dmandelin)
Assignee | ||
Updated•13 years ago
|
Attachment #554976 -
Flags: review?(dmandelin)
Assignee | ||
Comment 2•13 years ago
|
||
This fixes a small bug that caused overallocation for source notes. I also tried to simplify the code. Hopefully I didn't break anything in the process.
This is not the bug that I was looking for, but we need to fix it to make the assertions useful.
Attachment #555004 -
Flags: review?(brendan)
Updated•13 years ago
|
Attachment #554976 -
Flags: review?(dmandelin)
Assignee | ||
Comment 3•13 years ago
|
||
Did you mean to cancel the review, Dave? I'd still like to get the assertions in. I'm nearly certain that the srcnote patch won't fix the problem I'm tracking.
Updated•13 years ago
|
Attachment #554976 -
Flags: review+
Comment 4•13 years ago
|
||
Comment on attachment 555004 [details] [diff] [review]
sourcenote patch
>+ /* Don't forget to account for the terminator. */
>+ *count = cg->prolog.noteCount + cg->main.noteCount + 1;
>+
>+ return JS_TRUE;
Feel free in this patch to s/JS_TRUE/true/g and s/JS_FALSE/false/g -- doing so makes it easier to abut the two non-empty lines cited above, which is more like house style.
Thanks for doing this needed cleanup. We talked too long ago and I failed to check my review q -- sorry! Please mail me when I do that.
/be
Attachment #555004 -
Flags: review?(brendan) → review+
Comment 5•10 years ago
|
||
Bill: did you forget to land this r+'d patch from 2011? :) Are these script size assertions still relevant?
Flags: needinfo?(wmccloskey)
Assignee | ||
Comment 6•10 years ago
|
||
I think my intention here was to track down a crash that eventually was fixed in some other way. The code looks pretty different at this point, so it would probably be more trouble than it's worth to rebase these patches.
Flags: needinfo?(wmccloskey)
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•