| Summary: | Do bytecode validation as part of testing | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||
| Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | barraclough, benjamin, cmarcelo, commit-queue, ggaren, gyuyoung.kim, mark.lam, mhahnenberg, msaboff, nrotem, oliver, rakuco, sam | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Filip Pizlo
2013-11-26 21:10:17 PST
Created attachment 217924 [details]
the patch
LGTM. The FastVector change can go as a separate patch. What is this change: - unsigned numberOfVariables = unlinkedCodeBlock->m_numVars + + unsigned numberOfVariables = (In reply to comment #2) > LGTM. The FastVector change can go as a separate patch. Yeah, it can land separately. We often prefer not to land dead code - so if we add WTF methods that we use for feature X then the patch for feature X will usually include the WTF patch inside it. I guess the theory is that if you branched WebKit at any arbitrary revision, not only would you expect to get a reasonably stable web engine, but also you would get one that didn't have unneeded code. > > What is this change: > > - unsigned numberOfVariables = unlinkedCodeBlock->m_numVars + > + unsigned numberOfVariables = A bug I found by adding the bytecode liveness analysis verifier. I was surprised by the number of bits in the bitvector. It turns out that the liveness analysis had a benign bug where it assumed that the number of variables was the sum of numVars and numCalleeRegs. In fact, numCalleeRegs is numVars + numTemps, so adding numVars is incorrect. Prior to this patch it would have been a benign bug because it really doesn't matter if you create a bitvector that is larger than you need. I mean, you waste some memory, but probably not a lot. Comment on attachment 217924 [details] the patch Attachment 217924 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/37828025 Created attachment 217958 [details]
the patch
Landed in http://trac.webkit.org/changeset/159825 |