| Summary: | Use uninitialized register in "JIT::emit_op_neq_null" and "emit_op_eq_null" | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peter Wang <PeterHWang> | ||||||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | charles.wei, commit-queue, ctruta, fpizlo, ggaren, mhahnenberg, PeterHWang | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Other | ||||||||||||
| OS: | Other | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 202501 [details]
Patch
Can you add a test case for this? See LayoutTests/fast/js/typeof-codegen-crash.html for an example test case. There's a typo in ChangeLog: "cuased" --> "caused" Comment on attachment 202501 [details]
Patch
r=me
Created attachment 202639 [details]
Patch
(In reply to comment #2) > Can you add a test case for this? See LayoutTests/fast/js/typeof-codegen-crash.html for an example test case. Thank you very much for help (In reply to comment #5) > Created an attachment (id=202639) [details] > Patch Correct a typo and supplement a test case. Comment on attachment 202639 [details] Patch Rejecting attachment 202639 [details] from commit-queue. mawu@blackberry.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. Comment on attachment 202639 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202639&action=review > LayoutTests/fast/js/script-tests/neq-null-crash.js:9 > +for (var i = 1; i < 20; i++) { > + shouldBeFalse("crush()"); > +} Can you change this to i < 100? That will be enough to make the DFG JIT kick in, and this patch will automatically have way more coverage. :-) (In reply to comment #9) > (From update of attachment 202639 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202639&action=review > > > LayoutTests/fast/js/script-tests/neq-null-crash.js:9 > > +for (var i = 1; i < 20; i++) { > > + shouldBeFalse("crush()"); > > +} > > Can you change this to i < 100? That will be enough to make the DFG JIT kick in, and this patch will automatically have way more coverage. :-) I'll be happy to cq+ once you make that change. Comment on attachment 202639 [details]
Patch
crush == > crash
(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 202639 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=202639&action=review > > > > > LayoutTests/fast/js/script-tests/neq-null-crash.js:9 > > > +for (var i = 1; i < 20; i++) { > > > + shouldBeFalse("crush()"); > > > +} > > > > Can you change this to i < 100? That will be enough to make the DFG JIT kick in, and this patch will automatically have way more coverage. :-) > > I'll be happy to cq+ once you make that change. Ok. Thank you. Created attachment 202641 [details]
Patch
Comment on attachment 202641 [details]
Patch
Oh, oops! I lied. This is missing a LayoutTests/ChangeLog. You can list me as reviewer.
Created attachment 202642 [details]
Patch
Comment on attachment 202642 [details]
Patch
Awesome! Thanks for the fix and the test!
The commit-queue encountered the following flaky tests while processing attachment 202642 [details]: svg/batik/paints/patternPreserveAspectRatioA.svg bug 114139 (author: zimmermann@kde.org) svg/batik/text/textEffect3.svg bug 116521 (authors: darin@apple.com and zimmermann@kde.org) media/track/track-remove-crash.html bug 115892 (author: eric.carlson@apple.com) media/audio-repaint.html bug 116648 (authors: jer.noble@apple.com, pnormand@igalia.com, and rniwa@webkit.org) The commit-queue is continuing to process your patch. Comment on attachment 202642 [details] Patch Clearing flags on attachment: 202642 Committed r150569: <http://trac.webkit.org/changeset/150569> All reviewed patches have been landed. Closing bug. Nice! |
In JITOpcodes32_64.cpp:794 and JITOpcodes32_64.cpp:825, a statement loadPtr(Address(regT2, Structure::globalObjectOffset()), regT2); uses uninitialized register.This mistake will cause random crush. My port is based on ARMv7, repeatedly invoking following js code will cause crush: ---------------------------------------------------------------------------- function crush() { return (document.all!=null && b); } ---------------------------------------------------------------------------- The gdb output ---------------------------------------------------------------------------- Program received signal SIGSEGV, Segmentation fault. [Switching to pid 38916178 tid 3 name "webkit_main"] 0x79697bf4 in ?? () (gdb) bt #0 0x79697bf4 in ?? () #1 0x79697c74 in ?? () #2 0x79697c74 in ?? () Backtrace stopped: previous frame identical to this frame (corrupt stack?) (gdb) x/50i 0x79697bc4 0x79697bc4: adds r3, #90 ; 0x5a 0x79697bc6: str r0, [r3, #0] 0x79697bc8: movw r3, #31220 0x79697bcc: movt r3, #31578 0x79697bd0: str r1, [r3, #0] 0x79697bd2: str r0, [r5, #0] // Following instructions are generated by "emit_op_neq_null" 0x79697bd4: str r1, [r5, #4] 0x79697bd6: cmn.w r1, #5 0x79697bda: bne.n 0x79697c00 0x79697bdc: ldr r1, [r0, #0] 0x79697bde: ldrb.w r3, [r1, #53] 0x79697be2: tst.w r3, #1 0x79697be6: bne.n 0x79697bec 0x79697be8: movs r1, #1 0x79697bea: b.n 0x79697c16 0x79697bec: movw r0, #61496 0x79697bf0: movt r0, #31128 => 0x79697bf4: ldr r2, [r2, #4] // Generated by JITOpcodes32_64.cpp:825 0x79697bf6: cmp r0, r2 (gdb) p/x $r2 // The content of r2 is an invalid address $1 = 0xfffffffb ---------------------------------------------------------------------------- Dumped bytecode ---------------------------------------------------------------------------- Source: function IsFilterNeeded() { return (document.all!=null && b); } [ 0] enter [ 1] resolve r1, document(@id0), 2067299048 [ 6] get_by_id r0, r1, all(@id1) [ 15] neq_null r0, r0 [ 18] jfalse r0, 8(->26) [ 21] resolve r0, b(@id2), 2067299060 [ 26] ret r0 Identifiers: id0 = document id1 = all id2 = b ----------------------------------------------------------------------------